Skip to content

docs: v2 asset locks - automatic funding + bech32 addresses#182

Open
knst wants to merge 2 commits into
dashpay:masterfrom
knst:dip-0027-bech32m
Open

docs: v2 asset locks - automatic funding + bech32 addresses#182
knst wants to merge 2 commits into
dashpay:masterfrom
knst:dip-0027-bech32m

Conversation

@knst
Copy link
Copy Markdown
Collaborator

@knst knst commented Apr 21, 2026

Summary by CodeRabbit

  • Documentation
    • Added Asset Lock Transaction Version 2 spec: Platform automatically credits outputs to Identities (no Registration/Top-Up required).
    • Expanded valid credit output script/address formats for version 2 and added an explicit address-encoding requirement referencing DIP-0018.
    • Documented v24 hard fork activation supporting both Asset Lock Transaction versions 1 and 2.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 21, 2026

📝 Walkthrough

Walkthrough

Updates the DIP-0027 specification to introduce version 2 Asset Lock transactions featuring automatic credit assignment. Defines supported scriptPubKey forms, specifies v24 hard fork activation, and establishes address-encoding requirements with reference to DIP-0018.

Changes

DIP-0027 Specification Update

Layer / File(s) Summary
TOC
dip-0027.md
Adds a table-of-contents entry for "Version 2: Automatic Credit Assignment".
Previous Work
dip-0027.md
Adds DIP-0018: Dash Platform Payment Address Encodings to Previous Work.
Version 2: Automatic Credit Assignment
dip-0027.md
Adds a subsection defining version=2 Asset Lock behavior: Platform must automatically credit each credit_outputs entry to the Identity matching the entry’s pubkey/script hash; allows credit_outputs[i].scriptPubKey ∈ {P2PKH, P2SH}; mandates DIP-0018 address encoding; specifies v24 activation enabling version ∈ {1,2}.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main changes: documenting v2 asset locks with automatic funding and addressing bech32 addresses, which aligns with the updated DIP-0027 specification.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dip-0027.md`:
- Line 91: Update the DIP text to avoid ambiguity by replacing references to
"pubkey" with "pubkey hash" where appropriate: specifically clarify that in a
`version = 2` Asset Lock transaction each entry in `credit_outputs` MUST be
credited to the Identity matching the entry's pubkey hash (not a raw pubkey) or
script hash, and update any related wording that mentions `pubkey` to `pubkey
hash` to reflect P2PKH locking scripts.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: cca78d3d-d9e5-460f-861a-463449b01a03

📥 Commits

Reviewing files that changed from the base of the PR and between a4d46dd and 87cb45c.

📒 Files selected for processing (1)
  • dip-0027.md

Comment thread dip-0027.md Outdated
Comment thread dip-0027.md Outdated
Copy link
Copy Markdown
Collaborator

@thephez thephez left a comment

Choose a reason for hiding this comment

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

We might need some more changes, but check these for now

Comment thread dip-0027.md
Comment thread dip-0027.md Outdated

#### Version 2: Automatic Credit Assignment

A `version = 2` Asset Lock transaction signals that each entry in `credit_outputs` MUST be automatically credited by Platform to the Identity matching the entry's public key hash or script hash, without an accompanying Identity Registration or Top-Up state transition.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Maybe something like this. @PastaPastaPasta what do you think?

Suggested change
A `version = 2` Asset Lock transaction signals that each entry in `credit_outputs` MUST be automatically credited by Platform to the Identity matching the entry's public key hash or script hash, without an accompanying Identity Registration or Top-Up state transition.
When Dash Platform implemented the Platform address system defined by [DIP-17](https://github.com/dashpay/dips/blob/master/dip-0017.md), there was no way to directly fund a Platform address from the Core chain. Version 2 Asset Lock transactions enable transferring Core chain funds to Platform addresses by signalling that each entry in `credit_outputs` MUST be automatically credited to the Platform address matching the entry's public key hash or script hash. No accompanying state transition is required.

Comment thread dip-0027.md Outdated
Co-authored-by: thephez <thephez@users.noreply.github.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
dip-0027.md (1)

92-92: Clarify the relationship between Platform addresses and Identity crediting.

A past reviewer noted that "platform addresses aren't linked to particular identities" and asked for clarification. While the current wording states credits are "automatically credited to the Platform address," it would be helpful to explicitly clarify:

  • Are credits held by the Platform address itself (independent of any Identity)?
  • Or are credits automatically assigned to an Identity derived from or associated with that address?

This clarification would help implementers understand the precise semantics of the automatic credit assignment.

Could you confirm whether the DIP-0017 Platform address system allows addresses to hold credits independently, or if there's an implicit Identity association? Consider adding a brief clarification in the text if the current wording might be ambiguous.

🤖 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 `@dip-0027.md` at line 92, Clarify that Version 2 Asset Lock credits in the
`credit_outputs` are held by the Platform address itself (matched by the entry's
public key hash or script hash) and are not implicitly bound to any Identity
unless an explicit identity creation or association operation occurs; update the
paragraph mentioning "automatically credited to the Platform address" to state
that these credits are stored at the Platform address level and only become
associated with an Identity when an explicit Identity is derived from or linked
to that address (reference DIP-17, `credit_outputs`, public key hash, script
hash, and Identity in the text).
🤖 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.

Nitpick comments:
In `@dip-0027.md`:
- Line 92: Clarify that Version 2 Asset Lock credits in the `credit_outputs` are
held by the Platform address itself (matched by the entry's public key hash or
script hash) and are not implicitly bound to any Identity unless an explicit
identity creation or association operation occurs; update the paragraph
mentioning "automatically credited to the Platform address" to state that these
credits are stored at the Platform address level and only become associated with
an Identity when an explicit Identity is derived from or linked to that address
(reference DIP-17, `credit_outputs`, public key hash, script hash, and Identity
in the text).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: abf71d68-067c-4cc0-8c0e-d6c3fbaf20df

📥 Commits

Reviewing files that changed from the base of the PR and between 87cb45c and 9ac939d.

📒 Files selected for processing (1)
  • dip-0027.md

PastaPastaPasta added a commit to dashpay/dash that referenced this pull request May 12, 2026
…ash core and functional tests

535cb71 test: dashify segwit_addr.py and remove bitcoin's specific code (Konstantin Akimov)
89ec176 partial Merge bitcoin#20861: BIP 350: Implement Bech32m and use it for v1+ segwit addresses (Konstantin Akimov)
7c7e722 Merge bitcoin#19253: Tests: tidy up address.py and segwit_addr.py (Konstantin Akimov)
393ca5e partial Merge bitcoin#11167: Full BIP173 (Bech32) support (Konstantin Akimov)
c3261a9 test: regression tests for platform addresses (Konstantin Akimov)
51ce91f feat: add utils to parse platform bech32m destinations (Konstantin Akimov)

Pull request description:

  ## 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 bitcoin#11167
   - merged previously, extra fixes from bitcoin#19253
   - partial 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:
  - [x] 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
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 535cb71

Tree-SHA512: 38e4a924bb0ea3ac7ff99cefead131a8d1fd806585da86e356e1836c6792c73a1481afcb572e89eb1e4eff9448b234ea4a182f5127f3f4ce9b9b2c1a05a5cf5c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants