feat(platform)!: enable DPNS username transfers and sales in protocol version 13#4145
Merged
QuantumExplorer merged 2 commits intoJul 20, 2026
Merged
Conversation
… version 13 Protocol version 13 drops the data-trigger reject bindings for Transfer, Purchase and UpdatePrice on DPNS domain documents (bindings list v1), and bumps the document transfer/purchase high-level operation conversions to v1, which rewrite a transferred or purchased domain's records.identity to the new owner so the username resolves to the buyer. Replace and Delete stay rejected: name records remain immutable and permanent. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughProtocol v13 adds versioned DPNS data-trigger bindings and Drive conversion logic that rewrites ChangesDPNS protocol v13
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client
participant Platform
participant Drive
participant DomainDocument
Client->>Platform: submit DPNS transfer or purchase
Platform->>Drive: convert transition to operations
Drive->>DomainDocument: update records.identity
DomainDocument-->>Platform: updated ownership record
Platform-->>Client: transition result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Collaborator
|
✅ Final review complete — no blockers (commit 6b9827c) |
![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/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs`:
- Around line 1540-1563: Update the purchase test around the successful
execution assertion to snapshot Alice’s balance before the purchase and verify
it increases by dash_to_credits!(0.1) afterward. Use the existing Alice identity
and balance-access patterns, while preserving the current ownership and document
assertions.
- Around line 1455-1466: Extend the !expect_allowed branch in the rejected v12
price-update test to assert that the domain remains unchanged after
processing_result reports the expected PaidConsensusError. Compare the resulting
domain’s $price (or the relevant existing domain state representation) with its
pre-update value before returning, while preserving the current error assertion.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 223c450e-236b-44b9-a1ff-4e1f0ca5b591
📒 Files selected for processing (14)
packages/dpns-contract/src/v1/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/v1/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rspackages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_purchase_transition.rspackages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/document_transfer_transition.rspackages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/document/mod.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/v9.rspackages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/mod.rspackages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/v3.rspackages/rs-platform-version/src/version/drive_versions/mod.rspackages/rs-platform-version/src/version/drive_versions/v8.rspackages/rs-platform-version/src/version/v13.rs
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## v4.1-dev #4145 +/- ##
==========================================
Coverage 87.43% 87.44%
==========================================
Files 2646 2648 +2
Lines 333981 334166 +185
==========================================
+ Hits 292023 292218 +195
+ Misses 41958 41948 -10
🚀 New features to boost your workflow:
|
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Codex + Sonnet
This PR correctly enables DPNS username transfers and sales at protocol version 13: it drops the reject data-trigger bindings for Transfer/Purchase/UpdatePrice on domain documents in a new v1 bindings list while leaving Replace/Delete rejected, and adds a records.identity rewrite to the new owner in the v1 document transfer/purchase high-level operation converters. Both Sonnet and Codex reviewers independently traced the index-update path, GroveDB integrity checks, and PV12/PV13 version gating and found no correctness or consensus defects. The only issues found are test-coverage gaps in the new dpns.rs test module — most notably that the sale test never asserts the seller actually receives payment, and the PV12-rejection branches never assert that the domain document remains untouched — plus a stale inline comment.
Source: reviewers gpt-5.6-sol and claude-sonnet-5; final verifier claude-sonnet-5.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed),gpt-5.6-sol— security-auditor (failed),gpt-5.6-sol— rust-quality (failed),gpt-5.6-sol— ffi-engineer (failed),gpt-5.6-sol— general (failed),gpt-5.6-sol— security-auditor (failed),gpt-5.6-sol— rust-quality (failed),gpt-5.6-sol— ffi-engineer (failed),gpt-5.6-sol— general (failed),gpt-5.6-sol— security-auditor (failed),gpt-5.6-sol— rust-quality (failed),gpt-5.6-sol— ffi-engineer (failed),gpt-5.6-sol— general (superseded_coverage),gpt-5.6-sol— security-auditor (superseded_coverage),gpt-5.6-sol— rust-quality (superseded_coverage),gpt-5.6-sol— ffi-engineer (superseded_coverage),gpt-5.6-sol— general (completed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
claude-sonnet-5— final-verifier - Sonnet reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— security-auditor (failed),claude-sonnet-5— rust-quality (failed),claude-sonnet-5— ffi-engineer (completed),claude-sonnet-5— general (completed),claude-sonnet-5— security-auditor (completed),claude-sonnet-5— rust-quality (completed)
🟡 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/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs`:
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs:1370-1582: test_dpns_username_sale never asserts the seller receives the purchase proceeds
The PR description states this test "asserts ownership, records.identity rewrite, and the seller's balance credit," but `run_dpns_username_sale_at_protocol_version` never fetches or asserts Alice's (the seller's) identity balance before/after the purchase. It checks the buyer's ownership, the `records.identity` rewrite, and that `$price` is cleared, but the actual monetary effect this feature enables — `RemoveFromIdentityBalance` on the buyer and `AddToIdentityBalance` on the original owner in the v1 `DocumentPurchaseTransitionAction` arm — is exercised by zero assertions here. A regression that crediting the wrong identity, or the wrong amount, would pass this test undetected. Two independent reviewers (Claude's rust-quality pass and CodeRabbit) flagged this same gap.
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs:1455-1467: PV12-rejected price update never asserts the domain document is left unchanged
When `expect_allowed` is false, the test only checks that processing returns `PaidConsensusError` / `DataTriggerConditionError` and then returns immediately. It never re-queries the domain document to confirm the rejected update-price transition left `$price` unset and `records.identity`/ownership untouched. A regression that persisted `$price` (or otherwise mutated the document) despite returning the rejection error would pass this test unnoticed, since the sibling transfer test (`run_dpns_username_transfer_at_protocol_version`) does perform this exact post-rejection state check but this sale test's PV12 branch does not.
- [SUGGESTION] packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs:960-1582: No regression test that domain Replace/Delete remain rejected under the new v1 data-trigger bindings at PV13
The PR's stated invariant is that `Replace` and `Delete` stay rejected on DPNS `domain` documents even after v1 drops the reject bindings for `Transfer`/`Purchase`/`UpdatePrice` (per the `list/v1/mod.rs` comment: "Replace and Delete stay rejected: name records remain immutable and permanent"). The new `dpns_username_transfer_tests` module only exercises Transfer and Purchase (plus their PV12-rejected counterparts) — it never submits a `Replace` or `Delete` transition against a domain document at PV13 to confirm the reject binding still fires. The `Replace`/`Delete` `DataTriggerBindingV0` entries are copy-pasted unchanged from v0 into v1, so the risk of an actual regression is low, but nothing catches a future edit to `list/v1/mod.rs` that accidentally drops or alters one of these two bindings — which would silently allow permanent deletion or mutation of purchased/transferred usernames.
Comment on lines
+1370
to
+1582
| async fn run_dpns_username_sale_at_protocol_version( | ||
| protocol_version: ProtocolVersion, | ||
| expect_allowed: bool, | ||
| ) { | ||
| let mut platform = TestPlatformBuilder::new() | ||
| .with_initial_protocol_version(protocol_version) | ||
| .build_with_mock_rpc() | ||
| .set_genesis_state(); | ||
|
|
||
| let platform_state = platform.state.load(); | ||
| let platform_version = platform_state | ||
| .current_platform_version() | ||
| .expect("expected to get current platform version"); | ||
|
|
||
| let mut rng = StdRng::seed_from_u64(438); | ||
|
|
||
| let alice = setup_identity(&mut platform, 959, dash_to_credits!(0.5)); | ||
|
|
||
| let (bob, bob_signer, bob_key) = setup_identity(&mut platform, 451, dash_to_credits!(0.5)); | ||
|
|
||
| let (mut document, dpns_contract) = register_dpns_username( | ||
| &mut platform, | ||
| &alice, | ||
| &mut rng, | ||
| "quantum9", | ||
| platform_version, | ||
| ) | ||
| .await; | ||
|
|
||
| let (alice, signer, key) = &alice; | ||
|
|
||
| let domain = dpns_contract | ||
| .document_type_for_name("domain") | ||
| .expect("expected the domain document type"); | ||
|
|
||
| document.set_revision(Some(2)); | ||
|
|
||
| let documents_batch_update_price_transition = | ||
| BatchTransition::new_document_update_price_transition_from_document( | ||
| document, | ||
| domain, | ||
| dash_to_credits!(0.1), | ||
| key, | ||
| 4, | ||
| 0, | ||
| None, | ||
| signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create documents batch transition for the price update"); | ||
|
|
||
| let documents_batch_update_price_serialized_transition = | ||
| documents_batch_update_price_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected documents batch serialized state transition"); | ||
|
|
||
| let transaction = platform.drive.grove.start_transaction(); | ||
|
|
||
| let processing_result = platform | ||
| .platform | ||
| .process_raw_state_transitions( | ||
| &[documents_batch_update_price_serialized_transition], | ||
| &platform_state, | ||
| &BlockInfo::default_with_time( | ||
| platform_state | ||
| .last_committed_block_time_ms() | ||
| .unwrap_or_default() | ||
| + 3000, | ||
| ), | ||
| &transaction, | ||
| platform_version, | ||
| false, | ||
| None, | ||
| ) | ||
| .expect("expected to process state transition"); | ||
|
|
||
| platform | ||
| .drive | ||
| .grove | ||
| .commit_transaction(transaction) | ||
| .unwrap() | ||
| .expect("expected to commit transaction"); | ||
|
|
||
| if !expect_allowed { | ||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::PaidConsensusError { | ||
| error: ConsensusError::StateError(StateError::DataTriggerError( | ||
| DataTriggerError::DataTriggerConditionError(_) | ||
| )), | ||
| .. | ||
| }] | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::SuccessfulExecution { .. }] | ||
| ); | ||
|
|
||
| // Read the listed document back so the purchase is built on the | ||
| // priced revision | ||
| let mut listed_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | ||
|
|
||
| assert_eq!(listed_documents.len(), 1); | ||
|
|
||
| let mut document = listed_documents.remove(0); | ||
|
|
||
| let price: Credits = document | ||
| .properties() | ||
| .get_integer("$price") | ||
| .expect("expected to get back the price"); | ||
|
|
||
| assert_eq!(price, dash_to_credits!(0.1)); | ||
|
|
||
| document.set_revision(Some(3)); | ||
|
|
||
| let documents_batch_purchase_transition = | ||
| BatchTransition::new_document_purchase_transition_from_document( | ||
| document, | ||
| domain, | ||
| bob.id(), | ||
| dash_to_credits!(0.1), | ||
| &bob_key, | ||
| 1, | ||
| 0, | ||
| None, | ||
| &bob_signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create documents batch transition for the purchase"); | ||
|
|
||
| let documents_batch_purchase_serialized_transition = documents_batch_purchase_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected documents batch serialized state transition"); | ||
|
|
||
| let transaction = platform.drive.grove.start_transaction(); | ||
|
|
||
| let processing_result = platform | ||
| .platform | ||
| .process_raw_state_transitions( | ||
| &[documents_batch_purchase_serialized_transition], | ||
| &platform_state, | ||
| &BlockInfo::default_with_time( | ||
| platform_state | ||
| .last_committed_block_time_ms() | ||
| .unwrap_or_default() | ||
| + 3000, | ||
| ), | ||
| &transaction, | ||
| platform_version, | ||
| false, | ||
| None, | ||
| ) | ||
| .expect("expected to process state transition"); | ||
|
|
||
| platform | ||
| .drive | ||
| .grove | ||
| .commit_transaction(transaction) | ||
| .unwrap() | ||
| .expect("expected to commit transaction"); | ||
|
|
||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::SuccessfulExecution { .. }] | ||
| ); | ||
|
|
||
| // The username must now belong to and resolve to Bob, and it must no | ||
| // longer be for sale | ||
| let bob_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, bob.id()); | ||
|
|
||
| assert_eq!(bob_documents.len(), 1); | ||
|
|
||
| let purchased_document = bob_documents.first().expect("expected a document"); | ||
|
|
||
| assert_eq!(purchased_document.owner_id(), bob.id()); | ||
|
|
||
| assert_identity_record(purchased_document, bob.id()); | ||
|
|
||
| assert!(purchased_document.properties().get("$price").is_none()); | ||
|
|
||
| let alice_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | ||
|
|
||
| assert_eq!(alice_documents.len(), 0); | ||
|
|
||
| let issues = platform | ||
| .drive | ||
| .grove | ||
| .visualize_verify_grovedb(None, true, false, &platform_version.drive.grove_version) | ||
| .expect("expected to have no issues"); | ||
|
|
||
| assert_eq!( | ||
| issues.len(), | ||
| 0, | ||
| "issues are {}", | ||
| issues | ||
| .iter() | ||
| .map(|(hash, (a, b, c))| format!("{}: {} {} {}", hash, a, b, c)) | ||
| .collect::<Vec<_>>() | ||
| .join(" | ") | ||
| ); | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: test_dpns_username_sale never asserts the seller receives the purchase proceeds
The PR description states this test "asserts ownership, records.identity rewrite, and the seller's balance credit," but run_dpns_username_sale_at_protocol_version never fetches or asserts Alice's (the seller's) identity balance before/after the purchase. It checks the buyer's ownership, the records.identity rewrite, and that $price is cleared, but the actual monetary effect this feature enables — RemoveFromIdentityBalance on the buyer and AddToIdentityBalance on the original owner in the v1 DocumentPurchaseTransitionAction arm — is exercised by zero assertions here. A regression that crediting the wrong identity, or the wrong amount, would pass this test undetected. Two independent reviewers (Claude's rust-quality pass and CodeRabbit) flagged this same gap.
source: ['claude', 'coderabbit']
Member
Author
There was a problem hiding this comment.
Fixed in 6b9827c — the sale test now snapshots Alice's balance before the purchase and asserts it grows by exactly the sale price plus her released document-storage refund (the purchased revision is re-stored under Bob's storage flags, refunding the bytes Alice prepaid). Also asserts Bob's balance drops by more than the price (price + fees).
Collaborator
There was a problem hiding this comment.
Resolved in 6b9827c — test_dpns_username_sale never asserts the seller receives the purchase proceeds no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
Comment on lines
+1455
to
+1467
| if !expect_allowed { | ||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::PaidConsensusError { | ||
| error: ConsensusError::StateError(StateError::DataTriggerError( | ||
| DataTriggerError::DataTriggerConditionError(_) | ||
| )), | ||
| .. | ||
| }] | ||
| ); | ||
|
|
||
| return; | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: PV12-rejected price update never asserts the domain document is left unchanged
When expect_allowed is false, the test only checks that processing returns PaidConsensusError / DataTriggerConditionError and then returns immediately. It never re-queries the domain document to confirm the rejected update-price transition left $price unset and records.identity/ownership untouched. A regression that persisted $price (or otherwise mutated the document) despite returning the rejection error would pass this test unnoticed, since the sibling transfer test (run_dpns_username_transfer_at_protocol_version) does perform this exact post-rejection state check but this sale test's PV12 branch does not.
Suggested change
| if !expect_allowed { | |
| assert_matches!( | |
| processing_result.execution_results().as_slice(), | |
| [StateTransitionExecutionResult::PaidConsensusError { | |
| error: ConsensusError::StateError(StateError::DataTriggerError( | |
| DataTriggerError::DataTriggerConditionError(_) | |
| )), | |
| .. | |
| }] | |
| ); | |
| return; | |
| } | |
| if !expect_allowed { | |
| assert_matches!( | |
| processing_result.execution_results().as_slice(), | |
| [StateTransitionExecutionResult::PaidConsensusError { | |
| error: ConsensusError::StateError(StateError::DataTriggerError( | |
| DataTriggerError::DataTriggerConditionError(_) | |
| )), | |
| .. | |
| }] | |
| ); | |
| // The domain must still belong to Alice, keep its records.identity | |
| // pointing at her, and have no $price set | |
| let alice_documents = | |
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | |
| assert_eq!(alice_documents.len(), 1); | |
| let kept_document = alice_documents.first().expect("expected a document"); | |
| assert_eq!(kept_document.owner_id(), alice.id()); | |
| assert_identity_record(kept_document, alice.id()); | |
| assert!(kept_document.properties().get("$price").is_none()); | |
| return; | |
| } |
source: ['coderabbit']
Member
Author
There was a problem hiding this comment.
Fixed in 6b9827c — the PV12-rejected branch now re-queries the domain and asserts it still belongs to Alice, still resolves to her via records.identity, and has no $price persisted (mirroring the transfer test's post-rejection check).
Collaborator
There was a problem hiding this comment.
Resolved in 6b9827c — PV12-rejected price update never asserts the domain document is left unchanged no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
Comment on lines
+960
to
+1582
| mod dpns_username_transfer_tests { | ||
| use super::*; | ||
| use dpp::consensus::state::data_trigger::DataTriggerError; | ||
| use dpp::data_contract::DataContract; | ||
| use dpp::document::Document; | ||
| use dpp::identity::{Identity, IdentityPublicKey}; | ||
| use dpp::util::hash::hash_double; | ||
| use dpp::util::strings::convert_to_homograph_safe_chars; | ||
| use drive::query::{InternalClauses, WhereClause, WhereOperator}; | ||
| use simple_signer::signer::SimpleSigner; | ||
| use std::collections::BTreeMap; | ||
| use std::sync::Arc; | ||
|
|
||
| use crate::rpc::core::MockCoreRPCLike; | ||
| use crate::test::helpers::setup::TempPlatform; | ||
| use dpp::prelude::Identifier; | ||
| use dpp::version::ProtocolVersion; | ||
|
|
||
| /// Registers `<label>.dash` (preorder + domain) for the given identity and | ||
| /// returns the domain document as submitted along with the DPNS contract. | ||
| /// | ||
| /// The label must not match the contested-name regex (it should contain at | ||
| /// least one digit in 2-9) so the domain document is inserted immediately | ||
| /// instead of starting a masternode vote contest. | ||
| async fn register_dpns_username( | ||
| platform: &mut TempPlatform<MockCoreRPCLike>, | ||
| identity: &(Identity, SimpleSigner, IdentityPublicKey), | ||
| rng: &mut StdRng, | ||
| label: &str, | ||
| platform_version: &PlatformVersion, | ||
| ) -> (Document, Arc<DataContract>) { | ||
| let (identity, signer, key) = identity; | ||
|
|
||
| let platform_state = platform.state.load(); | ||
|
|
||
| let dpns = platform.drive.cache.system_data_contracts.load_dpns(); | ||
| let dpns_contract = dpns.clone(); | ||
|
|
||
| let preorder = dpns_contract | ||
| .document_type_for_name("preorder") | ||
| .expect("expected the preorder document type"); | ||
|
|
||
| let domain = dpns_contract | ||
| .document_type_for_name("domain") | ||
| .expect("expected the domain document type"); | ||
|
|
||
| let entropy = Bytes32::random_with_rng(rng); | ||
|
|
||
| let mut preorder_document = preorder | ||
| .random_document_with_identifier_and_entropy( | ||
| rng, | ||
| identity.id(), | ||
| entropy, | ||
| DocumentFieldFillType::FillIfNotRequired, | ||
| DocumentFieldFillSize::AnyDocumentFillSize, | ||
| platform_version, | ||
| ) | ||
| .expect("expected a random preorder document"); | ||
|
|
||
| let mut document = domain | ||
| .random_document_with_identifier_and_entropy( | ||
| rng, | ||
| identity.id(), | ||
| entropy, | ||
| DocumentFieldFillType::FillIfNotRequired, | ||
| DocumentFieldFillSize::AnyDocumentFillSize, | ||
| platform_version, | ||
| ) | ||
| .expect("expected a random domain document"); | ||
|
|
||
| let normalized_label = convert_to_homograph_safe_chars(label); | ||
|
|
||
| document.set("parentDomainName", "dash".into()); | ||
| document.set("normalizedParentDomainName", "dash".into()); | ||
| document.set("label", label.into()); | ||
| document.set("normalizedLabel", normalized_label.clone().into()); | ||
| document.set("records.identity", document.owner_id().into()); | ||
| document.set("subdomainRules.allowSubdomains", false.into()); | ||
|
|
||
| let salt: [u8; 32] = rng.gen(); | ||
|
|
||
| let mut salted_domain_buffer: Vec<u8> = vec![]; | ||
| salted_domain_buffer.extend(salt); | ||
| salted_domain_buffer.extend((normalized_label + ".dash").as_bytes()); | ||
|
|
||
| let salted_domain_hash = hash_double(salted_domain_buffer); | ||
|
|
||
| preorder_document.set("saltedDomainHash", salted_domain_hash.into()); | ||
| document.set("preorderSalt", salt.into()); | ||
|
|
||
| let documents_batch_create_preorder_transition = | ||
| BatchTransition::new_document_creation_transition_from_document( | ||
| preorder_document, | ||
| preorder, | ||
| entropy.0, | ||
| key, | ||
| 2, | ||
| 0, | ||
| None, | ||
| signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create preorder batch transition"); | ||
|
|
||
| let documents_batch_create_serialized_preorder_transition = | ||
| documents_batch_create_preorder_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected preorder batch serialized state transition"); | ||
|
|
||
| let documents_batch_create_transition = | ||
| BatchTransition::new_document_creation_transition_from_document( | ||
| document.clone(), | ||
| domain, | ||
| entropy.0, | ||
| key, | ||
| 3, | ||
| 0, | ||
| None, | ||
| signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create domain batch transition"); | ||
|
|
||
| let documents_batch_create_serialized_transition = documents_batch_create_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected domain batch serialized state transition"); | ||
|
|
||
| for serialized_transition in [ | ||
| documents_batch_create_serialized_preorder_transition, | ||
| documents_batch_create_serialized_transition, | ||
| ] { | ||
| let transaction = platform.drive.grove.start_transaction(); | ||
|
|
||
| let processing_result = platform | ||
| .platform | ||
| .process_raw_state_transitions( | ||
| &[serialized_transition], | ||
| &platform_state, | ||
| &BlockInfo::default_with_time( | ||
| platform_state | ||
| .last_committed_block_time_ms() | ||
| .unwrap_or_default() | ||
| + 3000, | ||
| ), | ||
| &transaction, | ||
| platform_version, | ||
| false, | ||
| None, | ||
| ) | ||
| .expect("expected to process state transition"); | ||
|
|
||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::SuccessfulExecution { .. }] | ||
| ); | ||
|
|
||
| platform | ||
| .drive | ||
| .grove | ||
| .commit_transaction(transaction) | ||
| .unwrap() | ||
| .expect("expected to commit transaction"); | ||
| } | ||
|
|
||
| (document, dpns_contract) | ||
| } | ||
|
|
||
| /// Queries domain documents whose `records.identity` name record points to | ||
| /// the given identity. | ||
| fn query_domain_documents_by_record_identity( | ||
| platform: &TempPlatform<MockCoreRPCLike>, | ||
| dpns_contract: &DataContract, | ||
| identity_id: Identifier, | ||
| ) -> Vec<Document> { | ||
| let domain = dpns_contract | ||
| .document_type_for_name("domain") | ||
| .expect("expected the domain document type"); | ||
|
|
||
| let drive_query = DriveDocumentQuery { | ||
| contract: dpns_contract, | ||
| document_type: domain, | ||
| internal_clauses: InternalClauses { | ||
| primary_key_in_clause: None, | ||
| primary_key_equal_clause: None, | ||
| in_clause: None, | ||
| range_clause: None, | ||
| equal_clauses: BTreeMap::from([( | ||
| "records.identity".to_string(), | ||
| WhereClause { | ||
| field: "records.identity".to_string(), | ||
| operator: WhereOperator::Equal, | ||
| value: Value::Identifier(identity_id.to_buffer()), | ||
| }, | ||
| )]), | ||
| }, | ||
| offset: None, | ||
| limit: None, | ||
| order_by: Default::default(), | ||
| start_at: None, | ||
| start_at_included: false, | ||
| block_time_ms: None, | ||
| }; | ||
|
|
||
| platform | ||
| .drive | ||
| .query_documents(drive_query, None, false, None, None) | ||
| .expect("expected to query domain documents") | ||
| .documents_owned() | ||
| } | ||
|
|
||
| fn assert_identity_record(document: &Document, expected_identity_id: Identifier) { | ||
| let records = document | ||
| .properties() | ||
| .get("records") | ||
| .expect("expected the domain document to have records"); | ||
|
|
||
| let record_identity = records | ||
| .get_optional_identifier("identity") | ||
| .expect("expected a valid identity record") | ||
| .expect("expected an identity record to be set"); | ||
|
|
||
| assert_eq!(record_identity, expected_identity_id); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_dpns_username_transfer() { | ||
| run_dpns_username_transfer_at_protocol_version( | ||
| PlatformVersion::latest().protocol_version, | ||
| true, | ||
| ) | ||
| .await; | ||
| } | ||
|
|
||
| /// PROTOCOL_VERSION_12: domain transfers are still rejected by the | ||
| /// `reject_data_trigger` binding. Pinned so v12 chain history stays | ||
| /// bit-for-bit reproducible. | ||
| #[tokio::test] | ||
| async fn test_dpns_username_transfer_protocol_version_12_rejected() { | ||
| run_dpns_username_transfer_at_protocol_version(12, false).await; | ||
| } | ||
|
|
||
| async fn run_dpns_username_transfer_at_protocol_version( | ||
| protocol_version: ProtocolVersion, | ||
| expect_allowed: bool, | ||
| ) { | ||
| let mut platform = TestPlatformBuilder::new() | ||
| .with_initial_protocol_version(protocol_version) | ||
| .build_with_mock_rpc() | ||
| .set_genesis_state(); | ||
|
|
||
| let platform_state = platform.state.load(); | ||
| let platform_version = platform_state | ||
| .current_platform_version() | ||
| .expect("expected to get current platform version"); | ||
|
|
||
| let mut rng = StdRng::seed_from_u64(437); | ||
|
|
||
| let alice = setup_identity(&mut platform, 958, dash_to_credits!(0.5)); | ||
|
|
||
| let (bob, _, _) = setup_identity(&mut platform, 450, dash_to_credits!(0.5)); | ||
|
|
||
| // "quantum7" contains a digit outside 0/1 so the name is not contested | ||
| let (mut document, dpns_contract) = register_dpns_username( | ||
| &mut platform, | ||
| &alice, | ||
| &mut rng, | ||
| "quantum7", | ||
| platform_version, | ||
| ) | ||
| .await; | ||
|
|
||
| let (alice, signer, key) = &alice; | ||
|
|
||
| let domain = dpns_contract | ||
| .document_type_for_name("domain") | ||
| .expect("expected the domain document type"); | ||
|
|
||
| document.set_revision(Some(2)); | ||
|
|
||
| let documents_batch_transfer_transition = | ||
| BatchTransition::new_document_transfer_transition_from_document( | ||
| document, | ||
| domain, | ||
| bob.id(), | ||
| key, | ||
| 4, | ||
| 0, | ||
| None, | ||
| signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create documents batch transition for transfer"); | ||
|
|
||
| let documents_batch_transfer_serialized_transition = documents_batch_transfer_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected documents batch serialized state transition"); | ||
|
|
||
| let transaction = platform.drive.grove.start_transaction(); | ||
|
|
||
| let processing_result = platform | ||
| .platform | ||
| .process_raw_state_transitions( | ||
| &[documents_batch_transfer_serialized_transition], | ||
| &platform_state, | ||
| &BlockInfo::default_with_time( | ||
| platform_state | ||
| .last_committed_block_time_ms() | ||
| .unwrap_or_default() | ||
| + 3000, | ||
| ), | ||
| &transaction, | ||
| platform_version, | ||
| false, | ||
| None, | ||
| ) | ||
| .expect("expected to process state transition"); | ||
|
|
||
| platform | ||
| .drive | ||
| .grove | ||
| .commit_transaction(transaction) | ||
| .unwrap() | ||
| .expect("expected to commit transaction"); | ||
|
|
||
| if expect_allowed { | ||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::SuccessfulExecution { .. }] | ||
| ); | ||
|
|
||
| // The username must now belong to and resolve to Bob | ||
| let bob_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, bob.id()); | ||
|
|
||
| assert_eq!(bob_documents.len(), 1); | ||
|
|
||
| let transferred_document = bob_documents.first().expect("expected a document"); | ||
|
|
||
| assert_eq!(transferred_document.owner_id(), bob.id()); | ||
|
|
||
| assert_identity_record(transferred_document, bob.id()); | ||
|
|
||
| // Alice must no longer have a name record pointing to her | ||
| let alice_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | ||
|
|
||
| assert_eq!(alice_documents.len(), 0); | ||
| } else { | ||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::PaidConsensusError { | ||
| error: ConsensusError::StateError(StateError::DataTriggerError( | ||
| DataTriggerError::DataTriggerConditionError(_) | ||
| )), | ||
| .. | ||
| }] | ||
| ); | ||
|
|
||
| // The username must still belong to and resolve to Alice | ||
| let alice_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | ||
|
|
||
| assert_eq!(alice_documents.len(), 1); | ||
|
|
||
| let kept_document = alice_documents.first().expect("expected a document"); | ||
|
|
||
| assert_eq!(kept_document.owner_id(), alice.id()); | ||
| } | ||
|
|
||
| let issues = platform | ||
| .drive | ||
| .grove | ||
| .visualize_verify_grovedb(None, true, false, &platform_version.drive.grove_version) | ||
| .expect("expected to have no issues"); | ||
|
|
||
| assert_eq!( | ||
| issues.len(), | ||
| 0, | ||
| "issues are {}", | ||
| issues | ||
| .iter() | ||
| .map(|(hash, (a, b, c))| format!("{}: {} {} {}", hash, a, b, c)) | ||
| .collect::<Vec<_>>() | ||
| .join(" | ") | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_dpns_username_sale() { | ||
| run_dpns_username_sale_at_protocol_version( | ||
| PlatformVersion::latest().protocol_version, | ||
| true, | ||
| ) | ||
| .await; | ||
| } | ||
|
|
||
| /// PROTOCOL_VERSION_12: setting a price on a domain document is still | ||
| /// rejected by the `reject_data_trigger` binding, so a sale can not even | ||
| /// be started. Pinned so v12 chain history stays bit-for-bit reproducible. | ||
| #[tokio::test] | ||
| async fn test_dpns_username_sale_protocol_version_12_rejected() { | ||
| run_dpns_username_sale_at_protocol_version(12, false).await; | ||
| } | ||
|
|
||
| async fn run_dpns_username_sale_at_protocol_version( | ||
| protocol_version: ProtocolVersion, | ||
| expect_allowed: bool, | ||
| ) { | ||
| let mut platform = TestPlatformBuilder::new() | ||
| .with_initial_protocol_version(protocol_version) | ||
| .build_with_mock_rpc() | ||
| .set_genesis_state(); | ||
|
|
||
| let platform_state = platform.state.load(); | ||
| let platform_version = platform_state | ||
| .current_platform_version() | ||
| .expect("expected to get current platform version"); | ||
|
|
||
| let mut rng = StdRng::seed_from_u64(438); | ||
|
|
||
| let alice = setup_identity(&mut platform, 959, dash_to_credits!(0.5)); | ||
|
|
||
| let (bob, bob_signer, bob_key) = setup_identity(&mut platform, 451, dash_to_credits!(0.5)); | ||
|
|
||
| let (mut document, dpns_contract) = register_dpns_username( | ||
| &mut platform, | ||
| &alice, | ||
| &mut rng, | ||
| "quantum9", | ||
| platform_version, | ||
| ) | ||
| .await; | ||
|
|
||
| let (alice, signer, key) = &alice; | ||
|
|
||
| let domain = dpns_contract | ||
| .document_type_for_name("domain") | ||
| .expect("expected the domain document type"); | ||
|
|
||
| document.set_revision(Some(2)); | ||
|
|
||
| let documents_batch_update_price_transition = | ||
| BatchTransition::new_document_update_price_transition_from_document( | ||
| document, | ||
| domain, | ||
| dash_to_credits!(0.1), | ||
| key, | ||
| 4, | ||
| 0, | ||
| None, | ||
| signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create documents batch transition for the price update"); | ||
|
|
||
| let documents_batch_update_price_serialized_transition = | ||
| documents_batch_update_price_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected documents batch serialized state transition"); | ||
|
|
||
| let transaction = platform.drive.grove.start_transaction(); | ||
|
|
||
| let processing_result = platform | ||
| .platform | ||
| .process_raw_state_transitions( | ||
| &[documents_batch_update_price_serialized_transition], | ||
| &platform_state, | ||
| &BlockInfo::default_with_time( | ||
| platform_state | ||
| .last_committed_block_time_ms() | ||
| .unwrap_or_default() | ||
| + 3000, | ||
| ), | ||
| &transaction, | ||
| platform_version, | ||
| false, | ||
| None, | ||
| ) | ||
| .expect("expected to process state transition"); | ||
|
|
||
| platform | ||
| .drive | ||
| .grove | ||
| .commit_transaction(transaction) | ||
| .unwrap() | ||
| .expect("expected to commit transaction"); | ||
|
|
||
| if !expect_allowed { | ||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::PaidConsensusError { | ||
| error: ConsensusError::StateError(StateError::DataTriggerError( | ||
| DataTriggerError::DataTriggerConditionError(_) | ||
| )), | ||
| .. | ||
| }] | ||
| ); | ||
|
|
||
| return; | ||
| } | ||
|
|
||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::SuccessfulExecution { .. }] | ||
| ); | ||
|
|
||
| // Read the listed document back so the purchase is built on the | ||
| // priced revision | ||
| let mut listed_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | ||
|
|
||
| assert_eq!(listed_documents.len(), 1); | ||
|
|
||
| let mut document = listed_documents.remove(0); | ||
|
|
||
| let price: Credits = document | ||
| .properties() | ||
| .get_integer("$price") | ||
| .expect("expected to get back the price"); | ||
|
|
||
| assert_eq!(price, dash_to_credits!(0.1)); | ||
|
|
||
| document.set_revision(Some(3)); | ||
|
|
||
| let documents_batch_purchase_transition = | ||
| BatchTransition::new_document_purchase_transition_from_document( | ||
| document, | ||
| domain, | ||
| bob.id(), | ||
| dash_to_credits!(0.1), | ||
| &bob_key, | ||
| 1, | ||
| 0, | ||
| None, | ||
| &bob_signer, | ||
| platform_version, | ||
| None, | ||
| ) | ||
| .await | ||
| .expect("expect to create documents batch transition for the purchase"); | ||
|
|
||
| let documents_batch_purchase_serialized_transition = documents_batch_purchase_transition | ||
| .serialize_to_bytes() | ||
| .expect("expected documents batch serialized state transition"); | ||
|
|
||
| let transaction = platform.drive.grove.start_transaction(); | ||
|
|
||
| let processing_result = platform | ||
| .platform | ||
| .process_raw_state_transitions( | ||
| &[documents_batch_purchase_serialized_transition], | ||
| &platform_state, | ||
| &BlockInfo::default_with_time( | ||
| platform_state | ||
| .last_committed_block_time_ms() | ||
| .unwrap_or_default() | ||
| + 3000, | ||
| ), | ||
| &transaction, | ||
| platform_version, | ||
| false, | ||
| None, | ||
| ) | ||
| .expect("expected to process state transition"); | ||
|
|
||
| platform | ||
| .drive | ||
| .grove | ||
| .commit_transaction(transaction) | ||
| .unwrap() | ||
| .expect("expected to commit transaction"); | ||
|
|
||
| assert_matches!( | ||
| processing_result.execution_results().as_slice(), | ||
| [StateTransitionExecutionResult::SuccessfulExecution { .. }] | ||
| ); | ||
|
|
||
| // The username must now belong to and resolve to Bob, and it must no | ||
| // longer be for sale | ||
| let bob_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, bob.id()); | ||
|
|
||
| assert_eq!(bob_documents.len(), 1); | ||
|
|
||
| let purchased_document = bob_documents.first().expect("expected a document"); | ||
|
|
||
| assert_eq!(purchased_document.owner_id(), bob.id()); | ||
|
|
||
| assert_identity_record(purchased_document, bob.id()); | ||
|
|
||
| assert!(purchased_document.properties().get("$price").is_none()); | ||
|
|
||
| let alice_documents = | ||
| query_domain_documents_by_record_identity(&platform, &dpns_contract, alice.id()); | ||
|
|
||
| assert_eq!(alice_documents.len(), 0); | ||
|
|
||
| let issues = platform | ||
| .drive | ||
| .grove | ||
| .visualize_verify_grovedb(None, true, false, &platform_version.drive.grove_version) | ||
| .expect("expected to have no issues"); | ||
|
|
||
| assert_eq!( | ||
| issues.len(), | ||
| 0, | ||
| "issues are {}", | ||
| issues | ||
| .iter() | ||
| .map(|(hash, (a, b, c))| format!("{}: {} {} {}", hash, a, b, c)) | ||
| .collect::<Vec<_>>() | ||
| .join(" | ") | ||
| ); | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: No regression test that domain Replace/Delete remain rejected under the new v1 data-trigger bindings at PV13
The PR's stated invariant is that Replace and Delete stay rejected on DPNS domain documents even after v1 drops the reject bindings for Transfer/Purchase/UpdatePrice (per the list/v1/mod.rs comment: "Replace and Delete stay rejected: name records remain immutable and permanent"). The new dpns_username_transfer_tests module only exercises Transfer and Purchase (plus their PV12-rejected counterparts) — it never submits a Replace or Delete transition against a domain document at PV13 to confirm the reject binding still fires. The Replace/Delete DataTriggerBindingV0 entries are copy-pasted unchanged from v0 into v1, so the risk of an actual regression is low, but nothing catches a future edit to list/v1/mod.rs that accidentally drops or alters one of these two bindings — which would silently allow permanent deletion or mutation of purchased/transferred usernames.
source: ['claude']
Member
Author
There was a problem hiding this comment.
Fixed in 6b9827c — added test_dpns_username_replace_and_delete_still_rejected at PV13. Worth noting the test is even more load-bearing than flagged: the domain doctype sets canBeDeleted: true, so the reject data trigger is the ONLY guard preventing deletion of name records — Delete asserts DataTriggerConditionError. Replace is stopped earlier by documentsMutable: false (InvalidDocumentTransitionActionError), before data triggers run, and the test pins that too. Both paths also assert the domain still belongs to and resolves to the original owner, plus a GroveDB integrity check.
Collaborator
There was a problem hiding this comment.
Resolved in 6b9827c — No regression test that domain Replace/Delete remain rejected under the new v1 data-trigger bindings at PV13 no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| identity_credit_withdrawal_transition: 0, | ||
| identity_top_up_transition: 0, | ||
| identity_top_up_from_addresses_transition: 0, | ||
| identity_update_transition: 1, //changed |
Collaborator
There was a problem hiding this comment.
💬 Nitpick: Misleading // changed marker copied forward from v2, not actually changed in v3
identity_update_transition: 1, //changed was carried over verbatim from drive_state_transition_method_versions/v2.rs, where the value genuinely changed from v1's 0. In this new v3 struct the value is identical to v2 (1), so relative to the version this struct actually supersedes, nothing changed here — only document_purchase_transition and document_transfer_transition changed. These // changed annotations are relied on by reviewers to understand what a new version struct actually altered without diffing the whole field literal; a stale marker here could lead a future auditor to believe v13 touched identity-update behavior when it did not.
Suggested change
| identity_update_transition: 1, //changed | |
| identity_update_transition: 1, |
source: ['claude']
Member
Author
There was a problem hiding this comment.
Fixed in 6b9827c — dropped the stale marker; only document_purchase_transition and document_transfer_transition are annotated as changed in v3.
Collaborator
There was a problem hiding this comment.
Resolved in 6b9827c — Misleading // changed marker copied forward from v2, not actually changed in v3 no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
…place/Delete pin - assert the seller's balance grows by exactly the sale price plus her released document storage on a username sale, and that the buyer pays the price plus fees - assert a PV12-rejected price update leaves the domain document untouched (owner, records.identity, no $price) - pin that Replace and Delete on domain documents stay rejected at PV13: Delete is guarded only by the reject data trigger since the doctype sets canBeDeleted true, Replace also by the doctype not being mutable - drop a stale '// changed' marker copied from v2 into drive_state_transition_method_versions/v3.rs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rs (1)
1456-1483: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRejected sale path skips GroveDB verification (parity gap with transfer test).
Because this branch
returns at Line 1482, thevisualize_verify_grovedbcheck at Lines 1619-1634 only runs on the allowed path. The transfer harness verifies grove integrity in both branches, so a v12-rejected price update that leaves grove in a bad state would go uncaught here. Consider running the verification before returning (and factoring the repeated block at 1336-1351, 1619-1634, 1779-1794 into a smallassert_no_grovedb_issues(&platform, platform_version)helper to cut the duplication).🤖 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/dpns.rs` around lines 1456 - 1483, Ensure the rejected sale branch in the test performs visualize_verify_grovedb before returning, matching the allowed path and transfer-test coverage. Prefer extracting the repeated GroveDB verification logic into an assert_no_grovedb_issues helper and invoke it from all existing verification sites, including the rejected branch.
🤖 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/document/dpns.rs`:
- Around line 1456-1483: Ensure the rejected sale branch in the test performs
visualize_verify_grovedb before returning, matching the allowed path and
transfer-test coverage. Prefer extracting the repeated GroveDB verification
logic into an assert_no_grovedb_issues helper and invoke it from all existing
verification sites, including the rejected branch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bcb13ef-425c-4994-af02-b8e450e44666
📒 Files selected for processing (2)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/dpns.rspackages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/v3.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/rs-platform-version/src/version/drive_versions/drive_state_transition_method_versions/v3.rs
thepastaclaw
approved these changes
Jul 20, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Codex + Sonnet
This delta (b6e1a67..6b9827c) is test-only: it adds seller/buyer balance assertions to the DPNS sale test, asserts the PV12-rejected price update leaves the domain document untouched, adds a new PV13 regression test pinning that Replace and Delete stay rejected on domain documents, and removes a stale '// changed' comment in v3.rs. Verified directly against the diff and current head — all four prior findings are genuinely fixed, no production/consensus code changed, and no new in-scope issues were found.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed),gpt-5.6-sol— security-auditor (failed),gpt-5.6-sol— rust-quality (failed),gpt-5.6-sol— ffi-engineer (failed),gpt-5.6-sol— general (failed),gpt-5.6-sol— security-auditor (completed),gpt-5.6-sol— rust-quality (completed),gpt-5.6-sol— ffi-engineer (failed),gpt-5.6-sol— general (completed),gpt-5.6-sol— ffi-engineer (completed) - Verifier:
claude-sonnet-5— verifier - Sonnet reviewers:
claude-sonnet-5— general (failed),claude-sonnet-5— security-auditor (failed),claude-sonnet-5— rust-quality (failed),claude-sonnet-5— ffi-engineer (failed),claude-sonnet-5— general (failed),claude-sonnet-5— security-auditor (completed),claude-sonnet-5— rust-quality (failed),claude-sonnet-5— ffi-engineer (failed),claude-sonnet-5— general (completed),claude-sonnet-5— rust-quality (completed),claude-sonnet-5— ffi-engineer (failed),claude-sonnet-5— ffi-engineer (completed)
shumkov
approved these changes
Jul 20, 2026
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
3 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
DPNS usernames could not change hands: even though the DPNS
domaindocument type has declaredtransferable: 1andtradeMode: 1(direct purchase) since genesis, data triggers unconditionally rejectedTransfer,PurchaseandUpdatePriceon domain documents. On top of that, a username resolves throughrecords.identity— not through the document's$ownerId— and domain documents are immutable, so even if a transfer had been allowed, the name would have kept resolving to the previous owner with no way for the new owner to repoint it.This PR enables username transfers and sales as of protocol version 13.
What was done?
rs-drive-abci): drops the reject bindings forTransfer,PurchaseandUpdatePriceon DPNSdomaindocuments, letting the generic document validation paths (which already honortransferable/tradeMode) accept these transitions.ReplaceandDeletestay rejected — name records remain immutable and permanent.records.identityrewrite (rs-drive): v1 of the document transfer and purchase high-level operation conversions rewrites a transferred or purchased domain document'srecords.identityto the new owner as part of the same batch operation, so the username resolves to the buyer atomically. All other document types are unaffected.rs-platform-version): newDRIVE_ABCI_VALIDATION_VERSIONS_V9(bindings: 1),DRIVE_STATE_TRANSITION_METHOD_VERSIONS_V3(transfer/purchase conversion: 1) andDRIVE_VERSION_V8, consumed byPLATFORM_V13. Protocol versions ≤ 12 keep the old structs for chain replay.identityproperty-name constant used by the rewrite.How Has This Been Tested?
New drive-abci execution tests in
batch/tests/document/dpns.rs(dpns_username_transfer_tests):test_dpns_username_transfer— registers a name, transfers the domain document to a second identity under PV13, and asserts the processed document's owner andrecords.identityboth point to the recipient.test_dpns_username_sale— sets a price withUpdatePrice, purchases with a second identity, and asserts ownership,records.identityrewrite, and the seller's balance credit.test_dpns_username_transfer_protocol_version_12_rejected/test_dpns_username_sale_protocol_version_12_rejected— the same flows under PV12 still hit the reject data trigger, proving activation is gated on PV13.All 9 DPNS-related drive-abci tests pass;
cargo clippyon the touched crates andcargo fmt --allare clean.Breaking Changes
Consensus-breaking at protocol version 13 activation: nodes without this change would reject DPNS domain transfer/purchase/update-price transitions (and would not perform the
records.identityrewrite) that nodes with this change accept once PV13 activates. No behavior change for protocol versions ≤ 12.Checklist:
For repository code-owners and collaborators only
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests