fix(dpp)!: make platform/orchard address decoders network-agnostic#3781
fix(dpp)!: make platform/orchard address decoders network-agnostic#3781Claudius-Maginificent wants to merge 4 commits into
Conversation
`signing_tests` for `ShieldFromAssetLockTransition` imports `crate::shielded::builder`, which only exists under `shielded-client`, but the module was gated on `state-transition-signing` + `core_key_wallet` only. With `core_key_wallet` on but `shielded-client` off, the dpp lib-test harness failed to compile (`unresolved import crate::shielded::builder`). Add `feature = "shielded-client"` to the module's gate (both the `mod` declaration and the file's inner `#![cfg]`) so the test only compiles when its `builder` dependency is available. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`PlatformAddress::from_bech32m_string` and `OrchardAddress::from_bech32m_string` previously returned `(Self, Network)`, deriving the Network from the bech32m HRP. That value was untruthful: the `tdash` HRP is shared by Testnet, Devnet, and Regtest, so the decoder hardcoded `tdash → Testnet` and mis-reported devnet/regtest addresses as Testnet — which broke devnet (a devnet platform address tripped a wrong-network guard). A `PlatformAddress`/`OrchardAddress` is network-agnostic internally; the network is supplied only at `to_bech32m_string(network)` encode time. Both decoders now make no network claim: they validate the HRP is a recognized platform HRP (`dash`/`tdash`) and return just `Self`. The wrong-network safety check moves to the one caller that needs it, `platform-wallet::shielded_unshield_to`, as an HRP-class comparison: the recipient's HRP (segment before the final bech32 `1` separator) is matched case-insensitively against `hrp_for_network(wallet.network)`, catching e.g. a mainnet `dash1…` address pasted into a `tdash` wallet. `shielded_withdraw_to` uses a separate Base58 core-address parser and is unaffected. BREAKING: `from_bech32m_string` return type changed `(Self, Network)` → `Self`. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughAddress parsing was changed to be network-agnostic: OrchardAddress and PlatformAddress parsers now return only decoded addresses and validate HRPs; PlatformAddress gained an HRP-based mainnet classifier. Wallet code now enforces network checks before decoding. Tests and WASM bindings updated; shielded test gates tightened. ChangesNetwork-agnostic address parsing with wallet-level validation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ctor Restores the truthful subset of the network information that the network-agnostic `from_bech32m_string` decoder no longer returns. Per DIP-0018, a bech32m platform address carries no network byte and the prefix `tdash` is shared by Testnet/Devnet/Regtest, so the only network fact recoverable from an address string is mainnet vs non-mainnet. `PlatformAddress::is_mainnet_bech32m(s) -> Result<bool, ProtocolError>` classifies by HRP alone (no payload decode): `dash` → Ok(true), `tdash` → Ok(false), a non-platform HRP or a malformed/separator-less string → Err. Comparison is case-insensitive (bech32m permits all-uppercase). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…is_mainnet_bech32m `check_recipient_hrp` no longer inlines the bech32 HRP extraction and HRP-class comparison; it delegates network classification (and malformed/non-platform rejection) to `PlatformAddress::is_mainnet_bech32m`, making the dpp detector the single source of truth. The guard reduces to `addr_is_mainnet != (network == Network::Mainnet)` — exactly the previous HRP-class check (mainnet wallet requires `dash`, any non-mainnet wallet requires `tdash`). Behavior-preserving: the existing `check_recipient_hrp_tests` (network mismatch, tdash cross-acceptance, uppercase, non-platform, missing-separator, empty) pass unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| ))) | ||
| } | ||
| }; | ||
| if hrp_lower != PLATFORM_HRP_MAINNET && hrp_lower != PLATFORM_HRP_TESTNET { |
There was a problem hiding this comment.
Looks like we use the same validation multiple times. DRY. Maybe just call is_mainnet_bech32m() ?
| ))) | ||
| } | ||
| }; | ||
| if hrp_lower != PLATFORM_HRP_MAINNET && hrp_lower != PLATFORM_HRP_TESTNET { |
There was a problem hiding this comment.
Looks like we use the same validation multiple times. DRY. Maybe just call is_mainnet_bech32m() ?
| /// [`Self::to_bech32m_string`] encode time. The HRP is validated to be a | ||
| /// recognized platform HRP (`dash`/`tdash`), but no network is inferred — | ||
| /// `tdash` is shared by Testnet/Devnet/Regtest, so the HRP cannot identify | ||
| /// the network. Callers needing a network guard must enforce it themselves. |
There was a problem hiding this comment.
Make the comment shorter, references to to_bench32m_string() are not needed here, and add reference to is_mainnet_bech32m()
| /// NOTE: This expects bech32m type bytes (0xb0/0x80) in the encoded string, | ||
| /// NOT the storage type bytes (0x00/0x01) used in GroveDB keys. | ||
| /// | ||
| /// A `PlatformAddress` is network-agnostic: the network is supplied only at |
There was a problem hiding this comment.
Make the comment shorter, references to to_bench32m_string() are not needed here, and add reference to is_mainnet_bech32m()
|
@coderabbitai review all |
|
✅ Actions performedFull review triggered. |
|
✅ Review complete (commit 71052a4) |
There was a problem hiding this comment.
Pull request overview
Updates platform/orchard bech32m address decoding to be network-agnostic (since tdash is shared by Testnet/Devnet/Regtest), and moves the network-class guard to the wallet caller that needs it.
Changes:
- Change
PlatformAddress::from_bech32m_string/OrchardAddress::from_bech32m_stringto return only the decoded address (no derivedNetwork) and validate only that HRP is a recognized platform HRP. - Add
PlatformAddress::is_mainnet_bech32mand use it inrs-platform-walletto enforce mainnet vs non-mainnet recipient HRP class inshielded_unshield_to, with targeted unit tests. - Feature-gate
shield_from_asset_lock_transitionsigning tests behindshielded-client; update wasm wrapper call sites for the new decoder return type.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/wasm-dpp2/src/platform_address/address.rs | Update wasm bindings to handle from_bech32m_string returning only PlatformAddress. |
| packages/rs-platform-wallet/src/wallet/platform_wallet.rs | Move wrong-network enforcement to wallet via check_recipient_hrp + add tests. |
| packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rs | Gate signing tests behind shielded-client feature to avoid compile failures. |
| packages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/mod.rs | Gate inclusion of signing tests behind shielded-client feature. |
| packages/rs-dpp/src/address_funds/platform_address.rs | Make platform bech32m decoder network-agnostic; add is_mainnet_bech32m helper + tests; update parsing/tests for new return type. |
| packages/rs-dpp/src/address_funds/orchard_address.rs | Make orchard bech32m decoder network-agnostic; update tests for new return type. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let addr_is_mainnet = PlatformAddress::is_mainnet_bech32m(recipient).map_err(|e| { | ||
| PlatformWalletError::ShieldedBuildError(format!("invalid platform address: {e}")) | ||
| })?; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-dpp/src/address_funds/platform_address.rs`:
- Around line 318-338: The is_mainnet_bech32m function currently only splits on
the last '1' and checks HRP, allowing malformed strings to pass; update
is_mainnet_bech32m to fully validate the bech32m string before classifying the
HRP: attempt to decode the input using the bech32 decoder (or the project’s
existing bech32/bech32m utility) and ensure the variant is Bech32m and the data
part is non-empty, then check hrp.eq_ignore_ascii_case(PLATFORM_HRP_MAINNET) or
PLATFORM_HRP_TESTNET and return Ok(true/false); on any decode/validation failure
return ProtocolError::DecodingError (keep check_recipient_hrp behavior unchanged
except it will now see validated addresses).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e5e0798f-49eb-4fcc-a3f2-093fc94f6e9f
📒 Files selected for processing (6)
packages/rs-dpp/src/address_funds/orchard_address.rspackages/rs-dpp/src/address_funds/platform_address.rspackages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/mod.rspackages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rspackages/rs-platform-wallet/src/wallet/platform_wallet.rspackages/wasm-dpp2/src/platform_address/address.rs
| pub fn is_mainnet_bech32m(s: &str) -> Result<bool, ProtocolError> { | ||
| let hrp = s | ||
| .rsplit_once('1') | ||
| .map(|(hrp, _)| hrp) | ||
| .filter(|h| !h.is_empty()) | ||
| .ok_or_else(|| { | ||
| ProtocolError::DecodingError( | ||
| "invalid platform address: missing bech32 separator".to_string(), | ||
| ) | ||
| })?; | ||
|
|
||
| if hrp.eq_ignore_ascii_case(PLATFORM_HRP_MAINNET) { | ||
| Ok(true) | ||
| } else if hrp.eq_ignore_ascii_case(PLATFORM_HRP_TESTNET) { | ||
| Ok(false) | ||
| } else { | ||
| Err(ProtocolError::DecodingError(format!( | ||
| "not a platform address: HRP '{hrp}' is neither \ | ||
| '{PLATFORM_HRP_MAINNET}' nor '{PLATFORM_HRP_TESTNET}'" | ||
| ))) | ||
| } |
There was a problem hiding this comment.
Validate the string before classifying it as bech32m.
This helper currently just splits on the last 1 and checks the HRP, so malformed inputs like dash1! still return Ok(true). That makes the public *_bech32m contract too permissive, and downstream check_recipient_hrp() can end up surfacing a misleading network-mismatch error before the actual decoder rejects the address.
Proposed fix
pub fn is_mainnet_bech32m(s: &str) -> Result<bool, ProtocolError> {
- let hrp = s
- .rsplit_once('1')
- .map(|(hrp, _)| hrp)
- .filter(|h| !h.is_empty())
- .ok_or_else(|| {
- ProtocolError::DecodingError(
- "invalid platform address: missing bech32 separator".to_string(),
- )
- })?;
-
- if hrp.eq_ignore_ascii_case(PLATFORM_HRP_MAINNET) {
+ let (hrp, _) =
+ bech32::decode(s).map_err(|e| ProtocolError::DecodingError(format!("{e}")))?;
+
+ if hrp.as_str().eq_ignore_ascii_case(PLATFORM_HRP_MAINNET) {
Ok(true)
- } else if hrp.eq_ignore_ascii_case(PLATFORM_HRP_TESTNET) {
+ } else if hrp.as_str().eq_ignore_ascii_case(PLATFORM_HRP_TESTNET) {
Ok(false)
} else {
Err(ProtocolError::DecodingError(format!(
- "not a platform address: HRP '{hrp}' is neither \
+ "not a platform address: HRP '{}' is neither \
'{PLATFORM_HRP_MAINNET}' nor '{PLATFORM_HRP_TESTNET}'"
+ , hrp
)))
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rs-dpp/src/address_funds/platform_address.rs` around lines 318 -
338, The is_mainnet_bech32m function currently only splits on the last '1' and
checks HRP, allowing malformed strings to pass; update is_mainnet_bech32m to
fully validate the bech32m string before classifying the HRP: attempt to decode
the input using the bech32 decoder (or the project’s existing bech32/bech32m
utility) and ensure the variant is Bech32m and the data part is non-empty, then
check hrp.eq_ignore_ascii_case(PLATFORM_HRP_MAINNET) or PLATFORM_HRP_TESTNET and
return Ok(true/false); on any decode/validation failure return
ProtocolError::DecodingError (keep check_recipient_hrp behavior unchanged except
it will now see validated addresses).
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3.1-dev #3781 +/- ##
=========================================
Coverage 87.18% 87.19%
=========================================
Files 2624 2624
Lines 321014 321066 +52
=========================================
+ Hits 279892 279940 +48
- Misses 41122 41126 +4
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Pinned commit 71052a4 is a small, behavior-preserving refactor that routes check_recipient_hrp through the existing PlatformAddress::is_mainnet_bech32m helper to centralize network classification. The diff is a clean inversion (17 added / 29 removed) and the existing check_recipient_hrp_tests continue to cover mismatch, tdash cross-acceptance, uppercase, non-platform HRP, missing-separator and empty cases. The convergent finding from Codex/Claude about is_mainnet_bech32m doing HRP-only classification rather than full bech32m decoding targets a function added in an earlier commit (7a02728) whose docstring explicitly documents the HRP-only contract — the pinned commit does not introduce that behavior and the address is still rejected by the subsequent from_bech32m_string decode for any HRP-class-matching but malformed input.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/rs-dpp/src/address_funds/platform_address.rs (1)
318-338:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate the full address before classifying its HRP.
This helper only splits on the last
1, so malformed inputs likedash1!ordash1still classify as mainnet. Downstream,shielded_unshield_tocan then return a bogus network-mismatch error before the real decoder gets a chance to reject the invalid address. Reuse the strict bech32m parser here instead of HRP-only string splitting. (docs.rs)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/rs-dpp/src/address_funds/platform_address.rs` around lines 318 - 338, The is_mainnet_bech32m helper currently only splits on the last '1' which misclassifies malformed inputs; replace the HRP-only split with a strict bech32 decoder (use bech32::decode) inside is_mainnet_bech32m to fully validate the address, ensure the variant is Bech32m, extract the decoded HRP, then compare it against PLATFORM_HRP_MAINNET and PLATFORM_HRP_TESTNET and return Ok(true)/Ok(false) or a ProtocolError::DecodingError on invalid variant/decoding failures; keep error messages descriptive and use the existing function name and HRP constants to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/rs-dpp/src/address_funds/orchard_address.rs`:
- Around line 100-102: OrchardAddress::from_bech32m_string currently uses
bech32::decode which accepts both Bech32 and Bech32m checksums; change it to use
the strict Bech32m API (e.g., bech32::primitives::CheckedHrpString /
decode_bech32m or equivalent decode_bech32m helper) so only Bech32m-checked
inputs succeed, map primitive decode errors into ProtocolError::DecodingError
like the current code, and update/add a unit test ensuring a Bech32
(non-Bech32m) encoded address is rejected by from_bech32m_string.
In `@packages/rs-dpp/src/address_funds/platform_address.rs`:
- Around line 259-262: PlatformAddress::from_bech32m_string currently calls
bech32::decode(s) which accepts both Bech32 and Bech32m checksums; change the
implementation to explicitly validate the checksum is Bech32m (rather than any
Bech32 variant) before proceeding. Replace the bech32::decode call in
from_bech32m_string with the bech32 primitives API that enforces Bech32m (e.g.,
the CheckedHrpstring/Bech32m-specific decode path) and map any non-Bech32m or
decode failures to ProtocolError::DecodingError so plain Bech32-checked strings
are rejected.
---
Duplicate comments:
In `@packages/rs-dpp/src/address_funds/platform_address.rs`:
- Around line 318-338: The is_mainnet_bech32m helper currently only splits on
the last '1' which misclassifies malformed inputs; replace the HRP-only split
with a strict bech32 decoder (use bech32::decode) inside is_mainnet_bech32m to
fully validate the address, ensure the variant is Bech32m, extract the decoded
HRP, then compare it against PLATFORM_HRP_MAINNET and PLATFORM_HRP_TESTNET and
return Ok(true)/Ok(false) or a ProtocolError::DecodingError on invalid
variant/decoding failures; keep error messages descriptive and use the existing
function name and HRP constants to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a96225bb-7095-4d18-8312-24685dbb767e
📒 Files selected for processing (6)
packages/rs-dpp/src/address_funds/orchard_address.rspackages/rs-dpp/src/address_funds/platform_address.rspackages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/mod.rspackages/rs-dpp/src/state_transition/state_transitions/shielded/shield_from_asset_lock_transition/signing_tests.rspackages/rs-platform-wallet/src/wallet/platform_wallet.rspackages/wasm-dpp2/src/platform_address/address.rs
| pub fn from_bech32m_string(s: &str) -> Result<Self, ProtocolError> { | ||
| let (hrp, data) = | ||
| bech32::decode(s).map_err(|e| ProtocolError::DecodingError(format!("{}", e)))?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the current from_bech32m_string implementation and surrounding code
sed -n '1,220p' packages/rs-dpp/src/address_funds/orchard_address.rs | nl -ba | sed -n '1,220p'
# Narrow to the specific function region (around lines 80-130 and any helper usage)
sed -n '70,150p' packages/rs-dpp/src/address_funds/orchard_address.rs | nl -ba
# Find bech32 usage in this file (and nearby modules, if any)
rg -n "bech32::|bech32m|from_bech32m_string" packages/rs-dpp/src/address_funds/orchard_address.rs
rg -n "bech32" packages/rs-dpp/Cargo.toml packages/rs-dpp/Cargo.lock Cargo.toml 2>/dev/null || true
# If Cargo.lock exists at repo root, print the bech32 version entry
if [ -f Cargo.lock ]; then
rg -n "name = \"bech32\"" -n Cargo.lock -C 2 || true
fi
if [ -f packages/rs-dpp/Cargo.lock ]; then
rg -n "name = \"bech32\"" -n packages/rs-dpp/Cargo.lock -C 2 || true
fiRepository: dashpay/platform
Length of output: 103
🌐 Web query:
bech32 crate version 0.11 bech32::decode accepts bech32 or bech32m strict api
💡 Result:
In the bech32 crate version 0.11 and later, the top-level bech32::decode function does not provide a strict API that allows you to specify or enforce a single checksum algorithm (such as Bech32 or Bech32m) directly within the function call [1][2]. Instead, bech32::decode is designed to handle strings that include a valid checksum according to either the BIP-173 (Bech32) or BIP-350 (Bech32m) specifications [3][4]. For strict validation or to enforce a specific checksum algorithm, you should use the primitives::decode module [5][6]. Specifically, the CheckedHrpstring type provides a strict API where you can explicitly specify the expected checksum algorithm as a generic parameter (e.g., CheckedHrpstring::new:: or CheckedHrpstring::new::) [6]. If your application requires parsing modern Bitcoin SegWit addresses, it is recommended to use the crate::segwit API, which handles the necessary version and checksum validations according to the relevant BIPs [5][1].
Citations:
- 1: https://docs.rs/bech32/
- 2: https://docs.rs/bech32/^0.11
- 3: https://docs.rs/crate/bech32/latest/source/CHANGELOG.md
- 4: https://docs.rs/crate/bech32/latest
- 5: https://docs.rs/bech32/latest/bech32/primitives/decode/index.html
- 6: https://docs.rs/bech32/0.11.1/x86_64-unknown-linux-gnu/bech32/primitives/decode/index.html
🌐 Web query:
docs.rs bech32 0.11 decode returns variant or detects checksum type
💡 Result:
In the bech32 crate version 0.11, the top-level decode function does not return a variant or explicitly indicate which checksum type (Bech32 or Bech32m) was detected [1][2]. The function signature is: pub fn decode(s: &str) -> Result<(Hrp, Vec), DecodeError> [1][2] According to the documentation, this function succeeds if the input string is well-formed and contains either a valid bech32m checksum or a valid bech32 checksum [1][2]. It returns the human-readable part (HRP) and the data bytes with the checksum removed [2]. If you require explicit control over the checksum algorithm or need to distinguish between them, you should use the primitives::decode::CheckedHrpstring type [3][2]. This type allows you to specify the checksum algorithm (e.g., Bech32, Bech32m, or NoChecksum) during construction [2]. The library explicitly states that for scenarios needing such control, CheckedHrpstring should be used instead of the top-level decode function [3][2].
Citations:
- 1: https://docs.rs/bech32/latest/src/bech32/lib.rs.html
- 2: https://docs.rs/bech32/latest/bech32/fn.decode.html
- 3: https://docs.rs/bech32/latest/bech32/primitives/decode/index.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the current from_bech32m_string implementation and surrounding code
sed -n '1,220p' packages/rs-dpp/src/address_funds/orchard_address.rs | nl -ba | sed -n '1,220p'
# Narrow to the specific function region (around lines 80-130 and any helper usage)
sed -n '70,150p' packages/rs-dpp/src/address_funds/orchard_address.rs | nl -ba
# Find bech32 usage in this file (and nearby modules, if any)
rg -n "bech32::|bech32m|from_bech32m_string" packages/rs-dpp/src/address_funds/orchard_address.rs
rg -n "bech32" packages/rs-dpp/Cargo.toml packages/rs-dpp/Cargo.lock Cargo.toml 2>/dev/null || true
# If Cargo.lock exists at repo root, print the bech32 version entry
if [ -f Cargo.lock ]; then
rg -n "name = \"bech32\"" -n Cargo.lock -C 2 || true
fi
if [ -f packages/rs-dpp/Cargo.lock ]; then
rg -n "name = \"bech32\"" -n packages/rs-dpp/Cargo.lock -C 2 || true
fiRepository: dashpay/platform
Length of output: 103
🌐 Web query:
bech32 crate version 0.11 bech32::decode accepts bech32 or bech32m strict api
💡 Result:
In bech32 crate 0.11.x, bech32::decode accepts both bech32 and bech32m checksummed strings; i.e., it is not “strict” to only one checksum variant. The crate’s docs explicitly say the input address “MUST include a valid bech32 or bech32m checksum” and that for individual specific checksum algorithms you should use decode_bech32 or decode_bech32m instead. It also notes that the primitives::decode module provides control over “valid checksum for specific checksum algorithm”. So: - bech32::decode: accepts bech32 OR bech32m (auto-detects which checksum is valid), and requires a valid checksum overall [1]. - Strict behavior (only bech32 OR only bech32m): use decode_bech32 or decode_bech32m (and/or the primitives::decode API that checks “valid checksum for specific checksum algorithm”) [1][2]. Sources: [1] and [2].
Citations:
🌐 Web query:
docs.rs bech32 0.11 decode returns variant or detects checksum type
💡 Result:
In bech32 crate 0.11, bech32::decode(s: &str) returns Result<(Hrp, Vec), DecodeError> (not a “variant” that tells you which checksum was used). It will accept either a valid bech32 checksum (BIP-173) or a valid bech32m checksum (BIP-350) and succeeds as long as one of them matches [1][2]. How it detects checksum type: - Internally, decode first constructs an UncheckedHrpstring and then tries to validate the checksum as Bech32m; if that fails with residue that matches a bech32 checksum, it continues, otherwise it returns DecodeError::Checksum [2]. - After it determines that “one of the checksums was valid”, it removes the checksum using remove_checksum:: (the code comments note Bech32m is only used there for its length because Bech32 and Bech32m are the same length; the function doesn’t return which algorithm matched) [2]. So: decode detects whether the input has a valid bech32 or bech32m checksum only to decide whether decoding succeeds (and to remove the checksum), but it does not surface the checksum algorithm as a return “variant”/tag to the caller [1][2]. If you need to explicitly select/identify which checksum algorithm to use, the docs suggest using primitives::decode::CheckedHrpstring / the constructor that allows selecting the checksum algorithm explicitly [1][2].
Citations:
- 1: https://docs.rs/bech32/latest/aarch64-unknown-linux-gnu/bech32/
- 2: https://docs.rs/bech32/latest/src/bech32/lib.rs.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/rs-dpp/src/address_funds/orchard_address.rs"
# Show the from_bech32m_string function body
sed -n '80,140p' "$FILE" | sed -e 's/\t/ /g'
# Show the relevant tests region if present around the cited lines
sed -n '160,260p' "$FILE" | sed -e 's/\t/ /g'
# Report all bech32-related calls in this file
rg -n "bech32|decode_bech32m|decode_bech32|CheckedHrpstring|from_bech32m_string" "$FILE"Repository: dashpay/platform
Length of output: 8177
Enforce Bech32m-only checksum validation in OrchardAddress::from_bech32m_string.
from_bech32m_string calls bech32::decode, which accepts valid Bech32 or Bech32m checksums; since the function then validates only HRP/payload length/type, non-canonical Bech32-checked inputs could still decode successfully. Switch to bech32’s primitives::decode strict API (e.g., CheckedHrpstring/decode_bech32m) to require Bech32m checksum, and add a test that a Bech32-encoded address is rejected.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rs-dpp/src/address_funds/orchard_address.rs` around lines 100 - 102,
OrchardAddress::from_bech32m_string currently uses bech32::decode which accepts
both Bech32 and Bech32m checksums; change it to use the strict Bech32m API
(e.g., bech32::primitives::CheckedHrpString / decode_bech32m or equivalent
decode_bech32m helper) so only Bech32m-checked inputs succeed, map primitive
decode errors into ProtocolError::DecodingError like the current code, and
update/add a unit test ensuring a Bech32 (non-Bech32m) encoded address is
rejected by from_bech32m_string.
| pub fn from_bech32m_string(s: &str) -> Result<Self, ProtocolError> { | ||
| // Decode the bech32m string | ||
| let (hrp, data) = | ||
| bech32::decode(s).map_err(|e| ProtocolError::DecodingError(format!("{}", e)))?; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/rs-dpp/src/address_funds/platform_address.rs"
echo "== File exists =="
ls -la "$FILE"
echo
echo "== from_bech32m_string (around 240-290) =="
nl -ba "$FILE" | sed -n '230,290p'
echo
echo "== is_mainnet_bech32m (around 300-360) =="
nl -ba "$FILE" | sed -n '300,360p'
echo
echo "== Any bech32 decode usage in this file =="
rg -n "bech32::decode|bech32m|decode_check|Checksum" "$FILE" || true
echo
echo "== Search for from_bech32m_string and is_mainnet_bech32m usage across repo =="
rg -n "from_bech32m_string\(|is_mainnet_bech32m\(" -S . || true
echo
echo "== bech32 crate version(s) =="
# Prefer workspace Cargo.toml
ls -ლა Cargo.toml >/dev/null || true
rg -n "bech32\s*=" -S Cargo.toml **/Cargo.toml || true
rg -n "name\s*=\s*\"bech32\"" Cargo.lock || trueRepository: dashpay/platform
Length of output: 288
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="packages/rs-dpp/src/address_funds/platform_address.rs"
echo "== from_bech32m_string (approx 240-290) =="
awk '{printf "%d\t%s\n", NR, $0}' "$FILE" | sed -n '230,290p'
echo
echo "== is_mainnet_bech32m (approx 300-360) =="
awk '{printf "%d\t%s\n", NR, $0}' "$FILE" | sed -n '300,360p'
echo
echo "== Any bech32/bech32m decode usage in this file =="
rg -n "bech32::decode|bech32m|decode_check|Checksum|checksum" "$FILE" || true
echo
echo "== Search for checksum-specific decode API in this file =="
rg -n "decode_check|decode_with|verify|validate.*checksum|ChecksumVariant|Variant" "$FILE" || true
echo
echo "== Find from_bech32m_string definition exactly =="
rg -n "fn\s+from_bech32m_string" "$FILE"
echo
echo "== Find is_mainnet_bech32m definition exactly =="
rg -n "fn\s+is_mainnet_bech32m" "$FILE"
echo
echo "== Usage of is_mainnet_bech32m across repo =="
rg -n "is_mainnet_bech32m\(" -S . || true
echo
echo "== bech32 crate version =="
rg -n "name\s*=\s*\"bech32\"" -S Cargo.toml **/Cargo.toml || true
ls -la Cargo.lock >/dev/null 2>&1 && rg -n "name\s*=\s*\"bech32\"" Cargo.lock || true
rg -n "bech32\s*=" -S Cargo.toml **/Cargo.toml || trueRepository: dashpay/platform
Length of output: 16185
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== bech32 entries in Cargo.lock =="
rg -n "name = \"bech32\"" Cargo.lock | head
# Print nearby lines for first entry
line1=$(rg -n "name = \"bech32\"" Cargo.lock | head -n1 | cut -d: -f1)
start=$((line1-8))
end=$((line1+25))
awk -v s=$start -v e=$end 'NR>=s && NR<=e {print NR ":" $0}' Cargo.lock | sed -n '1,40p'
echo
echo "== Search for bech32::decode::< usage =="
rg -n "bech32::decode::<" -S . || true
rg -n "bech32::decode\s*\::<" -S . || true
echo
echo "== Search for bech32::decode\( usage patterns in rs-dpp =="
rg -n "bech32::decode\(" packages/rs-dpp/src/address_funds/platform_address.rs
echo
echo "== Wallet call site around platform_wallet.rs:1139 =="
FILE2="packages/rs-platform-wallet/src/wallet/platform_wallet.rs"
awk '{print NR "\t" $0}' "$FILE2" | sed -n '1110,1180p'Repository: dashpay/platform
Length of output: 4541
🌐 Web query:
bech32 crate 0.11.1 bech32::decode accepts both checksum variants bech32 and bech32m
💡 Result:
Yes, the bech32 crate version 0.11.1's top-level bech32::decode function accepts and successfully validates strings using either the Bech32 (BIP-173) or Bech32m (BIP-350) checksum variants [1]. According to the official documentation, if the bech32::decode function succeeds, it indicates that the input string is well-formed—consisting of a valid human-readable part (HRP), a separator, and bech32 characters—and possesses a valid checksum under either the Bech32m or Bech32 algorithm [1]. If you require specific control over which checksum algorithm to enforce (e.g., to only allow Bech32m or only Bech32), you should use the primitives::decode::CheckedHrpstring type instead, which allows for explicit selection of the checksum algorithm [1][2].
Citations:
- 1: https://docs.rs/bech32/latest/bech32/fn.decode.html
- 2: https://docs.rs/bech32/0.11.1/x86_64-unknown-linux-gnu/bech32/primitives/decode/index.html
🌐 Web query:
bech32 0.11.1 decode generic parameter how to require Bech32m checksum (example bech32::decode::<Bech32m>)
💡 Result:
In version 0.11.1 of the bech32 crate, the top-level bech32::decode function attempts to validate both Bech32 and Bech32m checksums automatically, rather than allowing a generic parameter to restrict it [1][2]. To explicitly require a Bech32m checksum, you should use the bech32::primitives::decode::CheckedHrpstring type [3][4]. This type's new constructor accepts a generic parameter that implements the necessary checksum trait, allowing you to enforce Bech32m validation specifically [3][2]. Example usage for requiring Bech32m: use bech32::{Bech32m, primitives::decode::CheckedHrpstring}; let s = "your_bech32m_string_here"; // Explicitly require Bech32m checksum validation let p = CheckedHrpstring::new::(s).expect("failed to parse string with Bech32m checksum"); // Access the result let hrp = p.hrp; let data: Vec = p.byte_iter.collect; By using CheckedHrpstring::new::<Bech32m>, the library will reject any input that does not conform to the Bech32m checksum algorithm [4]. If you do not require an allocator, you can iterate over the data directly from the CheckedHrpstring instance using p.byte_iter() [3][5].
Citations:
- 1: https://github.com/rust-bitcoin/rust-bech32/blob/master/src/lib.rs
- 2: https://docs.rs/bech32/latest/src/bech32/lib.rs.html
- 3: https://docs.rs/bech32/^0.11
- 4: https://docs.rs/bech32/latest/bech32/primitives/decode/index.html
- 5: https://docs.rs/bech32/latest/aarch64-unknown-linux-gnu/bech32/
Make PlatformAddress::from_bech32m_string reject plain Bech32 encodings (enforce Bech32m checksum).
PlatformAddress::from_bech32m_string uses bech32::decode(s) (platform_address.rs lines 259-262), and with bech32 v0.11.1 that validator accepts either Bech32 or Bech32m checksum variants—so a Bech32-checked string can pass as long as HRP/payload/type-byte checks succeed. Require Bech32m explicitly (e.g., bech32::primitives::decode::CheckedHrpstring::new::<Bech32m>(s)).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/rs-dpp/src/address_funds/platform_address.rs` around lines 259 -
262, PlatformAddress::from_bech32m_string currently calls bech32::decode(s)
which accepts both Bech32 and Bech32m checksums; change the implementation to
explicitly validate the checksum is Bech32m (rather than any Bech32 variant)
before proceeding. Replace the bech32::decode call in from_bech32m_string with
the bech32 primitives API that enforces Bech32m (e.g., the
CheckedHrpstring/Bech32m-specific decode path) and map any non-Bech32m or decode
failures to ProtocolError::DecodingError so plain Bech32-checked strings are
rejected.
|
✅ DashSDKFFI.xcframework built for this PR.
SwiftPM (host the zip at a stable URL, then use): .binaryTarget(
name: "DashSDKFFI",
url: "https://your.cdn.example/DashSDKFFI.xcframework.zip",
checksum: "7ee5be1d943795694d3ecc9080a3288fd182f7953e0785150b8d2ef22abb95fa"
)Xcode manual integration:
|
Issue being fixed or feature implemented
PlatformAddress::from_bech32m_string/OrchardAddress::from_bech32m_stringreturned(Self, Network), derivingNetworkfrom the bech32m HRP. The HRPtdashis shared by Testnet/Devnet/Regtest (hrp_for_networkis non-injective), so the decoder could not truthfully determine the network and always returnedNetwork::Testnetfortdash.On devnet (paloma) a valid devnet platform address decoded as
Testnet, tripping the wrong-network guard in the wallet'sshielded_unshield_toand blocking unshield-to-transparent.What was done?
Option P — network-agnostic decoders. A
PlatformAddress/OrchardAddresscarries no network internally (network is supplied only atto_bech32m_string(network)encode time), so:from_bech32m_string(s) -> Result<Self, ProtocolError>validates the HRP is a recognized platform HRP (dash/tdash) but makes no network claim.shielded_unshield_to, via a private testable helpercheck_recipient_hrp(recipient, network)that compares the recipient's HRP class against the wallet network (PlatformAddress::hrp_for_network): rejects mainnetdash1…on atdashwallet, accepts devnettdash1…on a devnet wallet, and reports a clear "not a platform address" error for non-platform HRPs (e.g.bc1…).test(dpp): feature-gate signing_tests behind shielded-client— fixes a pre-existingv3.1-devcompile failure wheresigning_testsimportedcrate::shielded::builder(only present undershielded-client), which brokecargo test -p dpp.How Has This Been Tested?
cargo test -p dpp --lib address_funds→ 161 passed (incl.test_bech32m_invalid_hrp_fails)cargo test -p dpp --lib --features shielded-client address_funds::orchard→ 8 passedcargo test -p platform-wallet --features shielded check_recipient_hrp_tests→ 9 passed (mainnet→testnet/devnet REJECT, devnet→devnet ACCEPT, testnet→testnet ACCEPT, tdash cross-acceptance, uppercase ACCEPT,bc1…not-a-platform-address, no-separator/empty Err)cargo check+cargo clippy -- -D warningsclean on dpp, platform-wallet (--features shielded), wasm-dpp2 (changed files)shielded_unshield_to(previously rejected as wrong-network).Breaking Changes
from_bech32m_stringreturn type changes(Self, Network)→Self(hencefix(dpp)!). It is not in any prelude — external consumers reach it only viadpp::address_funds::PlatformAddress/OrchardAddress, so the break is a loud compile error, never silent.Checklist:
For repository code-owners and collaborators only
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Refactor
New Features
Tests