fix(swift-sdk): decode DIP-0018 platform addresses in address queries#4021
Conversation
The Get Address Info query form (and the rest of the Swift address
query/display layer) could not query bech32m platform addresses. Two
stacked client-side bugs:
1. Wrong HRP. Detection and bech32m routing keyed off `dashevo`/`tdashevo`,
but the canonical DIP-0018 HRP (rs-dpp, and the wallet's own persistence
handler) is `dash`/`tdash`. Real `tdash1...` addresses failed every
prefix check and fell through to hex → "Invalid hex string for address";
the format chip read "Unknown format".
2. Wrong type byte. Once routing was fixed, the FFI rejected the bytes with
`UnexpectedVariant { allowed: 0..1, found: 176 }`. Platform addresses
have two 21-byte encodings: the bech32m payload uses the user-facing type
byte (0xb0 P2PKH / 0x80 P2SH) while the FFI (bincode `PlatformAddress`)
uses the storage variant byte (0x00 / 0x01). The Swift codec treated the
bytes opaquely and never translated, breaking both query decode and
result display.
Centralize the fix in `Bech32m`: correct HRP constants, a
`looksLikePlatformAddress` detector, and two-way
`storageBytes(fromBech32mPayload:)` / `bech32mPayload(fromStorageBytes:)`
mapping. Wire through `getInfo`/`getInfos`, all `toBech32m` encoders,
`DataTransformers`, `Validation`, and the query form. The hex test field
feeds the FFI directly, so its vector uses the storage form (`00...`).
Verified on the simulator against a fresh testnet wallet: `tdash1kzdl4c3...`
→ Valid bech32m / 21 bytes → balance 295,643,280 credits, nonce 2, Found,
and the result round-trips back to the canonical `tdash1...` form.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 40 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
✅ Review complete (commit a0c6978) |
There was a problem hiding this comment.
Code Review
The HRP/type-byte fix itself is correct and matches rs-dpp's PlatformAddress. Two existing unit tests in ValidationTests.swift were not updated for the new HRP and will now fail, which is CI-breaking. Additional consistency gaps: isValidPlatformAddress doesn't verify the DIP-0018 type byte, the bech32m fetch APIs accept non-dash/tdash HRPs, and one transformer test silently degrades under the new HRP.
Source: reviewers claude:opus[general], codex:gpt-5.5[general]; verifier claude:opus; specialists none.
🔴 1 blocking | 🟡 3 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/swift-sdk/SwiftTests/SwiftDashSDKTests/ValidationTests.swift`:
- [BLOCKING] packages/swift-sdk/SwiftTests/SwiftDashSDKTests/ValidationTests.swift:78-107: Validation tests still assert legacy `tdashevo` HRP is valid
`testValidateBech32mAddress_valid` (line 81) and `testValidateAddress_autoDetect` (line 102) both assert `AddressValidator.validateBech32mAddress("tdashevo1qz4242…")` returns `true`. After this PR, `Bech32m.isValidPlatformAddress` only accepts `hrp == "dash"` or `hrp == "tdash"`, so `tdashevo` (the HRP parsed as everything before the final `1`) no longer validates. `looksLikePlatformAddress` also returns false because the literal doesn't start with `dash1` or `tdash1`, so the auto-detect test falls through to hex validation and also fails. Both `XCTAssertTrue` assertions will now flip and break the SwiftDashSDKTests target on CI. Replace the fixtures with real DIP-0018 addresses (e.g. `tdash1kzdl4c3apkekqevkqrzctgagv2v2ng5hysegt5x4`, matching the PR description).
Source: ['claude', 'codex']
In `packages/swift-sdk/Sources/SwiftDashSDK/Address/PlatformAddressInfo.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/Address/PlatformAddressInfo.swift:434-442: `isValidPlatformAddress` doesn't check the DIP-0018 type byte
The validator now checks the canonical HRP and the 21-byte length, but does not verify that the payload's leading byte is one of the DIP-0018 Platform type bytes (`0xb0` P2PKH or `0x80` P2SH). That leaves the public validator (and any UI chip built on it) reporting `true` for well-formed bech32m strings whose first byte then causes `storageBytes(fromBech32mPayload:)` and the FFI fetch path to immediately reject the address — a mismatch this PR otherwise closes. Reuse `storageBytes(fromBech32mPayload:)` to gate on both length and type byte.
Source: ['codex']
In `packages/swift-sdk/Sources/SwiftDashSDK/Address/Addresses.swift`:
- [SUGGESTION] packages/swift-sdk/Sources/SwiftDashSDK/Address/Addresses.swift:77-86: Bech32m fetch APIs accept non-`dash`/`tdash` HRPs
`getInfo(bech32mAddress:)` decodes any bech32m string and only validates the 21-byte payload / type byte before querying the FFI. A caller passing a bech32m string with an arbitrary HRP but a coincidentally-valid `[0xb0|0x80] || 20-byte hash` payload is accepted and queried as a Platform address, even though the Rust side (`PlatformAddress::from_bech32m_string`) and `AddressTransformer.parseBech32mAddress` both reject non-`dash`/`tdash` HRPs. Apply the same HRP check here and in `getInfos(bech32mAddresses:)` at line 222 for API consistency with the rest of this PR.
Source: ['codex']
In `packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DataTransformersTests.swift`:
- [SUGGESTION] packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DataTransformersTests.swift:87-93: `testParseAddressBech32m` silently degrades to a nil-check after HRP change
The fixture `tdashevo1qz4242…` no longer decodes as a Platform address, so `AddressTransformer.parseAddress` returns nil. The assertion `data == nil || data?.count == 21` is satisfied by the nil branch, so the test now provides no coverage of the bech32m path while appearing to pass. Replace the fixture with a real `tdash1…` DIP-0018 address and tighten to `XCTAssertEqual(data?.count, 21)` so the new HRP + storage-byte round-trip is actually exercised.
Source: ['claude']
In `packages/swift-sdk/Sources/SwiftDashSDK/Address/PlatformAddressInfo.swift`:
- [NITPICK] packages/swift-sdk/Sources/SwiftDashSDK/Address/PlatformAddressInfo.swift:389-389: Doc example for `Bech32m.decode` still shows deprecated `tdashevo` HRP
The rest of this PR replaces `dashevo`/`tdashevo` with `dash`/`tdash` everywhere, but the doc-comment example on `Bech32m.decode` still reads `tdashevo1qqyfsqyzcn5hzu7echru54njypdq0v4d7gv8pkdf`. Flip it to a `tdash1…` sample to avoid re-introducing confusion in future work.
Source: ['claude']
| guard let result = decode(address) else { | ||
| return false | ||
| } | ||
| // Valid Platform addresses have dashevo or tdashevo HRP and 21 bytes of data | ||
| let validHrp = result.hrp == "dashevo" || result.hrp == "tdashevo" | ||
| // Valid Platform addresses have the DIP-0018 dash/tdash HRP and 21 bytes of data | ||
| let validHrp = result.hrp == platformHrpMainnet || result.hrp == platformHrpTestnet | ||
| let validLength = result.data.count == 21 | ||
| return validHrp && validLength | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: isValidPlatformAddress doesn't check the DIP-0018 type byte
The validator now checks the canonical HRP and the 21-byte length, but does not verify that the payload's leading byte is one of the DIP-0018 Platform type bytes (0xb0 P2PKH or 0x80 P2SH). That leaves the public validator (and any UI chip built on it) reporting true for well-formed bech32m strings whose first byte then causes storageBytes(fromBech32mPayload:) and the FFI fetch path to immediately reject the address — a mismatch this PR otherwise closes. Reuse storageBytes(fromBech32mPayload:) to gate on both length and type byte.
| guard let result = decode(address) else { | |
| return false | |
| } | |
| // Valid Platform addresses have dashevo or tdashevo HRP and 21 bytes of data | |
| let validHrp = result.hrp == "dashevo" || result.hrp == "tdashevo" | |
| // Valid Platform addresses have the DIP-0018 dash/tdash HRP and 21 bytes of data | |
| let validHrp = result.hrp == platformHrpMainnet || result.hrp == platformHrpTestnet | |
| let validLength = result.data.count == 21 | |
| return validHrp && validLength | |
| } | |
| public static func isValidPlatformAddress(_ address: String) -> Bool { | |
| guard let result = decode(address) else { | |
| return false | |
| } | |
| // Valid Platform addresses have the DIP-0018 dash/tdash HRP and a | |
| // 21-byte payload with a P2PKH (0xb0) or P2SH (0x80) type byte. | |
| let validHrp = result.hrp == platformHrpMainnet || result.hrp == platformHrpTestnet | |
| return validHrp && storageBytes(fromBech32mPayload: result.data) != nil | |
| } |
source: ['codex']
| public func getInfo(bech32mAddress: String) throws -> PlatformAddressInfo? { | ||
| guard let decoded = Bech32m.decode(bech32mAddress) else { | ||
| throw SDKError.invalidParameter("Invalid bech32m address") | ||
| } | ||
| guard decoded.data.count == 21 else { | ||
| throw SDKError.invalidParameter("Invalid Platform address: expected 21 bytes, got \(decoded.data.count)") | ||
| guard let storageBytes = Bech32m.storageBytes(fromBech32mPayload: decoded.data) else { | ||
| throw SDKError.invalidParameter( | ||
| "Invalid Platform address: expected 21 bytes with a P2PKH (0xb0) or P2SH (0x80) type byte, got \(decoded.data.count) bytes") | ||
| } | ||
| return try getInfo(addressBytes: decoded.data) | ||
| return try getInfo(addressBytes: storageBytes) | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: Bech32m fetch APIs accept non-dash/tdash HRPs
getInfo(bech32mAddress:) decodes any bech32m string and only validates the 21-byte payload / type byte before querying the FFI. A caller passing a bech32m string with an arbitrary HRP but a coincidentally-valid [0xb0|0x80] || 20-byte hash payload is accepted and queried as a Platform address, even though the Rust side (PlatformAddress::from_bech32m_string) and AddressTransformer.parseBech32mAddress both reject non-dash/tdash HRPs. Apply the same HRP check here and in getInfos(bech32mAddresses:) at line 222 for API consistency with the rest of this PR.
| public func getInfo(bech32mAddress: String) throws -> PlatformAddressInfo? { | |
| guard let decoded = Bech32m.decode(bech32mAddress) else { | |
| throw SDKError.invalidParameter("Invalid bech32m address") | |
| } | |
| guard decoded.data.count == 21 else { | |
| throw SDKError.invalidParameter("Invalid Platform address: expected 21 bytes, got \(decoded.data.count)") | |
| guard let storageBytes = Bech32m.storageBytes(fromBech32mPayload: decoded.data) else { | |
| throw SDKError.invalidParameter( | |
| "Invalid Platform address: expected 21 bytes with a P2PKH (0xb0) or P2SH (0x80) type byte, got \(decoded.data.count) bytes") | |
| } | |
| return try getInfo(addressBytes: decoded.data) | |
| return try getInfo(addressBytes: storageBytes) | |
| } | |
| public func getInfo(bech32mAddress: String) throws -> PlatformAddressInfo? { | |
| guard let decoded = Bech32m.decode(bech32mAddress), | |
| decoded.hrp == Bech32m.platformHrpMainnet || decoded.hrp == Bech32m.platformHrpTestnet else { | |
| throw SDKError.invalidParameter("Invalid bech32m Platform address") | |
| } | |
| guard let storageBytes = Bech32m.storageBytes(fromBech32mPayload: decoded.data) else { | |
| throw SDKError.invalidParameter( | |
| "Invalid Platform address: expected 21 bytes with a P2PKH (0xb0) or P2SH (0x80) type byte, got \(decoded.data.count) bytes") | |
| } | |
| return try getInfo(addressBytes: storageBytes) | |
| } |
source: ['codex']
2 participants
Issue being fixed or feature implemented
The Get Address Info query form (Settings → Queries → Addresses → Get Address Info) could not query bech32m platform addresses — it always fell through to hex parsing and returned "Invalid Parameter: Invalid hex string for address". Reproduced on a fresh testnet wallet (iOS 26 sim) with both a real
tdash1kzdl4c3...address and the form's own canned test vector.The underlying FFI query is fine (
dash-sdkreturns the correct balance/nonce/proof for the same address). These were two stacked client-side input-handling bugs in the Swift address query/display layer.Bug 1 — wrong HRP
Format detection and bech32m routing keyed off
dashevo/tdashevo, but the canonical DIP-0018 HRP (perrs-dppplatform_address.rs, and the wallet's ownPlatformWalletPersistenceHandler) isdash/tdash. Realtdash1...addresses failed every prefix check and fell through to hex → "Invalid hex string for address"; the format chip read "Unknown format".Bug 2 — wrong type byte (surfaced once routing was fixed)
The FFI then rejected the bytes with
UnexpectedVariant { allowed: 0..1, found: 176 }. Platform addresses have two 21-byte encodings:0xb0= P2PKH,0x80= P2SHPlatformAddress, whatdash_sdk_address_fetch_infoconsumes and returns):0x00= P2PKH,0x01= P2SHThe Swift
Bech32mcodec treated the 21 bytes opaquely and never translated between them, breaking both the query decode and the result-row display.What was done?
Centralized the fix in the
Bech32mhelper (PlatformAddressInfo.swift):platformHrpMainnet=dash,platformHrpTestnet=tdash) + alooksLikePlatformAddressdetector (HRP-prefix based; a 42-char hex address can never match).storageBytes(fromBech32mPayload:)(0xb0→0x00,0x80→0x01) and its inversebech32mPayload(fromStorageBytes:).Wired through the whole query/display layer:
Addresses.getInfo(bech32mAddress:),getInfos(bech32mAddresses:), and thegetInfo/getInfos(addresses:)auto-detect routing.toBech32m(network:)encoders (now round-trip the canonical DIP-0018 form).DataTransformers(parseBech32mAddress,formatAddress,parseAddress) andValidation.validateAddress.GetAddressInfoViewModel,GetAddressesInfosViewModel,AddressQueriesView): detector, network detection, help text, and canned test vectors. The hex test field feeds the FFI directly, so its vector uses the storage form (00...), while the bech32m vector uses a realtdash1...address.How Has This Been Tested?
Built
SwiftExampleApp(BUILD SUCCEEDED) and ran the exact reported flow end-to-end on the booted iOS simulator against a fresh testnet wallet:tdash1kzdl4c3apkekqevkqrzctgagv2v2ng5hysegt5x4; the format chip now shows ✅ "Valid bech32m address", "Decoded: 21 bytes", hexb09bfae2…(P2PKH type byte0xb0+ 20-byte hash).tdash1kzdl4c3…, confirming the encode direction (storage → bech32m) is correct too.Note: the same fixed helpers back the other address query screens (e.g. Get Addresses Infos), but only the single-address form was exercised end-to-end.
Breaking Changes
None. Client-side only (Swift SDK + example app); no FFI signature or consensus changes.
Checklist: