fix(platform): correct burn identity in group actions#2615
Conversation
WalkthroughThis update introduces explicit tracking of the identity from which tokens are burned across the token platform. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Platform
participant GroupAction
participant TokenEvent
participant StateValidation
participant HistoryContract
Client->>Platform: Propose token burn (specify burn_from_id)
Platform->>GroupAction: Create GroupActionV0 (with proposer_id)
GroupAction->>TokenEvent: Create Burn event (amount, burn_from_id, note)
Platform->>StateValidation: Validate burn (uses burn_from_id)
StateValidation->>HistoryContract: Record burn (burnFromId in document)
Platform->>Client: Respond with burn result (includes burn_from_id)
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (2)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/v0/mod.rs (2)
31-41:⚠️ Potential issueAccessor signatures still use
u64– type mismatch withTokenAmount
burn_amountis nowTokenAmount, yet the trait exposesu64.
This will not compile unlessTokenAmountis a type-alias. Even then, the API is misleading.- fn burn_amount(&self) -> u64; + fn burn_amount(&self) -> TokenAmount; - fn set_burn_amount(&mut self, amount: u64); + fn set_burn_amount(&mut self, amount: TokenAmount);
92-106:⚠️ Potential issueImplementation must mirror the trait change
- fn burn_amount(&self) -> u64 { + fn burn_amount(&self) -> TokenAmount { self.burn_amount } - fn set_burn_amount(&mut self, amount: u64) { + fn set_burn_amount(&mut self, amount: TokenAmount) { self.burn_amount = amount; }
🧹 Nitpick comments (6)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/v0/transformer.rs (2)
115-118: Pattern match is brittle – use an exhaustive match or explicit guardAt runtime corruption is turned into a
ProtocolError, but the pattern only matches one exact variant shape.
If the enum later gains additional fields (e.g.TokenEvent::Burn { … }style) this line will silently break at compile-time.
Consider an exhaustivematchorif let TokenEvent::Burn { burn_from_id, .. }to make the intent future-proof.
231-238: Duplicate extraction logic – extract into a small helperThe
burn_from_identifierextraction is duplicated in both constructors. A tiny private function would remove the repetition and the risk of future divergence.fn derive_burn_from(base: &TokenBaseTransitionAction, fallback: Identifier) -> Result<Identifier, Error> { … }packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/v0/mod.rs (1)
15-19: Field order: put new, small fields before large ones
Identifier(32 bytes) beforeTokenAmount(8 bytes) avoids padding and maintains the previous struct size layout. This is a minor but free optimisation.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs (3)
516-517: Incorrect comment in the assertion sectionThe comment on line 516 indicates "Original identity should have no tokens" but the assertion checks for
Some(98663)tokens remaining. This comment appears to be misleading.- assert_eq!(balance1, Some(98663)); // Original identity should have no tokens + assert_eq!(balance1, Some(98663)); // Original identity should have reduced balance (100000 - 1337)
517-518: Inconsistent comment in assertion sectionThe comment on line 517 incorrectly states "Recipient should not keep transferred tokens if burn was enforced" but the test is actually verifying that the recipient keeps the transferred tokens (which is the expected behavior).
- assert_eq!(balance2, Some(1337)); // Recipient should not keep transferred tokens if burn was enforced + assert_eq!(balance2, Some(1337)); // Recipient should keep transferred tokens even after burn is enforced
746-747: Misleading comment in validation sectionThe comment "Validate the burn still succeeded even though tokens were transferred" is incorrect since the burn actually failed with an error, as expected in this test case.
- // Validate the burn still succeeded even though tokens were transferred + // Validate balances remain unchanged after burn failed due to insufficient tokens
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (24)
packages/dapi-grpc/protos/platform/v0/platform.proto(1 hunks)packages/rs-dpp/src/group/group_action/mod.rs(2 hunks)packages/rs-dpp/src/group/group_action/v0/mod.rs(2 hunks)packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs(1 hunks)packages/rs-dpp/src/tokens/token_event.rs(5 hunks)packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs(1 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_burn_transition_action/state_v0/mod.rs(3 hunks)packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs(2 hunks)packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_burn_transition.rs(3 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_config_update_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_destroy_frozen_funds_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_emergency_action_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_freeze_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_mint_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_unfreeze_transition.rs(1 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_base_transition_action/v0/transformer.rs(2 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/mod.rs(2 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/v0/mod.rs(4 hunks)packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/v0/transformer.rs(3 hunks)packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs(1 hunks)packages/token-history-contract/schema/v1/token-history-contract-documents.json(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_burn_transition_action/state_v0/mod.rs (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GroupActionEvent(8224-8249)TokenEvent(8366-8416)
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (1)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (2)
GroupActionEvent(8224-8249)TokenEvent(8366-8416)
packages/rs-dpp/src/group/group_action/mod.rs (1)
packages/rs-dpp/src/group/group_action/v0/mod.rs (1)
proposer_id(25-27)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/mod.rs (1)
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/v0/mod.rs (4)
burn_from_identifier(32-32)burn_from_identifier(92-94)set_burn_from_identifier(35-35)set_burn_from_identifier(96-98)
packages/rs-dpp/src/group/group_action/v0/mod.rs (1)
packages/rs-dpp/src/group/group_action/mod.rs (2)
proposer_id(22-22)proposer_id(33-37)
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_burn_transition.rs (3)
packages/rs-dpp/src/group/group_action/v0/mod.rs (1)
event(33-35)packages/rs-dpp/src/group/group_action/mod.rs (2)
event(24-24)event(45-49)packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
TokenEvent(8366-8416)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (drive-abci) / Detect immutable structure changes
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (rs-dapi-client) / Detect immutable structure changes
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (rs-dapi-client) / Tests
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Rust packages (rs-dapi-client) / Unused dependencies
- GitHub Check: Rust packages (dapi-grpc) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (token-history-contract) / Linting
- GitHub Check: Rust packages (dapi-grpc) / Formatting
- GitHub Check: Rust packages (dapi-grpc) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (37)
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_freeze_transition.rs (1)
59-67: Correctly tracks proposer identity in group actionsThe addition of
proposer_id: owner_idin theGroupActionV0initialization ensures that the identity of the action proposer is explicitly tracked in group actions, which aligns with the broader changes introduced in this PR to fix identity attribution in token events.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_mint_transition.rs (1)
59-67: Properly associates group actions with their proposersThe addition of
proposer_id: owner_idcorrectly tracks which identity initiated the token mint action in a group context, enabling proper attribution in event history and ensuring consistency with other token-related operations.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_destroy_frozen_funds_transition.rs (1)
59-66: Correctly tracks proposer identity in destroy frozen funds eventAdding
proposer_id: owner_idensures proper attribution of the identity that initiated the action to destroy frozen funds, maintaining consistency with other token operations and supporting accurate historical tracking of token-related events.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_emergency_action_transition.rs (1)
59-66: Ensures proper proposer tracking for emergency actionsThe addition of
proposer_id: owner_idguarantees that emergency token actions correctly identify their initiator, which is particularly important for accountability in these sensitive operations and aligns with the PR's goal of fixing identity attribution.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs (1)
60-69: Correctly adds proposer_id for tracking burn identity originThis change adds the
proposer_idfield to theGroupActionV0struct when initializing the group action, which properly identifies the original proposer of the action rather than the last approver.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_unfreeze_transition.rs (1)
60-69: Consistent implementation of proposer_id trackingThis change correctly adds the
proposer_idfield toGroupActionV0, maintaining consistency with the pattern established in other token operations and properly supporting the fix for burn identity attribution.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_config_update_transition.rs (1)
61-70: Properly adds proposer identity trackingThe addition of
proposer_id: owner_idto theGroupActionV0struct maintains consistency with the changes in other token operations and ensures that the group action correctly tracks the identity of the proposing user.packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (1)
368-370: Correctly updates burn verification to handle new identity fieldThis change adds a case to ignore the new
burnFromIdandnotefields when comparing historical documents during burn token transition verification. This is necessary to maintain compatibility with the updated burn event structure that now tracks the identity from which tokens are burned.packages/rs-drive-abci/src/query/group_queries/group_actions/v0/mod.rs (1)
146-151: Updated TokenEvent::Burn to include burn_from_id parameterThe pattern matching for TokenEvent::Burn now correctly includes the burn_from_id parameter, which is properly serialized to the response. This change ensures that the identity of the token burner is properly tracked and exposed in the API response.
packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (1)
90-95: Added proposer_id and updated burn event parametersThe GroupActionV0 struct now properly includes the proposer_id field and the TokenEvent::Burn has been updated to include the burn_from_id parameter (IDENTITY_ID_1). This ensures that test data correctly reflects the new structure for tracking the identity from which tokens are burned.
packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_base_transition_action/v0/transformer.rs (2)
88-88: Variable name improved for clarityThe variable has been renamed from
current_group_action_eventtocurrent_group_action, which better reflects that it contains the entire group action object, not just the event.
192-192: Consistent variable renamingThe tuple return value has been updated to use the renamed variable, maintaining consistency with the earlier change.
packages/rs-dpp/src/group/group_action/mod.rs (2)
21-23: Added proposer_id accessor to GroupActionAccessors traitThis new accessor method properly extends the trait interface to provide access to the proposer_id field, which is necessary for tracking the identity that originated the group action.
33-37: Implemented proposer_id accessor for GroupActionThe implementation follows the established pattern for accessor methods in this enum, properly delegating to the inner variant's implementation. This ensures consistent access to the proposer_id across the codebase.
packages/rs-dpp/src/group/group_action/v0/mod.rs (2)
15-15: Adding theproposer_idfield improves clarity and tracking.The addition of this field explicitly tracks who initiated a group action, rather than just tracking approvers. This is particularly important for token burns to ensure the identity is correctly attributed.
25-27: Accessor method implementation for the new field follows proper patterns.The implementation correctly returns the field by reference, maintaining consistent accessor patterns with the rest of the struct.
packages/dapi-grpc/protos/platform/v0/platform.proto (1)
663-664: Schema update correctly adds burn identity tracking.The
BurnEventmessage now includes a requiredburn_from_idfield to explicitly track which identity's tokens are being burned, fixing the attribution issue in group actions. Thepublic_notefield is appropriately maintained but moved to field #3.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/action_validation/token/token_burn_transition_action/state_v0/mod.rs (3)
78-79: Updated pattern matching handles new event structure.The pattern match correctly updates to handle the additional
burn_from_idfield in theTokenEvent::Burnvariant. The underscore pattern appropriately ignores this field since it's not used in the validation logic at this point.
114-114: Token balance validation now checks against the correct identity.The balance check now uses
self.burn_from_identifier()instead ofowner_id, ensuring token balances are verified against the actual identity from which tokens are being burned, not just the action initiator.
126-126: Error reporting correctly references burn source identity.Error message construction is updated to use
self.burn_from_identifier(), ensuring error messages accurately identify which identity has insufficient tokens.packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs (1)
402-404: Token burn event construction includes burn source identity.The
TokenEvent::Burnconstructor now includesowner_idas a parameter to track which identity's tokens are being burned. The comments helpfully explain that this might need correction during group actions.This change is essential for proper tracking of token burning in the context of group actions, ensuring the burn is attributed to the right identity.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_transition.rs (1)
77-77: Good implementation of burn action attribution.The added parameter
burn_from_identifier()properly ensures that the token burn event now tracks the identity from which tokens are burned. This aligns with the PR objective to correctly attribute burn actions to their originator rather than the last approver.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_burn_transition_action/mod.rs (3)
2-2: LGTM - necessary import for the burn identifier.Adding the Identifier import is needed for the new burn_from_identifier accessor methods.
32-36: LGTM - proper accessor implementation for burn identifier.This getter method correctly retrieves the burn_from_identifier from the underlying TokenBurnTransitionActionV0 struct. The implementation matches the accessors already defined in the v0 module.
38-42: LGTM - proper setter implementation for burn identifier.This setter method correctly updates the burn_from_identifier on the underlying TokenBurnTransitionActionV0 struct. The implementation matches the accessors already defined in the v0 module.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_burn_transition.rs (4)
56-56: LGTM - correctly constructs burn event with burn_from identifier.This change correctly adds the burn_from_identifier to the TokenEvent::Burn construction for group actions, which is in line with the PR objective to properly track the identity from which tokens are burned.
61-61: Good addition of proposer_id to group action.Setting the proposer_id to owner_id in the GroupActionV0 initialization ensures proper tracking of who proposed the group action. This is essential for the correct attribution of burn actions.
83-83: Critical fix for burn attribution.Changed identity_balance_holder_id from owner_id to burn_from_identifier, which is the core fix for the issue. This ensures that tokens are burned from the correct identity as indicated by burn_from_identifier rather than the owner_id.
93-93: LGTM - consistent use of burn_from_identifier.This change correctly updates the TokenEvent::Burn constructor in the TokenHistory operation to include burn_from_identifier, maintaining consistency with the rest of the changes.
packages/rs-dpp/src/tokens/token_event.rs (5)
36-37: Good semantic type alias for clarity.The introduction of the BurnFromIdentifier type alias provides clear semantic meaning when used in the context of a token burn, improving code readability and maintainability.
71-73: LGTM - proper signature update for Burn event.The TokenEvent::Burn variant signature now correctly includes BurnFromIdentifier as the second parameter, which is essential for tracking the identity from which tokens are burned.
151-152: LGTM - updated Display implementation.The Display implementation for TokenEvent::Burn has been properly updated to include the burn_from_identifier in the formatted output.
226-226: LGTM - updated pattern matching for public note.Pattern matching in the public_note method has been correctly updated to accommodate the new burn_from_identifier parameter.
273-283: LGTM - comprehensive historical document update.The historical document creation for burn events now properly includes the burnFromId property, ensuring it's stored and queryable in the token history. All related handling is consistent with the updated TokenEvent::Burn signature.
packages/token-history-contract/schema/v1/token-history-contract-documents.json (1)
69-77: Position shift – verify compatibility with existing data
burnFromIdinserted at position 1 pushes subsequent fields. Make sure any client relying on positional ordering (e.g. sparse binary layouts or older SDKs) is updated simultaneously; otherwise decoding will be wrong.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/burn/mod.rs (2)
282-518: Well-structured test for token burn with group actions!This test thoroughly validates that token burn actions properly track the identity originating the burn even when tokens are transferred between the proposal and confirmation stages of a group action. The test correctly:
- Sets up a contract with group-based authorization
- Mints tokens to the first identity
- Initiates a burn as the proposer
- Transfers some tokens to a recipient
- Confirms the burn as the second group member
- Validates final balances reflect the correct burn attribution
This effectively tests the fix for ensuring burn identity is correctly attributed to the originator rather than the last approver.
521-759: Good edge case test for insufficient balance scenarios!This complementary test properly validates the error handling when a burn can't be completed due to insufficient balance after token transfers. It correctly:
- Initiates a burn as the proposer without first minting tokens
- Transfers tokens away, reducing available balance
- Attempts to confirm the burn, which appropriately fails with
IdentityDoesNotHaveEnoughTokenBalanceError- Verifies balances remain unchanged after the failed confirmation
The test ensures the system correctly handles cases where tokens are transferred between proposal and confirmation, causing the burn to fail due to insufficient balance.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-0acdeecd6241d2cdec7ff61f91adb6f84188c97ff5d6f33f2225522b6f666be8.json(1 hunks)packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-4308e13f7335a56c12a2fd4a2e64670383282934865ccc3241c0f7fedf218853.json(0 hunks)packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-0acdeecd6241d2cdec7ff61f91adb6f84188c97ff5d6f33f2225522b6f666be8.json(1 hunks)packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-101a4299cad9e06288ea7a290a730a1c696ab53d03e0b241f35f9d002eee3233.json(0 hunks)
💤 Files with no reviewable changes (2)
- packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-4308e13f7335a56c12a2fd4a2e64670383282934865ccc3241c0f7fedf218853.json
- packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-101a4299cad9e06288ea7a290a730a1c696ab53d03e0b241f35f9d002eee3233.json
🧰 Additional context used
🪛 Biome (1.9.4)
packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-0acdeecd6241d2cdec7ff61f91adb6f84188c97ff5d6f33f2225522b6f666be8.json
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-0acdeecd6241d2cdec7ff61f91adb6f84188c97ff5d6f33f2225522b6f666be8.json
[error] 1-1: String values must be double quoted.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (dapi-grpc) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive-abci) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
QuantumExplorer
changed the title
…hpay/platform into branch-fixgroup-action-burn-identity
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/token-history-contract/test/unit/tokenHistoryContract.spec.js (1)
47-56: Consider adding specific validation tests for burnFromId field.The test suite includes validation tests for other required fields like
tokenIdandamount, but is missing similar tests for the newly addedburnFromIdfield. Adding dedicated tests would ensure the field is properly validated.You could add a new test section like:
describe('burnFromId', () => { it('should be defined', async () => { delete rawBurnDocument.burnFromId; const document = dpp.document.create(dataContract, identityId, 'burn', rawBurnDocument); const validationResult = document.validate(dpp.protocolVersion); const error = expectJsonSchemaError(validationResult); expect(error.keyword).to.equal('required'); expect(error.params.missingProperty).to.equal('burnFromId'); }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs(1 hunks)packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs(2 hunks)packages/token-history-contract/test/unit/tokenHistoryContract.spec.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs
- packages/rs-dpp/src/state_transition/state_transitions/document/batch_transition/batched_transition/token_transition.rs
⏰ Context from checks skipped due to timeout of 90000ms (20)
- GitHub Check: Rust packages (drive-abci) / Detect immutable structure changes
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Rust packages (drive) / Formatting
- GitHub Check: Rust packages (drive) / Linting
- GitHub Check: Rust packages (drive) / Tests
- GitHub Check: Rust packages (drive) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (wasm-dpp) / Tests
- GitHub Check: Rust packages (token-history-contract) / Unused dependencies
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (2)
packages/token-history-contract/test/unit/tokenHistoryContract.spec.js (2)
52-52: Addition of burnFromId field aligns with the PR objectives.This change correctly adds the
burnFromIdfield to the test document, which is now required according to the updated schema. This addition supports the fix for group action burns to correctly track the identity from which tokens are burned, rather than using the last person who approved the action.
100-104: Verify existing test effectiveness with new field requirement.This test now implicitly validates that a document with the
burnFromIdfield is valid, which is good. The test passes with the new field added, confirming the schema update is working correctly.
2 participants
Issue being fixed or feature implemented
Group action burns were incorrectly using the last person who approved the group action as the burn identity instead of the originator.
What was done?
BurnEventmessage to includeburn_from_idas a required field.TokenEvent::Burnto acceptburn_from_identifierand adjusted related methods and structures accordingly.TokenBurnTransitionActionto set the correctburn_from_identifierbased on the group action originator.How Has This Been Tested?
Breaking Changes
None
Checklist
For repository code-owners and collaborators only
Summary by CodeRabbit