feat(dpp)!: dashpay profile payment addresses for core and platform - #4380
Merged
Conversation
DashPay contract v2 (protocol version 14): the profile document gains two optional public payment fields — corePaymentAddress (position 5) and platformPaymentAddress (position 6), both the 21-byte storage form (variant byte 0x00 P2PKH / 0x01 P2SH + 20-byte HASH160). Payments to them are intentionally public and linkable to the profile (per DIP-33, the public tier); no derivation-source publishing. Follows the DPNS-v2 versioning pattern: schema/v2 + system_data_contract_versions v3 (dashpay: 2) at PV14, with the contract reload on the first block after the protocol change. Rebaselines PV14 fees (check_tx, replace, delete), profile fixture strings, and the deterministic root hash. Extracted from the larger payment-addresses branch; payment detection key purposes and stealth derivation follow in a separate PR. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDashPay v2 schemas and loading were added. Platform v14 now selects and reapplies the updated system contract. Profile payment addresses receive data-trigger validation. Contract-cache insertion now rejects stale versions atomically. Related fee, hash, migration, and document expectations were updated. ChangesDashPay v2 rollout
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ProtocolDispatcher
participant UpgradeTransaction
participant DashPaySystemContract
participant ProfileSchema
ProtocolDispatcher->>UpgradeTransaction: invoke transition_to_version_14
UpgradeTransaction->>DashPaySystemContract: load and reapply DashPay v2
DashPaySystemContract->>ProfileSchema: add payment-address fields
ProfileSchema-->>UpgradeTransaction: return updated profile schema
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
🕓 Ready for review — 4 ahead in queue (commit cbd7e9c) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/dashpay-contract/schema/v2/dashpay.schema.json`:
- Around line 62-76: The payment-address schema fields corePaymentAddress and
platformPaymentAddress currently validate only length; add first-byte validation
allowing only 0x00 or 0x01 while preserving the 21-byte constraint. Update the
11 replacement assertions in
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs
at lines 1451, 1532, 1672, 1754, 1798, 1934, 2016, 2060, 2200, 2282, and 2326 to
use valid type bytes instead of values beginning with 0x3a or 0x14.
In
`@packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`:
- Around line 1087-1090: Extend the pre-upgrade assertions near the existing
corePaymentAddress check to also verify that pre_profile does not contain
"platformPaymentAddress". Preserve the existing rejection behavior and
diagnostic style for both payment-address fields before
transition_to_version_14.
🪄 Autofix
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 Plus
Run ID: fd37586f-bab4-4d0b-a437-c8841fbd1678
📒 Files selected for processing (13)
packages/dashpay-contract/schema/v2/dashpay.schema.jsonpackages/dashpay-contract/src/lib.rspackages/dashpay-contract/src/v2/mod.rspackages/rs-drive-abci/src/execution/check_tx/v0/mod.rspackages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rspackages/rs-drive/src/cache/data_contract.rspackages/rs-drive/src/drive/contract/refresh_cache/mod.rspackages/rs-drive/tests/deterministic_root_hash.rspackages/rs-platform-version/src/version/system_data_contract_versions/mod.rspackages/rs-platform-version/src/version/system_data_contract_versions/v3.rspackages/rs-platform-version/src/version/v14.rs
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4380 +/- ##
============================================
+ Coverage 87.63% 87.64% +0.01%
============================================
Files 2670 2672 +2
Lines 339447 339890 +443
============================================
+ Hits 297465 297892 +427
- Misses 41982 41998 +16
🚀 New features to boost your workflow:
|
6 tasks
thepastaclaw
suggested changes
Aug 12, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The protocol-v14 contract versioning and cache-refresh changes follow the established upgrade structure, but the new payment-address fields accept values whose leading byte is not a supported P2PKH/P2SH type, including values already exercised by the replacement fixtures. The focused DashPay and upgrade tests also leave the v2 schema boundaries, production dispatcher/cache path, and legacy-document compatibility insufficiently covered.
Source: reviewer backends gpt-5.6-sol (general), gpt-5.6-sol (security-auditor), and gpt-5.6-sol (rust-quality); final verifier backend gpt-5.6-sol. openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.
Validated blockers were found in the Codex precheck. Opus is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 2 suggestion(s)
1 additional finding(s) omitted (not in diff).
🤖 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/dashpay-contract/schema/v2/dashpay.schema.json`:
- [BLOCKING] packages/dashpay-contract/schema/v2/dashpay.schema.json:62-76: Reject unsupported payment-address type bytes
These fields are defined as storage-form P2PKH/P2SH addresses, whose leading byte must be 0x00 or 0x01, but consensus currently checks only that the value contains 21 bytes. The replacement tests confirm the gap rather than merely exposing a theoretical possibility: accepted profile documents contain corePaymentAddress beginning with 0x3a and platformPaymentAddress beginning with 0x14. Such values cannot be decoded as the advertised address type, so Platform can publish a payment endpoint that conforming clients must reject. Enforce the type-byte invariant for both fields during profile creation and replacement, using a supported custom content type or a DashPay profile data trigger, and test acceptance of 0x00/0x01 plus rejection of all other leading bytes.
In `packages/dashpay-contract/test/unit/schema.spec.js`:
- [SUGGESTION] packages/dashpay-contract/test/unit/schema.spec.js:8: Exercise the DashPay v2 schema in the contract package
The package's only schema suite imports schema/v1/dashpay.schema.json, so the focused DashPay test command does not validate the newly added v2 profile definitions. It therefore cannot detect invalid 20- or 22-byte payment-address values being accepted, valid 21-byte values being rejected, changes to existing profile constraints, or accidental exposure of the new fields in v1. Keep the v1 regression suite and run equivalent profile validation against v2, including explicit boundary and type-byte cases for both payment-address fields.
In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:1087-1092: Cover the production v13-to-v14 upgrade boundary
This test invokes transition_to_version_14 directly, bypassing perform_events_on_first_block_of_protocol_change and its version-1 wrapper that refreshes the contract caches. It consequently cannot catch an incorrect method-version dispatch, a missing v13-to-v14 condition, or stale cached contract data in the production path. It also stores no v1 profile before replacing the contract, so compatibility of existing profile bytes with the appended v2 properties remains untested. In the current pre-upgrade assertion, only corePaymentAddress is checked even though the comment claims both fields are absent, allowing accidental early exposure of platformPaymentAddress to pass. Add a boundary test that stores a v1 profile, warms the contract cache, asserts both properties are absent, calls the public protocol-change dispatcher, and then reads both the legacy profile and refreshed v2 contract through normal Drive APIs.
…er, cover the v13-v14 boundary Address review: the 21-byte schema constraint alone accepted values whose leading byte is not a supported address type. A new data trigger (bindings list v2, PV14) rejects profile creates/replaces whose corePaymentAddress or platformPaymentAddress does not start with 0x00 (P2PKH) or 0x01 (P2SH) — unlike a Base58Check string, the storage form has no checksum, so this check makes accepted values fully decodable. - profile fixtures now carry valid type bytes (guarded to schema versions that have the fields, so protocol-version-11 variants are untouched) - acceptance/rejection matrix test for both fields across leading bytes - boundary test drives the v13→v14 upgrade through the production perform_events_on_first_block_of_protocol_change dispatcher with a stored v1 profile and warmed contract cache, asserting both fields absent before, present after, and the legacy profile still readable - pre-upgrade assertion now also covers platformPaymentAddress - rebaselines for the schema description change (check_tx fees, root hash) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs (1)
81-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated payment-address fixture setup into a shared helper.
The same block appears at lines 81-89 and 802-810 in this file, and in
replacement.rsandcreation.rs. See the consolidated comment for the full list of sites.🤖 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-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs` around lines 81 - 89, Extract the repeated corePaymentAddress and platformPaymentAddress fixture initialization into a shared helper, then replace the inline blocks in this deletion test and the corresponding sites in replacement.rs and creation.rs with calls to it. Preserve the existing profile-property guard and exact address byte values through the helper.
🤖 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-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs`:
- Around line 154-155: Rename the test function
test_profile_payment_address_type_byte_enforcement to a descriptive
should-prefixed name, such as should_enforce_profile_payment_address_type_bytes,
without changing its test behavior.
---
Nitpick comments:
In
`@packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs`:
- Around line 81-89: Extract the repeated corePaymentAddress and
platformPaymentAddress fixture initialization into a shared helper, then replace
the inline blocks in this deletion test and the corresponding sites in
replacement.rs and creation.rs with calls to it. Preserve the existing
profile-property guard and exact address byte values through the helper.
🪄 Autofix
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 Plus
Run ID: 2558b0c4-40c8-4f10-9ace-dc205f4116d8
📒 Files selected for processing (22)
packages/dashpay-contract/schema/v2/dashpay.schema.jsonpackages/rs-drive-abci/src/execution/check_tx/v0/mod.rspackages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/data_triggers/bindings/list/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/data_triggers/bindings/list/v2/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/data_triggers/triggers/dashpay/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/data_triggers/triggers/dashpay/profile_v0/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rspackages/rs-drive/tests/deterministic_root_hash.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/mod.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v1.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v10.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v2.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v3.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v4.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v5.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v6.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v7.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v8.rspackages/rs-platform-version/src/version/drive_abci_versions/drive_abci_validation_versions/v9.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-drive/tests/deterministic_root_hash.rs
- packages/rs-drive-abci/src/execution/check_tx/v0/mod.rs
- packages/dashpay-contract/schema/v2/dashpay.schema.json
…xed test name Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs (2)
8-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the shared fixture helper in all document test modules.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs,deletion.rs, andreplacement.rsstill define local copies ofset_valid_profile_payment_addressesat lines 8-18. Their calls therefore do not use this implementation. Remove the local copies and importsuper::set_valid_profile_payment_addressesin each module. This keeps the payment-address fixture format in one place.🤖 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-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs` around lines 8 - 18, Remove the local set_valid_profile_payment_addresses definitions from the creation, deletion, and replacement test modules, and import the shared super::set_valid_profile_payment_addresses helper in each module so all calls use the centralized fixture implementation.
12-17: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCheck each payment field independently.
The condition checks only
corePaymentAddress, but the body also setsplatformPaymentAddress. If a versioned profile schema exposes only one optional field, this helper can omit a declared field or add an undeclared field. Check each property before setting it. The current DashPay v2 schema defines both fields, so this protects other versioned fixtures.Suggested guard change
if profile.properties().contains_key("corePaymentAddress") { document.set("corePaymentAddress", vec![0u8; 21].into()); + } + if profile.properties().contains_key("platformPaymentAddress") { let mut platform_payment_address = vec![1u8]; platform_payment_address.extend([0u8; 20]); document.set("platformPaymentAddress", platform_payment_address.into()); }🤖 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-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs` around lines 12 - 17, Update the profile fixture setup around the existing properties().contains_key checks so corePaymentAddress and platformPaymentAddress are validated independently before calling document.set for each field. Preserve the current values, but ensure each optional field is set only when that property exists in the versioned schema.
🤖 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
`@packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs`:
- Around line 8-18: Remove the local set_valid_profile_payment_addresses
definitions from the creation, deletion, and replacement test modules, and
import the shared super::set_valid_profile_payment_addresses helper in each
module so all calls use the centralized fixture implementation.
- Around line 12-17: Update the profile fixture setup around the existing
properties().contains_key checks so corePaymentAddress and
platformPaymentAddress are validated independently before calling document.set
for each field. Preserve the current values, but ensure each optional field is
set only when that property exists in the versioned schema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3d4a29dc-efac-4740-86eb-41dbcdef9d3d
📒 Files selected for processing (4)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/mod.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/creation.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs
- packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs
Member
Author
|
Re the latest CodeRabbit nitpick (review 4917495684): not applicable — 🤖 Addressed by Claude Code |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bfoss765
added a commit
that referenced
this pull request
Aug 12, 2026
Carries #4380 (dashpay profile payment addresses, breaking dpp) + #4381 (rust-dashcore pin bump) + the shield-preflight family. Resolutions: - platform_wallet.rs / shielded_send.rs: false overlaps — keep BOTH our identity_create_from_one_time_key surface and v4.2-dev's shielded_shield_preflight/plan additions. - Error-code collision: v4.2-dev allocated 37-40 to the DPNS marketplace block and 41 to the shield-capacity shortfall, colliding with our ErrorShieldedInviteAlreadyClaimed = 37. Renumbered ours to 43 on every surface (Rust FFI enum, Kotlin arm + test pin, Swift raw value), the allocation the integration branch already ships in QA AARs (42 stays reserved to match it). - DashSdkError.kt / PlatformWalletResult.swift: keep both sides' new error classes/cases, ours renumbered and ordered after the v4.2-dev blocks. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
added a commit
that referenced
this pull request
Aug 12, 2026
Carries #4380 (dashpay profile payment addresses) + #4381 (rust-dashcore pin bump). Conflict + pooled-funding reconciliation, mirroring the resolutions already proven on the integration branch: - identity/network/payments.rs: keep the in-broadcast refusal block, return the 5-tuple with offered_accounts, and sweep the release across every offered account (pooled selection) instead of the single managed_account. - asset_lock/build.rs: the refusal-path release now sweeps result.funding_accounts owner-guarded (the pre-merge single funding_account param no longer exists under pooled funding). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
First slice of DIP-33 (dashpay/dips#188): the public "tips" tier. A DashPay profile can carry public payment addresses for the Core chain and for Platform address funds, so anyone resolving a profile can pay its owner with zero handshake. Payments to these addresses are intentionally public and linkable to the profile; the non-public tier (payment detection keys + stealth one-time addresses) follows in a separate PR stacked on this one (#4272).
What was done?
schema/v2,src/v2,system_data_contract_versionsv3 withdashpay: 2,v14.rsrepoint, contract reload intransition_to_version_14).profiledocument type gains two optional fields, positions appended afterdisplayName:corePaymentAddress(position 5) — 21-byte storage form: variant byte0x00P2PKH /0x01P2SH + 20-byte HASH160. No network byte: the network follows the Platform chain, so a mismatch is unrepresentable. Clients render Base58Check for their network.platformPaymentAddress(position 6) — same 21-byte DIP-18 storage form.How Has This Been Tested?
transition_to_version_14test asserts the profile document type gains both fields across the protocol-change boundary.drive-abcicheck_tx + batch document suites (177 tests),drivedeterministic root hash and cache tests,dashpay-contracttests — all green locally.cargo clippy --workspace --all-featuresclean;cargo fmtapplied.Breaking Changes
Consensus-breaking at protocol version 14: the DashPay system contract is reloaded at v2 on the first block after the protocol change. Pre-release; no migration concerns.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit