feat: basic support for platform bech32m addresses for dash core and functional tests by knst · Pull Request #7285 · dashpay/dash

knst · 2026-04-21T18:50:23Z

What was done?

It's a step forward to implement changes in dip-0008 with platform addresses and asset-lock transactions v2, see dashpay/dips#182

This PR adds code to parse and encode platform bech32m addresses in C++ code and in functional tests.

Some changes are done as backports:

partial Full BIP173 (Bech32) support bitcoin/bitcoin#11167
merged previously, extra fixes from Tests: tidy up address.py and segwit_addr.py bitcoin/bitcoin#19253
partial BIP 350: Implement Bech32m and use it for v1+ segwit addresses bitcoin/bitcoin#20861

How Has This Been Tested?

See changes in regression tests, in functional tests.

$ test/functional/test_runner.py rpc_help.py
Temporary test directory at /tmp/test_runner_∋_🏃_20260422_014702
Running Unit Tests for Test Framework Modules
....................
----------------------------------------------------------------------
Ran 20 tests in 3.427s
...

Amount of unit tests is increased from 19 to 20 as expected.

Breaking Changes

N/A

This PR doesn't include any breaking changes in RPCs or consensus related changes.
It's an utility PR that could be merged with low risk.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have made corresponding changes to the documentation
I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

github-actions · 2026-04-21T18:50:59Z

✅ No Merge Conflicts Detected

This PR currently has no conflicts with other open PRs.

thepastaclaw · 2026-04-21T18:51:05Z

✅ Review complete (commit 535cb71)

coderabbitai · 2026-04-21T18:55:11Z

Walkthrough

This pull request adds DIP-18 Dash Platform bech32m address support. It adds a bech32_platform_hrp member and accessor to chain parameters (initialized to "dash" for mainnet and "tdash" for testnet/devnet/regtest); new destination types PlatformP2PKHDestination and PlatformP2SHDestination; functions to encode, decode, and validate Platform destinations using bech32m; and unit/functional tests covering valid roundtrips and many invalid cases.

Sequence Diagram(s)

sequenceDiagram
    participant Wallet as Caller
    participant KeyIO as key_io (Encode/Decode)
    participant Params as CChainParams
    participant Bech32 as bech32 lib
    Wallet->>KeyIO: EncodePlatformDestination(dest)
    KeyIO->>Params: Bech32PlatformHRP()
    Params-->>KeyIO: hrp ("dash"/"tdash")
    KeyIO->>Bech32: Convert payload to 5-bit + bech32::Encode(encoding=BECH32M, hrp, data)
    Bech32-->>KeyIO: bech32m string
    KeyIO-->>Wallet: address string

    Wallet->>KeyIO: DecodePlatformDestination(str, params)
    KeyIO->>Bech32: bech32::Decode(str)
    Bech32-->>KeyIO: (encoding, hrp, data) or error
    alt decode success
        KeyIO->>Params: Bech32PlatformHRP()
        Params-->>KeyIO: hrp
        KeyIO->>KeyIO: Validate encoding==BECH32M, hrp match, ConvertBits(5->8), length==21, type byte known
        KeyIO-->>Wallet: PlatformDestination (P2PKH or P2SH) or CNoDestination + error_str
    else decode failure
        KeyIO-->>Wallet: CNoDestination + error_str
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 44.12% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely summarizes the main change: adding basic support for platform bech32m addresses in Dash Core and functional tests, directly matching the PR's core objective.
Description check	✅ Passed	The description clearly explains what was done (adding platform bech32m address parsing/encoding per DIP-0008), how it was tested (unit/functional tests with specific example output), references relevant upstream backports, and confirms no breaking changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

thepastaclaw

Code Review

Three valid low-severity observations on a clean DIP-18 Platform address implementation: a minor dead-copy before type-byte validation, an asymmetric encode API (no CChainParams overload), and thin Python-side test coverage compared to the comprehensive C++ negative cases. No blocking issues.

Reviewed commit: 3130ed6

🟡 1 suggestion(s) | 💬 2 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 `src/key_io.h`:
- [SUGGESTION] lines 56-60: EncodePlatformDestination lacks a CChainParams-injecting overload
  DecodePlatformDestination is exposed with both a `(str, params, error_str)` and a global-Params form, but EncodePlatformDestination only exists in the form that reads the global `Params()` (see src/key_io.cpp:223-226). That asymmetry makes it harder to encode Platform addresses from contexts that already thread a non-global CChainParams (future multi-chain unit tests, RPC helpers, wallet code). Adding a `EncodePlatformDestination(const PlatformDestination&, const CChainParams&)` overload and forwarding the existing free function to it mirrors the base58 Encode/Decode factoring and lets the encoder be tested without touching global state.

thepastaclaw · 2026-04-23T12:07:53Z

+bool IsValidPlatformDestination(const PlatformDestination& dest);
+std::string EncodePlatformDestination(const PlatformDestination& dest);
+PlatformDestination DecodePlatformDestination(const std::string& str);
+PlatformDestination DecodePlatformDestination(const std::string& str, std::string& error_str);
+PlatformDestination DecodePlatformDestination(const std::string& str, const CChainParams& params, std::string& error_str);


🟡 Suggestion: EncodePlatformDestination lacks a CChainParams-injecting overload

DecodePlatformDestination is exposed with both a (str, params, error_str) and a global-Params form, but EncodePlatformDestination only exists in the form that reads the global Params() (see src/key_io.cpp:223-226). That asymmetry makes it harder to encode Platform addresses from contexts that already thread a non-global CChainParams (future multi-chain unit tests, RPC helpers, wallet code). Adding a EncodePlatformDestination(const PlatformDestination&, const CChainParams&) overload and forwarding the existing free function to it mirrors the base58 Encode/Decode factoring and lets the encoder be tested without touching global state.

_{source: ['claude']}

🤖 Fix this 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 `src/key_io.h`: - [SUGGESTION] lines 56-60: EncodePlatformDestination lacks a CChainParams-injecting overload DecodePlatformDestination is exposed with both a `(str, params, error_str)` and a global-Params form, but EncodePlatformDestination only exists in the form that reads the global `Params()` (see src/key_io.cpp:223-226). That asymmetry makes it harder to encode Platform addresses from contexts that already thread a non-global CChainParams (future multi-chain unit tests, RPC helpers, wallet code). Adding a `EncodePlatformDestination(const PlatformDestination&, const CChainParams&)` overload and forwarding the existing free function to it mirrors the base58 Encode/Decode factoring and lets the encoder be tested without touching global state.

thepastaclaw · 2026-04-23T12:07:53Z

+class TestFrameworkScript(unittest.TestCase):
+    def test_platform_encode_decode(self):
+        def test_platform_roundtrip(hrp, addr, expected_type):
+            typ, payload = decode_platform_address(hrp, addr)
+            self.assertIsNotNone(typ)
+            self.assertEqual(typ, expected_type)
+            if expected_type == DIP18_TYPE_P2PKH:
+                self.assertEqual(encode_platform_p2pkh(hrp, payload), addr)
+            else:
+                self.assertEqual(encode_platform_p2sh(hrp, payload), addr)
+
+        # DIP-18 P2PKH
+        test_platform_roundtrip('dash', 'dash1krma5z3ttj75la4m93xcndna9ullamq9y5e9n5rs', DIP18_TYPE_P2PKH)
+        test_platform_roundtrip('tdash', 'tdash1krma5z3ttj75la4m93xcndna9ullamq9y5fzq2j7', DIP18_TYPE_P2PKH)
+        # DIP-18 P2SH
+        test_platform_roundtrip('dash', 'dash1sppl5xpu70aka8nacc4kj2htflydspzkxch4cad6', DIP18_TYPE_P2SH)
+        test_platform_roundtrip('tdash', 'tdash1sppl5xpu70aka8nacc4kj2htflydspzkxc8jtru5', DIP18_TYPE_P2SH)


💬 Nitpick: Python DIP-18 helper only exercises the happy path

TestFrameworkScript::test_platform_encode_decode only asserts round-trip success on four valid vectors. The C++ unit test covers wrong HRP for network, BIP-173 bech32 checksum on a DIP-18 payload, unknown type byte, wrong payload length, and mixed case, none of which are exercised here. Since decode_platform_address is the helper functional tests will use when composing DIP-18 addresses with asset-lock v2 flows, a couple of explicit negative-case assertions would prevent silent regressions.

_{source: ['claude']}

8213838 [Qt] tolerate BIP173/bech32 addresses during input validation (Jonas Schnelli) 06eaca6 [RPC] Wallet: test importing of native witness scripts (NicolasDorier) fd0041a Use BIP173 addresses in segwit.py test (Pieter Wuille) e278f12 Support BIP173 in addwitnessaddress (Pieter Wuille) c091b99 Implement BIP173 addresses and tests (Pieter Wuille) bd355b8 Add regtest testing to base58_tests (Pieter Wuille) 6565c55 Convert base58_tests from type/payload to scriptPubKey comparison (Pieter Wuille) 8fd2267 Import Bech32 C++ reference code & tests (Pieter Wuille) 1e46ebd Implement {Encode,Decode}Destination without CBitcoinAddress (Pieter Wuille) Pull request description: Builds on top of bitcoin#11117. This adds support for: * Creating BIP173 addresses for testing (through `addwitnessaddress`, though by default it still produces P2SH versions) * Sending to BIP173 addresses (including non-v0 ones) * Analysing BIP173 addresses (through `validateaddress`) It includes a reformatted version of the [C++ Bech32 reference code](https://github.com/sipa/bech32/tree/master/ref/c%2B%2B) and an independent implementation of the address encoding/decoding logic (integrated with CTxDestination). All BIP173 test vectors are included. Not included (and intended for other PRs): * Full wallet support for SegWit (which would include automatically adding witness scripts to the wallet during automatic keypool topup, SegWit change outputs, ...) [see bitcoin#11403] * Splitting base58.cpp and tests/base58_tests.cpp up into base58-specific code, and "address encoding"-code [see bitcoin#11372] * Error locating in UI for BIP173 addresses. Tree-SHA512: 238031185fd07f3ac873c586043970cc2db91bf7735c3c168cb33a3db39a7bda81d4891b649685bb17ef90dc63af0328e7705d8cd3e8dafd6c4d3c08fb230341 Co-authored-by: Wladimir J. van der Laan <laanwj@gmail.com>

BACKPORT NOTE: missing changes for segwit_address.py and enabling this unit test 825fcae [tests] Replace bytes literals with hex literals (John Newbery) 64eca45 [tests] Fix pep8 style violations in address.py (John Newbery) b230f8b [tests] Correct docstring for address.py (John Newbery) ea70e6a [tests] Tidy up imports in address.py (John Newbery) 7f639df [tests] Remove unused optional verify_checksum parameter (John Newbery) 011e784 [tests] Rename segwit encode and decode functions (John Newbery) e455713 [tests] Move bech32 unit tests to test framework (John Newbery) Pull request description: Lots of small fixes: - moving unit tests to test_framework implementation files - renaming functions to be clearer - removing multiple imports - removing unreadable byte literals from the code - fixing pep8 violations - correcting out-of-date docstring ACKs for top commit: jonatack: re-ACK 825fcae per `git range-diff a0a422c 7edcdcd 825fcae` and verified `wallet_address_types.py` and `wallet_basic.py --descriptors` (the failure on one travis job) are green locally. MarcoFalke: ACK 825fcae fanquake: ACK 825fcae - looks ok to me. Tree-SHA512: aea509c27c1bcb94bef11205b6a79836c39c62249672815efc9822f411bc2e2336ceb3d72b3b861c3f4054a08e16edb28c6edd3aa5eff72eec1d60ea6ca82dc4 Co-authored-by: MarcoFalke <falke.marco@gmail.com>

…r v1+ segwit addresses BACKPORT NOTE: only test/functional/test_framework/segwit_addr.py changes 0334602 naming nits (Fabian Jahr) 2e7c80f Add signet support to gen_key_io_test_vectors.py (Pieter Wuille) fe5e495 Use Bech32m encoding for v1+ segwit addresses (Pieter Wuille) 25b1c6e Add Bech32m test vectors (Pieter Wuille) da2bb69 Implement Bech32m encoding/decoding (Pieter Wuille) Pull request description: This implements [BIP 350](https://github.com/bitcoin/bips/blob/master/bip-0350.mediawiki): * For segwit v1+ addresses, a new checksum algorithm called Bech32m is used. * Segwit v0 address keep using Bech32 as specified in [BIP 173](https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki). ACKs for top commit: Sjors: utACK 0334602 jnewbery: utACK 0334602 achow101: ACK 0334602 fjahr: re-ACK 0334602 benthecarman: ACK 0334602 Tree-SHA512: 4424cfd44869d813d6152fb3ed867b204036736bc2344a039b93700b6f36a43e9110478f138eb81c97c77ab27ecb776dada5ba632cb5a3a9d244924d2540a557 Co-authored-by: Wladimir J. van der Laan <laanwj@protonmail.com>

coderabbitai

🧹 Nitpick comments (1)

test/functional/test_framework/segwit_addr.py (1)
102-103: Rename constants to match C++ source to avoid confusion.

C++ uses DIP18_TYPE_BYTE_P2PKH / DIP18_TYPE_BYTE_P2SH (see src/key_io.cpp:182-184). The Python helpers drop the _BYTE_ infix. Since these cross-reference each other in reviews and future DIP-18 code, aligning names reduces friction. Optional.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@test/functional/test_framework/segwit_addr.py` around lines 102 - 103, Rename
the two constants DIP18_TYPE_P2PKH and DIP18_TYPE_P2SH to match the C++ names
DIP18_TYPE_BYTE_P2PKH and DIP18_TYPE_BYTE_P2SH; update their definitions and
replace all usages in this module (e.g., any references inside encode/decode
helpers or tests within segwit_addr.py) to the new names so nothing breaks;
ensure imports or external references in other files are updated to the new
constant names if they reference these symbols.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@test/functional/test_framework/segwit_addr.py`:
- Around line 102-103: Rename the two constants DIP18_TYPE_P2PKH and
DIP18_TYPE_P2SH to match the C++ names DIP18_TYPE_BYTE_P2PKH and
DIP18_TYPE_BYTE_P2SH; update their definitions and replace all usages in this
module (e.g., any references inside encode/decode helpers or tests within
segwit_addr.py) to the new names so nothing breaks; ensure imports or external
references in other files are updated to the new constant names if they
reference these symbols.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 47b5478c-633e-4f3a-b16f-74a654214801

📥 Commits

Reviewing files that changed from the base of the PR and between ce784e3 and 535cb71.

📒 Files selected for processing (2)

test/functional/test_framework/segwit_addr.py
test/functional/test_runner.py

🚧 Files skipped from review as they are similar to previous changes (1)

test/functional/test_runner.py

thepastaclaw

Code Review

I reviewed this as a genuinely scoped address-format PR and did not find a concrete correctness regression in the checked-out change. The implementation consistently adds Platform bech32m HRPs to chain params, encodes/decodes the new Platform destination types with bech32m only, and includes both unit and functional coverage for valid roundtrips and malformed inputs.

Reviewed commit: 535cb71

knst added 2 commits April 22, 2026 00:38

feat: add utils to parse platform bech32m destinations

51ce91f

test: regression tests for platform addresses

c3261a9

knst added this to the 24 milestone Apr 21, 2026

knst requested review from PastaPastaPasta and UdjinM6 April 22, 2026 10:54

thepastaclaw reviewed Apr 23, 2026

View reviewed changes

knst and others added 4 commits April 23, 2026 22:39

test: dashify segwit_addr.py and remove bitcoin's specific code

535cb71

knst force-pushed the bech-platform-addresses branch from ce784e3 to 535cb71 Compare April 23, 2026 15:42

coderabbitai Bot reviewed Apr 23, 2026

View reviewed changes

thepastaclaw reviewed Apr 24, 2026

View reviewed changes

knst mentioned this pull request Apr 27, 2026

feat: asset lock tx v2 with basic RPC support + tests #7294

Draft

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: basic support for platform bech32m addresses for dash core and functional tests#7285

feat: basic support for platform bech32m addresses for dash core and functional tests#7285
knst wants to merge 6 commits intodashpay:developfrom
knst:bech-platform-addresses

knst commented Apr 21, 2026

Uh oh!

github-actions Bot commented Apr 21, 2026 •

edited

Loading

Uh oh!

thepastaclaw commented Apr 21, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

thepastaclaw left a comment

Uh oh!

Uh oh!

thepastaclaw Apr 23, 2026

Uh oh!

thepastaclaw Apr 23, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

thepastaclaw left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

knst commented Apr 21, 2026

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Uh oh!

github-actions Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ No Merge Conflicts Detected

Uh oh!

thepastaclaw commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Sequence Diagram(s)

Estimated code review effort

❌ Failed checks (1 warning)

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

thepastaclaw Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

thepastaclaw Apr 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

thepastaclaw left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Apr 21, 2026 •

edited

Loading

thepastaclaw commented Apr 21, 2026 •

edited

Loading

coderabbitai Bot commented Apr 21, 2026 •

edited

Loading