-
Notifications
You must be signed in to change notification settings - Fork 45
feat(drive): add WalletUtils system data contract during initialization on devnets/local networks #2696
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v3.0-dev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe changes add inline documentation for genesis state versioning, integrate the WalletUtils system data contract into the platform initialization flow alongside existing system contracts, and update expected processing fee values in batch operation tests to align with revised fee calculations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lklimek
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Checked locally, works but tests fail.
The base branch was changed.
…atform into featadd-wallet-utils-contract
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs (1)
8-8: WalletUtils is intentionally loaded directly rather than cached—not an oversight.The WalletUtils contract is loaded via
load_system_data_contract(line 70) instead of from thesystem_data_contractscache because it is not yet integrated into theSystemDataContractsstruct. Other contracts (DPNS, Withdrawals, Dashpay, etc.) have correspondingload_*()methods because they are cached, but WalletUtils lacks both a struct field and a cache accessor method. This pattern is consistent with how new system contracts are handled before full cache integration. No action needed unless cache integration is planned.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/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.rs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.rs: Rust code must passcargo clippy --workspacelinter checks
Rust code must be formatted usingcargo fmt --all
**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants
Files:
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rspackages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
**/tests/**/*.{js,jsx,ts,tsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
**/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should …'
Unit and integration tests should not perform network calls; mock dependencies
Files:
packages/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.rs
🧠 Learnings (17)
📓 Common learnings
Learnt from: shumkov
Repo: dashpay/platform PR: 2345
File: packages/wallet-utils-contract/schema/v1/wallet-utils-contract-documents.json:49-55
Timestamp: 2024-11-25T07:49:05.419Z
Learning: In the schema definitions for the `wallet-utils-contract` (file `packages/wallet-utils-contract/schema/v1/wallet-utils-contract-documents.json`), the `$createdAt` field is a system field augmented by DPP and does not need to be defined explicitly in the schema's properties.
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2439
File: packages/rs-dpp/src/errors/consensus/state/token/mod.rs:4-22
Timestamp: 2025-01-23T02:10:08.979Z
Learning: The user QuantumExplorer prefers to handle documentation of breaking changes separately from the code changes, particularly for token-related error types and validation rules.
Learnt from: CR
Repo: dashpay/platform PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-25T13:10:23.481Z
Learning: Use state transitions for updates in documents and data contracts
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2711
File: packages/wasm-sdk/AI_REFERENCE.md:771-783
Timestamp: 2025-07-28T20:00:08.502Z
Learning: In packages/wasm-sdk/AI_REFERENCE.md, the documentation correctly shows the actual SDK method signatures (including identityCreate and identityTopUp with their full parameter lists), which may differ from the UI inputs shown in fixed_definitions.json. The UI may collect fewer parameters from users while handling additional requirements internally.
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/mimic/test_quorum.rs:159-164
Timestamp: 2024-11-20T16:16:01.830Z
Learning: QuantumExplorer prefers not to receive auto-generated messages asking to post on social media.
📚 Learning: 2025-10-01T08:37:32.168Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2790
File: packages/rs-drive/src/drive/document/index_uniqueness/validate_document_purchase_transition_action_uniqueness/v1/mod.rs:65-0
Timestamp: 2025-10-01T08:37:32.168Z
Learning: In purchase document transitions (packages/rs-drive/src/drive/document/index_uniqueness/validate_document_purchase_transition_action_uniqueness/v1/mod.rs), the `changed_data_values` field in `UniquenessOfDataRequestUpdateType::ChangedDocument` should be set to an empty BTreeSet (`Cow::Owned(BTreeSet::new())`) because purchase transitions do not modify document data properties (like "price"), only ownership and transfer metadata. An empty set signals the uniqueness validation logic to skip checking unique indexes on data properties.
Applied to files:
packages/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.rs
📚 Learning: 2024-11-25T01:17:02.001Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2347
File: packages/rs-drive/tests/query_tests.rs:438-460
Timestamp: 2024-11-25T01:17:02.001Z
Learning: In Rust test files (`packages/rs-drive/tests/query_tests.rs`), when code is used only in tests, defining explicit enums for fields (like the `status` field in the `Withdrawal` struct) may not be necessary; using primitive types is acceptable.
Applied to files:
packages/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.rs
📚 Learning: 2025-10-09T15:59:28.329Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2716
File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187
Timestamp: 2025-10-09T15:59:28.329Z
Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.
Applied to files:
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2185
File: packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v1/mod.rs:47-62
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In `rebroadcast_expired_withdrawal_documents_v1`, the variable `expired_withdrawal_indices` needs to be a `Vec<WithdrawalTransactionIndex>` rather than a `BTreeSet<WithdrawalTransactionIndex>`, because a vector is necessary for subsequent processing.
Applied to files:
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs
📚 Learning: 2024-10-29T14:40:54.727Z
Learnt from: lklimek
Repo: dashpay/platform PR: 2277
File: packages/rs-sdk/src/core/transaction.rs:0-0
Timestamp: 2024-10-29T14:40:54.727Z
Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.
Applied to files:
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs
📚 Learning: 2024-10-09T00:22:57.778Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2201
File: packages/rs-platform-version/src/version/v2.rs:1186-1188
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the `IdentityTransitionVersions` structure within `packages/rs-platform-version/src/version/v2.rs`, the field `credit_withdrawal` does not need the `identity_` prefix since it is already encompassed within identity state transitions.
Applied to files:
packages/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.rs
📚 Learning: 2024-10-08T13:28:03.529Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2227
File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141
Timestamp: 2024-10-08T13:28:03.529Z
Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mod.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.
Applied to files:
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
📚 Learning: 2024-10-06T16:18:07.994Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:119-120
Timestamp: 2024-10-06T16:18:07.994Z
Learning: In the `run_block_proposal` function in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, it's acceptable to pass `platform_state` to `perform_events_on_first_block_of_protocol_change`, even if `block_platform_state` has been updated.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
📚 Learning: 2024-10-06T16:17:34.571Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs:105-105
Timestamp: 2024-10-06T16:17:34.571Z
Learning: In `run_block_proposal` in `packages/rs-drive-abci/src/execution/engine/run_block_proposal/mod.rs`, when retrieving `last_block_time_ms`, it's acceptable to use `platform_state` instead of `block_platform_state`, even after updating the protocol version.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
📚 Learning: 2024-10-06T16:11:34.946Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2215
File: packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs:19-30
Timestamp: 2024-10-06T16:11:34.946Z
Learning: In the Rust file `packages/rs-drive-abci/src/execution/platform_events/core_based_updates/update_masternode_identities/create_owner_identity/v1/mod.rs`, within the `create_owner_identity_v1` function, the `add_public_keys` method of the `Identity` struct cannot fail and does not require explicit error handling.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs
📚 Learning: 2024-11-22T08:19:14.448Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2345
File: packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:93-99
Timestamp: 2024-11-22T08:19:14.448Z
Learning: In `packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs`, the `insert_contract` method requires an owned `BlockInfo`, so cloning `block_info` is necessary when calling it.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rspackages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
📚 Learning: 2024-11-03T10:39:11.242Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2305
File: packages/rs-drive-abci/src/abci/handler/finalize_block.rs:81-0
Timestamp: 2024-11-03T10:39:11.242Z
Learning: In `packages/rs-drive-abci/src/abci/handler/finalize_block.rs`, the use of `.expect("commit transaction")` after `app.commit_transaction(platform_version)` is intentional due to the provided comment explaining its necessity. Do not flag this usage in future reviews.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs
📚 Learning: 2024-11-20T10:01:50.837Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2257
File: packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs:94-94
Timestamp: 2024-11-20T10:01:50.837Z
Learning: In `packages/rs-drive-abci/src/platform_types/platform_state/v0/old_structures/mod.rs`, when converting with `PublicKey::try_from`, it's acceptable to use `.expect()` to handle potential conversion errors.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs
📚 Learning: 2025-05-28T16:22:26.334Z
Learnt from: QuantumExplorer
Repo: dashpay/platform PR: 2644
File: packages/rs-drive/src/cache/system_contracts.rs:18-19
Timestamp: 2025-05-28T16:22:26.334Z
Learning: In packages/rs-drive/src/cache/system_contracts.rs, the `active_since_protocol_version` field in `ActiveSystemDataContract` struct is intentionally added for future use, not current implementation. QuantumExplorer confirmed this is "meant for later" when questioned about the `#[allow(unused)]` attribute.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
📚 Learning: 2024-11-25T07:49:05.419Z
Learnt from: shumkov
Repo: dashpay/platform PR: 2345
File: packages/wallet-utils-contract/schema/v1/wallet-utils-contract-documents.json:49-55
Timestamp: 2024-11-25T07:49:05.419Z
Learning: In the schema definitions for the `wallet-utils-contract` (file `packages/wallet-utils-contract/schema/v1/wallet-utils-contract-documents.json`), the `$createdAt` field is a system field augmented by DPP and does not need to be defined explicitly in the schema's properties.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
📚 Learning: 2025-04-11T09:08:05.652Z
Learnt from: pauldelucia
Repo: dashpay/platform PR: 2523
File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151
Timestamp: 2025-04-11T09:08:05.652Z
Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.
Applied to files:
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs
🧬 Code graph analysis (3)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs (1)
packages/rs-drive-abci/src/platform_types/state_transitions_processing_result/mod.rs (1)
aggregated_fees(99-101)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs (1)
packages/rs-drive-abci/src/platform_types/state_transitions_processing_result/mod.rs (1)
aggregated_fees(99-101)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/v1/mod.rs (1)
packages/rs-dpp/src/system_data_contracts.rs (1)
load_system_data_contract(101-108)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
- GitHub Check: Rust crates security audit
🔇 Additional comments (3)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/deletion.rs (1)
144-144: Test expectation updated to reflect new fee calculation.The processing fee expectation has been adjusted from 1,711,420 to 1,666,860 (a decrease of 44,560 credits). This change is consistent with the fee adjustment in the replacement test and aligns with the PR's fee recalibration following the WalletUtils contract integration.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/document/replacement.rs (1)
147-147: Test expectation updated to reflect new fee calculation.The processing fee expectation has been adjusted from 1,443,820 to 1,399,260 (a decrease of 44,560 credits). This matches the fee adjustment magnitude in the deletion test, confirming a systematic recalibration of fees.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/mod.rs (1)
30-30: Documentation clarifies genesis state version usage.The inline comments clearly explain which genesis state version is used for each network type (v0 for mainnet/testnet, v1 for devnets). This is helpful context with no functional impact.
Also applies to: 37-37
Issue being fixed or feature implemented
This update adds the WalletUtils system data contract to the platform's initialization process.
What was done?
How Has This Been Tested?
The changes have been tested within the existing framework, ensuring that the WalletUtils contract is loaded and registered correctly without introducing any errors.
Breaking Changes
None
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
For repository code-owners and collaborators only
I have assigned this pull request to a milestone
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.