Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 22 additions & 18 deletions packages/rs-dpp/src/address_funds/orchard_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,24 +87,30 @@ impl OrchardAddress {

/// Decodes a bech32m-encoded Orchard address string.
///
/// An `OrchardAddress` is network-agnostic: the network is supplied only at
/// [`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.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the comment shorter, references to to_bench32m_string() are not needed here, and add reference to is_mainnet_bech32m()

///
/// # Returns
/// - `Ok((OrchardAddress, Network))` - The decoded address and its network
/// - `Err(ProtocolError)` - If the address is invalid
pub fn from_bech32m_string(s: &str) -> Result<(Self, Network), ProtocolError> {
/// - `Ok(OrchardAddress)` - The decoded address
/// - `Err(ProtocolError)` - If the address is invalid or its HRP is not a
/// recognized platform HRP
pub fn from_bech32m_string(s: &str) -> Result<Self, ProtocolError> {
let (hrp, data) =
bech32::decode(s).map_err(|e| ProtocolError::DecodingError(format!("{}", e)))?;
Comment on lines +100 to 102
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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
fi

Repository: 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:


🌐 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:


🏁 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
fi

Repository: 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:


🏁 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.


// Validate the HRP is a recognized platform HRP (case-insensitive). No
// network is derived — the HRP is ambiguous across the tdash-shared
// networks.
let hrp_lower = hrp.as_str().to_ascii_lowercase();
let network = match hrp_lower.as_str() {
s if s == PLATFORM_HRP_MAINNET => Network::Mainnet,
s if s == PLATFORM_HRP_TESTNET => Network::Testnet,
_ => {
return Err(ProtocolError::DecodingError(format!(
"invalid HRP '{}': expected '{}' or '{}'",
hrp, PLATFORM_HRP_MAINNET, PLATFORM_HRP_TESTNET
)))
}
};
if hrp_lower != PLATFORM_HRP_MAINNET && hrp_lower != PLATFORM_HRP_TESTNET {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we use the same validation multiple times. DRY. Maybe just call is_mainnet_bech32m() ?

return Err(ProtocolError::DecodingError(format!(
"invalid HRP '{}': expected '{}' or '{}'",
hrp, PLATFORM_HRP_MAINNET, PLATFORM_HRP_TESTNET
)));
}

// Validate payload: 1 type byte + 11 diversifier + 32 pk_d = 44 bytes
if data.len() != 1 + ORCHARD_ADDRESS_SIZE {
Expand All @@ -125,7 +131,7 @@ impl OrchardAddress {

let mut raw = [0u8; ORCHARD_ADDRESS_SIZE];
raw.copy_from_slice(&data[1..]);
Self::from_raw_bytes(&raw).map(|addr| (addr, network))
Self::from_raw_bytes(&raw)
}
}

Expand Down Expand Up @@ -189,10 +195,9 @@ mod tests {
encoded
);

let (decoded, network) =
let decoded =
OrchardAddress::from_bech32m_string(&encoded).expect("decoding should succeed");
assert_eq!(decoded, address);
assert_eq!(network, Network::Mainnet);
}

#[test]
Expand All @@ -206,10 +211,9 @@ mod tests {
encoded
);

let (decoded, network) =
let decoded =
OrchardAddress::from_bech32m_string(&encoded).expect("decoding should succeed");
assert_eq!(decoded, address);
assert_eq!(network, Network::Testnet);
}

#[test]
Expand Down
168 changes: 134 additions & 34 deletions packages/rs-dpp/src/address_funds/platform_address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,26 +246,31 @@ impl PlatformAddress {
/// 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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make the comment shorter, references to to_bench32m_string() are not needed here, and add reference to 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.
///
/// # Returns
/// - `Ok((PlatformAddress, Network))` - The decoded address and its network
/// - `Err(ProtocolError)` - If the address is invalid
pub fn from_bech32m_string(s: &str) -> Result<(Self, Network), ProtocolError> {
/// - `Ok(PlatformAddress)` - The decoded address
/// - `Err(ProtocolError)` - If the address is invalid or its HRP is not a
/// recognized platform HRP
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)))?;
Comment on lines +259 to 262
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 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 || true

Repository: 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 || true

Repository: 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:


🌐 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:


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.


// Determine network from HRP (case-insensitive per DIP-0018)
// Validate the HRP is a recognized platform HRP (case-insensitive per
// DIP-0018). No network is derived — the HRP is ambiguous across the
// tdash-shared networks.
let hrp_lower = hrp.as_str().to_ascii_lowercase();
let network = match hrp_lower.as_str() {
s if s == PLATFORM_HRP_MAINNET => Network::Mainnet,
s if s == PLATFORM_HRP_TESTNET => Network::Testnet,
_ => {
return Err(ProtocolError::DecodingError(format!(
"invalid HRP '{}': expected '{}' or '{}'",
hrp, PLATFORM_HRP_MAINNET, PLATFORM_HRP_TESTNET
)))
}
};
if hrp_lower != PLATFORM_HRP_MAINNET && hrp_lower != PLATFORM_HRP_TESTNET {
Copy link
Copy Markdown
Contributor

@lklimek lklimek Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we use the same validation multiple times. DRY. Maybe just call is_mainnet_bech32m() ?

return Err(ProtocolError::DecodingError(format!(
"invalid HRP '{}': expected '{}' or '{}'",
hrp, PLATFORM_HRP_MAINNET, PLATFORM_HRP_TESTNET
)));
}

// Validate payload length: 1 type byte + 20 hash bytes = 21 bytes
if data.len() != 1 + ADDRESS_HASH_SIZE {
Expand All @@ -291,7 +296,46 @@ impl PlatformAddress {
))),
}?;

Ok((address, network))
Ok(address)
}

/// Classifies a bech32m platform-address string as mainnet or non-mainnet
/// by its HRP alone, without decoding the payload.
///
/// This is the only truthful network signal an address string carries: per
/// DIP-0018 the prefix `dash` means mainnet and `tdash` means non-mainnet,
/// but `tdash` is shared by Testnet, Devnet, and Regtest and the payload
/// holds no network byte — so the specific non-mainnet network is NOT
/// recoverable from an address string. The HRP is the segment before the
/// final `'1'` separator (bech32's data charset excludes `'1'`); the
/// comparison is case-insensitive since bech32m permits all-uppercase.
///
/// # Returns
/// - `Ok(true)` - mainnet (`dash` HRP)
/// - `Ok(false)` - non-mainnet (`tdash` HRP: Testnet/Devnet/Regtest)
/// - `Err(ProtocolError)` - malformed (no bech32 separator) or a
/// non-platform HRP
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}'"
)))
}
Comment on lines +318 to +338
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

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).

}

/// Converts the PlatformAddress to a dashcore Address with the specified network.
Expand Down Expand Up @@ -683,17 +727,13 @@ impl FromStr for PlatformAddress {
/// Parses a bech32m-encoded Platform address string.
///
/// This accepts addresses with either mainnet ("dash") or testnet ("tdash") HRP.
/// The network information is discarded; use `from_bech32m_string` if you need
/// to preserve the network.
///
/// # Example
/// ```ignore
/// let address: PlatformAddress = "dash1k...".parse()?;
/// ```
fn from_str(s: &str) -> Result<Self, Self::Err> {
Self::from_bech32m_string(s)
.map(|(addr, _network)| addr)
.map_err(|e| PlatformAddressParseError(e.to_string()))
Self::from_bech32m_string(s).map_err(|e| PlatformAddressParseError(e.to_string()))
}
}

Expand Down Expand Up @@ -1132,10 +1172,9 @@ mod tests {
);

// Decode and verify roundtrip
let (decoded, network) =
let decoded =
PlatformAddress::from_bech32m_string(&encoded).expect("decoding should succeed");
assert_eq!(decoded, address);
assert_eq!(network, Network::Mainnet);
}

#[test]
Expand All @@ -1157,10 +1196,9 @@ mod tests {
);

// Decode and verify roundtrip
let (decoded, network) =
let decoded =
PlatformAddress::from_bech32m_string(&encoded).expect("decoding should succeed");
assert_eq!(decoded, address);
assert_eq!(network, Network::Testnet);
}

#[test]
Expand All @@ -1182,10 +1220,9 @@ mod tests {
);

// Decode and verify roundtrip
let (decoded, network) =
let decoded =
PlatformAddress::from_bech32m_string(&encoded).expect("decoding should succeed");
assert_eq!(decoded, address);
assert_eq!(network, Network::Mainnet);
}

#[test]
Expand All @@ -1207,10 +1244,9 @@ mod tests {
);

// Decode and verify roundtrip
let (decoded, network) =
let decoded =
PlatformAddress::from_bech32m_string(&encoded).expect("decoding should succeed");
assert_eq!(decoded, address);
assert_eq!(network, Network::Testnet);
}

#[test]
Expand Down Expand Up @@ -1336,8 +1372,8 @@ mod tests {
let uppercase = lowercase.to_uppercase();

// Both should decode to the same address
let (decoded_lower, _) = PlatformAddress::from_bech32m_string(&lowercase).unwrap();
let (decoded_upper, _) = PlatformAddress::from_bech32m_string(&uppercase).unwrap();
let decoded_lower = PlatformAddress::from_bech32m_string(&lowercase).unwrap();
let decoded_upper = PlatformAddress::from_bech32m_string(&uppercase).unwrap();

assert_eq!(decoded_lower, decoded_upper);
assert_eq!(decoded_lower, address);
Expand All @@ -1348,7 +1384,7 @@ mod tests {
// Edge case: all-zero hash
let address = PlatformAddress::P2pkh([0u8; 20]);
let encoded = address.to_bech32m_string(Network::Mainnet);
let (decoded, _) = PlatformAddress::from_bech32m_string(&encoded).unwrap();
let decoded = PlatformAddress::from_bech32m_string(&encoded).unwrap();
assert_eq!(decoded, address);
}

Expand All @@ -1357,7 +1393,7 @@ mod tests {
// Edge case: all-ones hash
let address = PlatformAddress::P2sh([0xFF; 20]);
let encoded = address.to_bech32m_string(Network::Mainnet);
let (decoded, _) = PlatformAddress::from_bech32m_string(&encoded).unwrap();
let decoded = PlatformAddress::from_bech32m_string(&encoded).unwrap();
assert_eq!(decoded, address);
}

Expand Down Expand Up @@ -1408,10 +1444,74 @@ mod tests {
let p2pkh_encoded = p2pkh.to_bech32m_string(Network::Mainnet);
let p2sh_encoded = p2sh.to_bech32m_string(Network::Mainnet);

let (p2pkh_decoded, _) = PlatformAddress::from_bech32m_string(&p2pkh_encoded).unwrap();
let (p2sh_decoded, _) = PlatformAddress::from_bech32m_string(&p2sh_encoded).unwrap();
let p2pkh_decoded = PlatformAddress::from_bech32m_string(&p2pkh_encoded).unwrap();
let p2sh_decoded = PlatformAddress::from_bech32m_string(&p2sh_encoded).unwrap();

assert_eq!(p2pkh_decoded, p2pkh);
assert_eq!(p2sh_decoded, p2sh);
}

#[test]
fn test_is_mainnet_bech32m_mainnet_is_true() {
let encoded = PlatformAddress::P2pkh([0x11; 20]).to_bech32m_string(Network::Mainnet);
assert!(encoded.starts_with("dash1"));
assert!(PlatformAddress::is_mainnet_bech32m(&encoded).unwrap());
}

#[test]
fn test_is_mainnet_bech32m_all_non_mainnet_networks_are_false() {
// Testnet, Devnet, and Regtest all share the `tdash` HRP, so all three
// classify as non-mainnet (false) — the only truthful answer DIP-0018
// allows from the address string alone.
for network in [Network::Testnet, Network::Devnet, Network::Regtest] {
let encoded = PlatformAddress::P2pkh([0x22; 20]).to_bech32m_string(network);
assert!(encoded.starts_with("tdash1"), "network {network:?}");
assert!(
!PlatformAddress::is_mainnet_bech32m(&encoded).unwrap(),
"network {network:?} must classify as non-mainnet"
);
}
}

#[test]
fn test_is_mainnet_bech32m_is_case_insensitive() {
let mainnet = PlatformAddress::P2pkh([0x33; 20])
.to_bech32m_string(Network::Mainnet)
.to_uppercase();
assert!(mainnet.starts_with("DASH1"));
assert!(PlatformAddress::is_mainnet_bech32m(&mainnet).unwrap());

let testnet = PlatformAddress::P2pkh([0x44; 20])
.to_bech32m_string(Network::Testnet)
.to_uppercase();
assert!(testnet.starts_with("TDASH1"));
assert!(!PlatformAddress::is_mainnet_bech32m(&testnet).unwrap());
}

#[test]
fn test_is_mainnet_bech32m_non_platform_hrp_errors() {
let err = PlatformAddress::is_mainnet_bech32m("bc1qexampledata").unwrap_err();
assert!(
err.to_string().contains("not a platform address"),
"unexpected error: {err}"
);
}

#[test]
fn test_is_mainnet_bech32m_missing_separator_errors() {
let err = PlatformAddress::is_mainnet_bech32m("nodelimiterhere").unwrap_err();
assert!(
err.to_string().contains("missing bech32 separator"),
"unexpected error: {err}"
);
}

#[test]
fn test_is_mainnet_bech32m_empty_errors() {
let err = PlatformAddress::is_mainnet_bech32m("").unwrap_err();
assert!(
err.to_string().contains("missing bech32 separator"),
"unexpected error: {err}"
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ mod proved;
#[cfg(all(
test,
feature = "state-transition-signing",
feature = "core_key_wallet"
feature = "core_key_wallet",
feature = "shielded-client"
))]
mod signing_tests;
mod state_transition_estimated_fee_validation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@
#![cfg(all(
test,
feature = "state-transition-signing",
feature = "core_key_wallet"
feature = "core_key_wallet",
feature = "shielded-client"
))]

use crate::identity::state_transition::asset_lock_proof::chain::ChainAssetLockProof;
Expand Down
Loading
Loading