feat(platform): group action proofs #2605
Conversation
|
Warning Rate limit exceeded@QuantumExplorer has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 25 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis set of changes introduces comprehensive enhancements and new features for group action management within the platform. Key updates include distinguishing between active and closed group actions, tracking and verifying group action signers' total power, supporting batch move operations, and extending error handling and query capabilities. Numerous structs, enums, and method signatures are updated to accommodate new fields and parameters, with corresponding adjustments in tests and platform versioning. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Drive
participant GroveDB
Client->>Drive: add_group_action(..., closes_group_action, ...)
alt closes_group_action == false
Drive->>GroveDB: Insert/ensure active group action tree
Drive->>GroveDB: Insert/update signer sum tree
Drive->>GroveDB: Insert/update signer power with flags
else closes_group_action == true
Drive->>GroveDB: Insert closed group action tree
Drive->>GroveDB: Insert signer sum tree (closed)
Drive->>GroveDB: Insert signer power (no flags)
Drive->>GroveDB: Move signer entries from active to closed
Drive->>GroveDB: Move or insert action info to closed tree
Drive->>GroveDB: Delete active signer sums and action entries
end
Drive-->>Client: FeeResult or error
sequenceDiagram
participant Verifier
participant Drive
participant GroveDB
Verifier->>Drive: verify_action_signer_and_total_power(proof, ...)
Drive->>GroveDB: Query proof for active/closed group action state
alt Exactly one state present
Drive->>GroveDB: Query for signer's power in group action
Drive->>Verifier: Return (root_hash, action_status, sum_power)
else Both or neither state present
Drive-->>Verifier: Error (proof invalid or ambiguous)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ 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://waps.l3s.uni-hannover.de/live/im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (24)
packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-101a4299cad9e06288ea7a290a730a1c696ab53d03e0b241f35f9d002eee3233.json (1)
1-1: Format JSON string value with double quotesThe static analysis tool correctly flagged that this JSON file contains an unquoted string value. JSON requires string values to be enclosed in double quotes.
-aa8fcaad83dc96339d97ce135e408ca1eb5f7392ade678e224856b394b6370b2fa522a002acb149020e17d3bbbe6444e +"aa8fcaad83dc96339d97ce135e408ca1eb5f7392ade678e224856b394b6370b2fa522a002acb149020e17d3bbbe6444e"🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: String values must be double quoted.
(parse)
packages/rs-drive/src/drive/group/fetch/fetch_active_action_info/v0/mod.rs (1)
27-27: Refine comment to reflect active action context
The generic comment above path construction could mention "active" to improve clarity:- // Construct the GroveDB path for the action signers + // Construct the GroveDB path for the active action infopackages/rs-drive/src/util/grove_operations/mod.rs (1)
217-219: Update to BatchMoveApplyType makes sense but contains a grammatical errorMaking
tree_typeoptional allows for more flexibility in batch move operations. However, the comment "Are we moving a trees?" contains a grammatical error - it should be either "Are we moving a tree?" or "Are we moving trees?".- /// Are we moving a trees? + /// Are we moving trees?packages/rs-drive/src/fees/op.rs (1)
6-6: Version-aware fee refund handling improves backward compatibilityThis change implements a version-specific approach to fee refunds, using an empty BTreeMap for fee version 1 and the previous behavior for other versions. This provides a clean way to handle version differences while maintaining backward compatibility.
The conditional approach is appropriate, but consider adding a brief comment explaining why fee version 1 is handled differently to improve future maintainability.
if fee_version.fee_version_number == 1 { + // For fee version 1, we use an empty map as we don't need previous fee versions ( FeeRefunds::from_storage_removal( removal_per_epoch_by_identifier,Also applies to: 247-268
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_destroy_frozen_funds_transition.rs (1)
58-60: Prefer re-using the already-cloneddata_contract_idto avoid an extra allocation
self.base().data_contract_id()is already cloned intodata_contract_id(line 33).
Calling it again here performs a second clone/allocation of the 32-byte buffer every time the proposer path is taken.- contract_id: self.base().data_contract_id(), + contract_id: data_contract_id,A micro-optimisation, but this code runs for every destroy-frozen-funds transition in a batch.
packages/rs-drive/src/drive/group/estimated_costs/for_add_group_action/mod.rs (1)
50-57: Missing docstring for the newalso_add_closed_treeparameterThe public comment block explains every parameter except the newly-added
also_add_closed_tree.
Readers have to dive intov0to understand the flag’s purpose.Add a short bullet such as:
/// - `also_add_closed_tree`: if `true`, cost estimation also covers the closed-action subtreeto keep the API self-documenting.
packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs (2)
44-65:create_token_historical_document_queryis now orphanedSince the token-transition branch delegates to
try_transition_into_path_query_with_contract, the helpercreate_token_historical_document_query(lines 44-65) is no longer referenced inside this module.If the function is truly obsolete, please delete it to avoid dead-code warnings (several crates use
#![deny(dead_code)]).
Otherwise, add a comment or#[allow(dead_code)]explaining the future use.
174-178: Surface trait-level errors for easier debugging
try_transition_into_path_query_with_contractis forwarded with?, which will wrap itsError::Drive/Error::Proofvariants indistinguishably from surrounding logic. Consider mapping the error to include the transition’s id for context:token_transition .try_transition_into_path_query_with_contract(contract, owner_id, platform_version) .map_err(|e| e.into_additional_context(format!("while building query for token {}", token_transition.token_id())))?Not mandatory, but greatly simplifies troubleshooting complex batch proofs.
packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/v0/mod.rs (1)
44-44: Consider adding a comment about the active root checkThe implementation currently only checks if the action exists in the closed root. If you intend to keep this implementation, it would be helpful to add a comment explaining why the active root check is not performed or when it would be performed.
- Ok(false) + // Return false if not found in closed root, as we only care about closed status + Ok(false)packages/rs-drive/src/util/grove_operations/batch_move/mod.rs (1)
15-15: Consider adding more detailed documentationThe current comment is brief. Since this is a new public API, it would be helpful to add more details about what the batch move operation does, especially explaining the purpose of the
alter_flags_to_new_flagsparameter and theBatchMoveApplyTypeenum.packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs (1)
21-21: Improve parameter documentationThe documentation for the new parameter is quite brief. Consider enhancing it to explain the possible values and their effects.
- /// * `alter_flags_to_new_flags`: Should we alter flags of items to new flags + /// * `alter_flags_to_new_flags`: Controls flag modification during the move: + /// - `None`: Preserve existing flags + /// - `Some(None)`: Remove all flags + /// - `Some(Some(flags))`: Set specific flagspackages/rs-drive/src/drive/group/estimated_costs/for_add_group_action/v0/mod.rs (1)
36-44: Doc-comment is stale – newalso_add_closed_treeparameter is undocumentedThe parameter list in the docstring has not been updated after introducing the
also_add_closed_tree: boolargument. Readers of the public API (and future
maintainers) will miss this important behaviour switch.@@ - /// - `action_id`: An optional identifier ... - /// will be inserted. The keys represent paths to the trees, and the values represent their estimated layer information. + /// - `action_id`: An optional identifier for the specific action being added. + /// - `also_add_closed_tree`: When `true`, the same set of cost estimates is + /// added for the *closed*-action sub-trees. + /// - `estimated_costs_only_with_layer_info`: ...packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs (2)
21-30: Missing explanation of the newalter_flags_to_new_flagsparameterThe doc-comment enumerates the function parameters but the freshly added
alter_flags_to_new_flagsis only mentioned in a brief bullet without
describing its semantics (e.g.Some(None)strips flags,Some(Some(flags))
replaces them,Nonekeeps them).
124-132: Order-of-operations may cause unintended double charging of sum-tree updates
element.set_flags(..)is executed after the delete operation has been
generated but before the insert op is queued. That is fine for correctness,
yet it means the replacement item may carry a different flag length (thereby
different byte-size) than the element that was removed. GroveDB’s fee logic
usually assumes delete/insert symmetry; if flag size changes, fee estimation
may become inaccurate.Two safer alternatives:
- Perform the flag mutation before computing
delete_operationso the fee
calculator sees symmetrical sizes, or- When flags change, explicitly add a fee refund/charge delta.
packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/token_transition.rs (1)
163-178: Query merge may introduce duplicate paths leading to oversize proofs
PathQuery::mergeconcatenates the two queries; because both queries refer to
the same contract subtree they share a large common prefix. The resulting
proof could include redundant nodes and bloat proof sizes.A small optimisation is to build one
Querywith the union of keys instead
of merging two independentPathQuerys.packages/rs-drive/src/util/grove_operations/batch_delete/v0/mod.rs (1)
53-55: Redundant batch calculation
LowLevelDriveOperation::grovedb_operations_batch(drive_operations)is
performed before any new operations are added, yet the result is passed to
delete_operation_for_delete_internal, which again inspects the batch.
This duplicate conversion costs time on large batches.Store the
GroveDbOpBatchonce in the outer scope of the caller and pass a
reference, or push the current delete op after the delete-internal call so
it isn’t converted twice.packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs (2)
29-47: Stateless move still performs a DB readWhen
apply_typeisStatelessBatchMovethe caller promises to supply size
estimates, so performing a fullgrove_get()defeats the purpose and becomes a
hot-path read.Consider:
- Skipping the read for stateless mode and letting the caller provide the
element (or its size) up-front, or- Downgrading
StatelessBatchMovetoStatefulinternally if the element is
really needed.This will cut a network/local-SSD round-trip in many batch scenarios.
90-99: Flag alteration mutates the original element
element.set_flags()modifies the in-memoryElementthat was fetched from the
database, which may have shared ownership with caches.
Cloning before mutation avoids surprising aliasing:- if let Some(flags) = alter_flags_to_new_flags.as_ref() { - element.set_flags(StorageFlags::map_to_some_element_flags(flags.as_ref())); - } + if let Some(flags) = alter_flags_to_new_flags.as_ref() { + let mut new_el = element.clone(); + new_el.set_flags(StorageFlags::map_to_some_element_flags(flags.as_ref())); + element = new_el; + }packages/rs-drive/src/drive/group/queries.rs (2)
94-121:group_action_querynaming collides with existinggroup_action_signers_queryBoth functions return signer-related data, yet one is called
group_action_queryand the othergroup_action_signers_query. The naming
does not convey their semantic difference (single key vs full range) and will
confuse future maintainers.Consider:
- Renaming
group_action_query→group_action_signers_root_query
(or similar)- Adding doc-comments that explicitly state the returned path & key.
195-224: Function name suggests “active or closed” but still requires anaction_statusargument
group_active_or_closed_action_single_signer_queryostensibly hides the active
/ closed distinction, yet the caller must passaction_status. Either:
- Remove the parameter and probe both sub-trees internally, or
- Rename to
group_action_single_signer_queryand drop the “active_or_closed”
qualifier.The current API invites misuse and double-checks by reviewers.
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/batch/tests/token/freeze/mod.rs (1)
1-667: High value tests, but duplication is skyrocketingThe three basic freeze/unfreeze tests and the group freeze scenarios repeat
large swaths of setup code (platform boot-strap, identity creation, contract
template tweaking, transaction execution boiler-plate).Extract helpers (e.g.
fn setup_platform(),fn exec_transition(platform, tx_bytes))
or use Rust test fixtures (#[fixture]withrstest) to cut ~300 lines.
This keeps individual test intent clear and shortens compile time.packages/rs-drive/src/drive/group/insert/add_group_action/mod.rs (3)
21-35: Consider updating method documentationWhile the implementation is correct, the method documentation should be updated to explain the purpose of the new
closes_group_actionparameter for better code maintainability./// Adds an action to the state +/// +/// When `closes_group_action` is true, the action is marked as closed, finalizing its state +/// and moving relevant data from active to closed trees. #[allow(clippy::too_many_arguments)]
58-74: Consider updating documentation for operation building methodSimilar to the main method, the documentation for this method should include information about the new parameter.
/// Adds action creation operations to drive operations +/// +/// When `closes_group_action` is true, operations for closing the group action will be included, +/// handling the transition from active to closed state. #[allow(clippy::too_many_arguments)]
97-114: Consider updating documentation for operations methodThe operations method would also benefit from documentation explaining the new parameter.
/// The operations needed to create a new group action +/// +/// When `closes_group_action` is true, includes operations for closing the group action +/// and managing the transition between active and closed states. #[allow(clippy::too_many_arguments)]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
packages/rs-dpp/src/data_contract/group/v0/mod.rs(1 hunks)packages/rs-dpp/src/group/group_action/mod.rs(2 hunks)packages/rs-dpp/src/group/group_action/v0/mod.rs(1 hunks)packages/rs-dpp/src/group/group_action_status.rs(1 hunks)packages/rs-dpp/src/state_transition/proof_result.rs(2 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/tests/token/freeze/mod.rs(1 hunks)packages/rs-drive/Cargo.toml(1 hunks)packages/rs-drive/src/drive/group/estimated_costs/for_add_group_action/mod.rs(2 hunks)packages/rs-drive/src/drive/group/estimated_costs/for_add_group_action/v0/mod.rs(4 hunks)packages/rs-drive/src/drive/group/fetch/fetch_action_id_info_keep_serialized/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/group/fetch/fetch_action_id_signers_power/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/mod.rs(1 hunks)packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/v0/mod.rs(1 hunks)packages/rs-drive/src/drive/group/fetch/fetch_active_action_info/v0/mod.rs(3 hunks)packages/rs-drive/src/drive/group/fetch/mod.rs(1 hunks)packages/rs-drive/src/drive/group/insert/add_group_action/mod.rs(6 hunks)packages/rs-drive/src/drive/group/insert/add_group_action/v0/mod.rs(7 hunks)packages/rs-drive/src/drive/group/paths.rs(2 hunks)packages/rs-drive/src/drive/group/prove/prove_action_infos/v0/mod.rs(9 hunks)packages/rs-drive/src/drive/group/prove/prove_action_signers/v0/mod.rs(8 hunks)packages/rs-drive/src/drive/group/queries.rs(3 hunks)packages/rs-drive/src/error/drive.rs(1 hunks)packages/rs-drive/src/fees/op.rs(2 hunks)packages/rs-drive/src/lib.rs(1 hunks)packages/rs-drive/src/prove/mod.rs(1 hunks)packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs(3 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_burn_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_config_update_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_destroy_frozen_funds_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_emergency_action_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_freeze_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_mint_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs(2 hunks)packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_unfreeze_transition.rs(2 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/util/batch/drive_op_batch/group.rs(2 hunks)packages/rs-drive/src/util/batch/drive_op_batch/withdrawals.rs(2 hunks)packages/rs-drive/src/util/grove_operations/batch_delete/mod.rs(2 hunks)packages/rs-drive/src/util/grove_operations/batch_delete/v0/mod.rs(2 hunks)packages/rs-drive/src/util/grove_operations/batch_move/mod.rs(1 hunks)packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs(1 hunks)packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs(4 hunks)packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs(8 hunks)packages/rs-drive/src/util/grove_operations/mod.rs(2 hunks)packages/rs-drive/src/verify/group/mod.rs(1 hunks)packages/rs-drive/src/verify/group/verify_action_signers_total_power/mod.rs(1 hunks)packages/rs-drive/src/verify/group/verify_action_signers_total_power/v0/mod.rs(1 hunks)packages/rs-drive/src/verify/state_transition/mod.rs(1 hunks)packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/mod.rs(1 hunks)packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/token_transition.rs(1 hunks)packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs(10 hunks)packages/rs-platform-version/Cargo.toml(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_group_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_group_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs(1 hunks)packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs(1 hunks)packages/rs-platform-version/src/version/system_limits/mod.rs(1 hunks)packages/rs-sdk/tests/fetch/group_actions.rs(2 hunks)packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-1f0a25d463a2912cd31dd4f91b899a143d6ae990bb9bc89cb34f7c5db8b1b705.json(0 hunks)packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-4308e13f7335a56c12a2fd4a2e64670383282934865ccc3241c0f7fedf218853.json(1 hunks)packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-101a4299cad9e06288ea7a290a730a1c696ab53d03e0b241f35f9d002eee3233.json(1 hunks)packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-1f0a25d463a2912cd31dd4f91b899a143d6ae990bb9bc89cb34f7c5db8b1b705.json(0 hunks)
💤 Files with no reviewable changes (2)
- packages/rs-sdk/tests/vectors/test_fetch_one_group_action_since_existing_one_with_limit/quorum_pubkey-106-1f0a25d463a2912cd31dd4f91b899a143d6ae990bb9bc89cb34f7c5db8b1b705.json
- packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-1f0a25d463a2912cd31dd4f91b899a143d6ae990bb9bc89cb34f7c5db8b1b705.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: QuantumExplorer
PR: dashpay/platform#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.
packages/rs-platform-version/Cargo.toml (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
packages/rs-drive/Cargo.toml (1)
Learnt from: QuantumExplorer
PR: dashpay/platform#2431
File: packages/rs-drive/Cargo.toml:55-60
Timestamp: 2025-01-19T07:36:46.042Z
Learning: The grovedb dependencies in packages/rs-drive/Cargo.toml and related files are intentionally kept at specific revisions rather than using the latest stable version, with plans to update them at a later time.
packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (2)
Learnt from: lklimek
PR: dashpay/platform#2207
File: packages/rs-drive-proof-verifier/src/proof.rs:1646-1664
Timestamp: 2024-10-04T14:16:05.798Z
Learning: In the implementation of `FromProof<platform::GetContestedResourceIdentityVotesRequest>` in `packages/rs-drive-proof-verifier/src/proof.rs`, when matching `maybe_votes`, using `.expect()` on `v.into_iter().next()` is acceptable because the prior match arm `Some(v) if v.is_empty()` ensures that the map is not empty, preventing a panic.
Learnt from: lklimek
PR: dashpay/platform#2207
File: packages/rs-drive-proof-verifier/src/proof.rs:1646-1664
Timestamp: 2024-10-09T00:22:57.778Z
Learning: In the implementation of `FromProof<platform::GetContestedResourceIdentityVotesRequest>` in `packages/rs-drive-proof-verifier/src/proof.rs`, when matching `maybe_votes`, using `.expect()` on `v.into_iter().next()` is acceptable because the prior match arm `Some(v) if v.is_empty()` ensures that the map is not empty, preventing a panic.
🧬 Code Graph Analysis (12)
packages/rs-drive/src/drive/group/fetch/mod.rs (1)
packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/mod.rs (1)
fetch_action_is_closed(18-48)
packages/rs-drive/src/fees/op.rs (1)
packages/rs-dpp/src/fee/fee_result/refunds.rs (1)
from_storage_removal(37-78)
packages/rs-drive/src/drive/group/fetch/fetch_action_id_info_keep_serialized/v0/mod.rs (1)
packages/rs-drive/src/drive/group/paths.rs (1)
group_active_action_path(91-103)
packages/rs-drive/src/drive/group/fetch/fetch_action_id_signers_power/v0/mod.rs (1)
packages/rs-drive/src/drive/group/paths.rs (1)
group_active_action_path(91-103)
packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/token_transition.rs (9)
packages/rs-dpp/src/system_data_contracts.rs (1)
load_system_data_contract(42-49)packages/rs-drive/src/prove/prove_state_transition/v0/mod.rs (1)
create_token_historical_document_query(44-64)packages/rs-dpp/src/group/group_action/mod.rs (2)
contract_id(20-20)contract_id(25-29)packages/rs-dpp/src/group/group_action/v0/mod.rs (1)
contract_id(20-22)packages/rs-drive/src/drive/group/queries.rs (1)
group_active_and_closed_action_single_signer_query(249-275)packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/mod.rs (1)
try_transition_into_path_query_with_contract(42-47)packages/rs-drive/src/drive/tokens/balance/queries.rs (2)
token_balance_for_identity_id_query(12-20)token_balances_for_identity_ids_query(23-38)packages/rs-drive/src/drive/tokens/info/queries.rs (1)
token_info_for_identity_id_query(11-19)packages/rs-drive/src/drive/tokens/direct_purchase/queries.rs (1)
token_direct_purchase_price_query(24-32)
packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/mod.rs (1)
packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/token_transition.rs (1)
try_transition_into_path_query_with_contract(69-184)
packages/rs-dpp/src/group/group_action/mod.rs (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
GroupActionEvent(8224-8249)packages/rs-dpp/src/group/group_action/v0/mod.rs (3)
contract_id(20-22)token_contract_position(24-26)event(28-30)
packages/rs-dpp/src/group/group_action/v0/mod.rs (2)
packages/dapi-grpc/clients/platform/v0/web/platform_pb.d.ts (1)
GroupActionEvent(8224-8249)packages/rs-dpp/src/group/group_action/mod.rs (6)
contract_id(20-20)contract_id(25-29)token_contract_position(21-21)token_contract_position(31-35)event(22-22)event(37-41)
packages/rs-drive/src/verify/group/verify_action_signers_total_power/mod.rs (1)
packages/rs-drive/src/verify/group/verify_action_signers_total_power/v0/mod.rs (1)
verify_action_signers_total_power_v0(18-132)
packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/v0/mod.rs (2)
packages/rs-drive/src/drive/group/paths.rs (1)
group_closed_action_root_path(153-163)packages/rs-dpp/src/errors/consensus/state/token/new_authorized_action_taker_group_does_not_exist_error.rs (1)
group_contract_position(25-27)
packages/rs-drive/src/verify/group/verify_action_signers_total_power/v0/mod.rs (1)
packages/rs-drive/src/drive/group/queries.rs (2)
group_active_or_closed_action_query(227-246)group_active_or_closed_action_single_signer_query(196-224)
packages/rs-drive/src/util/grove_operations/batch_delete/v0/mod.rs (1)
packages/rs-drive/src/fees/op.rs (1)
grovedb_operations_batch(310-321)
🪛 Biome (1.9.4)
packages/rs-sdk/tests/vectors/test_fetch_all_group_actions/quorum_pubkey-106-4308e13f7335a56c12a2fd4a2e64670383282934865ccc3241c0f7fedf218853.json
[error] 1-1: Missing exponent
Expected a digit as the exponent
(parse)
[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-101a4299cad9e06288ea7a290a730a1c696ab53d03e0b241f35f9d002eee3233.json
[error] 1-1: String values must be double quoted.
(parse)
⏰ Context from checks skipped due to timeout of 90000ms (19)
- GitHub Check: Rust packages (wasm-dpp) / Unused dependencies
- GitHub Check: Rust packages (wasm-dpp) / Linting
- GitHub Check: Rust packages (dpp) / Check each feature
- GitHub Check: Rust packages (dpp) / Tests
- GitHub Check: Rust packages (dpp) / Linting
- GitHub Check: Rust packages (dpp) / Unused dependencies
- GitHub Check: Rust packages (dpp) / Formatting
- GitHub Check: Rust packages (dash-sdk) / Linting
- GitHub Check: Rust packages (dash-sdk) / Unused dependencies
- GitHub Check: Rust packages (dash-sdk) / Tests
- GitHub Check: Rust packages (dash-sdk) / Check each feature
- GitHub Check: Rust packages (rs-dapi-client) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Check each feature
- GitHub Check: Rust packages (drive-abci) / Tests
- GitHub Check: Rust packages (drive-abci) / Linting
- GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
- GitHub Check: Build Docker images (DAPI, dapi, dapi) / Build DAPI image
- GitHub Check: Build Docker images (Drive, drive, drive-abci) / Build Drive image
- GitHub Check: Build JS packages / Build JS
🔇 Additional comments (103)
packages/rs-drive/Cargo.toml (1)
55-60: Align Grovedb dependencies to new Git revisionThe
grovedb,grovedb-costs,grovedb-path,grovedb-storage,grovedb-version, andgrovedb-epoch-based-storage-flagscrates are now consistently pinned to commit221ca6ec6b8d2b192d334192bd5a7b2cab5b0aa8. This matches the intended GroveDB update for enhanced group action proofs and follows the existing strategy of locking to specific revisions.packages/rs-platform-version/Cargo.toml (1)
14-14: Updategrovedb-versionto match GroveDB revisionThe
grovedb-versiondependency has been updated to rev221ca6ec6b8d2b192d334192bd5a7b2cab5b0aa8, aligning it with the changes in thers-drivepackage and ensuring version consistency across the workspace.packages/rs-drive/src/drive/group/fetch/mod.rs (1)
1-10: New module declaration looks correct.
Thefetch_action_is_closedsubmodule is added in the correct alphabetical grouping and matches the file pathfetch_action_is_closed/mod.rs.packages/rs-platform-version/src/version/system_limits/mod.rs (1)
12-12:Details
❓ Verification inconclusive
Verify type change safety for
max_contract_group_size.
Changing fromu32tou16may truncate values if the JSON defaults exceedu16::MAX. Please ensure the default version JSON and any runtime configurations use values ≤ 65535.Run this script to confirm the JSON spec values:
🏁 Script executed:
#!/bin/bash # Find the default v1 version defining system limits version_file=$(fd -t f 'v1.json' packages/rs-platform-version/src/version | head -n1) echo "Checking max_contract_group_size in $version_file" jq '.drive.system_limits.max_contract_group_size' "$version_file"Length of output: 282
I couldn’t locate
v1.jsondirectly—let’s surface all JSON specs undersrc/versionand extract the field:This will confirm which version files define
max_contract_group_sizeand their values.
🏁 Script executed:
#!/bin/bash set -e # List all JSON spec files echo "JSON spec files under src/version:" find packages/rs-platform-version/src/version -type f -name '*.json' || echo " (none found)" # For each spec, report max_contract_group_size (or note if missing) echo for file in $(find packages/rs-platform-version/src/version -type f -name '*.json'); do echo "Checking $file:" jq '.drive.system_limits.max_contract_group_size // "not defined"' "$file" echo doneLength of output: 238
Let’s search the entire repo for any JSON specs and where
max_contract_group_sizeis mentioned:#!/bin/bash set -e echo "🔍 Looking for any v1.json anywhere in the repo:" fd -t f 'v1.json' || echo " → v1.json not found" echo echo "🔍 Listing all .json files:" find . -type f -name '*.json' || echo " → No JSON files found" echo echo "🔍 Grepping for max_contract_group_size occurrences:" rg 'max_contract_group_size' -n . || echo " → No occurrences found" echo echo "🔍 Grepping for system_limits occurrences:" rg 'system_limits' -n . || echo " → No occurrences found"
Let’s inspect the Rust defaults for
SystemLimitsto see what valuemax_contract_group_sizegets when no JSON spec is present:#!/bin/bash set -e echo "🔍 struct SystemLimits definition:" rg -n "struct SystemLimits" -n . echo echo "🔍 Default implementation for SystemLimits:" files=$(rg -l "impl Default for SystemLimits" -n . || true) if [ -z "$files" ]; then echo "⚠️ No Default impl found for SystemLimits" else for file in $files; do echo echo "=== $file ===" sed -n '/impl Default for SystemLimits/,/}/p' "$file" done fi echo echo "🔍 All occurrences of max_contract_group_size:" rg -n "max_contract_group_size" -n .
Verify JSON defaults and code defaults for
max_contract_group_size
I wasn’t able to locate any JSON spec files underpackages/rs-platform-version/src/version. Please ensure that all version spec JSON (e.g.v1.json) define"drive": { "system_limits": { "max_contract_group_size": … } }with values ≤
u16::MAX(65535). If your specs live elsewhere or are generated, double-check their definitions. Also inspect theimpl Default for SystemLimitsinmod.rsto confirm it initializesmax_contract_group_sizewithin theu16range.• Verify JSON spec(s) define
drive.system_limits.max_contract_group_sizeand that it never exceeds 65535.
• Examine theDefaultimplementation forSystemLimitsto ensure its fallback value fits inu16.packages/rs-drive/src/verify/group/mod.rs (1)
1-5: New verification module added.
Theverify_action_signers_total_powersubmodule is correctly declared alongside existing modules.packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/mod.rs (1)
60-60:Details
✅ Verification successful
Ensure version definitions include
batch_move.
The newbatch_move: FeatureVersionfield is added to the struct. Confirm that the corresponding v1 version definitions (e.g., JSON orv1.rs) have been updated to include this feature, otherwise deserialization will break.
🏁 Script executed:
#!/bin/bash # Verify v1 definitions contain "batch_move" rg -n '"batch_move"' -n packages/rs-platform-version/src/version/drive_versionsLength of output: 81
🏁 Script executed:
#!/bin/bash # List Rust files in drive_grove_method_versions find packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions -maxdepth 1 -type f -name "*.rs" # Show the first 200 lines of v1.rs to check for batch_move if [[ -f packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/v1.rs ]]; then sed -n '1,200p' packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/v1.rs else echo "v1.rs not found" fiLength of output: 2877
Resolved:
batch_movepresent in v1 definitions
Thebatch_movefield is already defined inpackages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/v1.rswithinDriveGroveBatchMethodVersions, so no further changes are needed.packages/rs-platform-version/src/version/drive_versions/drive_group_method_versions/mod.rs (1)
23-23: Appropriate addition of feature version fieldThe addition of
fetch_action_is_closedto theDriveGroupFetchMethodVersionsstruct aligns well with the PR's goal of improving the group action proof system. This field will enable distinguishing between active and closed group actions.packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/mod.rs (1)
55-55: Good addition of verification feature fieldThe new
verify_action_signers_total_powerfield is a necessary extension to support verification of the total signing power in group actions. This aligns with the PR's objective of enhancing the proof system for group actions.packages/rs-dpp/src/data_contract/group/v0/mod.rs (1)
91-91: Explicit type casting improves code clarityThe explicit cast to
u32makes the code more type-safe and clearer. This is a good practice, especially when dealing with numeric comparisons in validation logic.packages/rs-drive/src/prove/mod.rs (1)
3-4: Clean gate for server-specific functionalityThe conditional compilation flag ensures the
prove_state_transitionmodule is only included when the "server" feature is enabled, which aligns with the architectural separation of concerns in the platform. This matches the pattern used for other server-specific modules.packages/rs-platform-version/src/version/drive_versions/drive_grove_method_versions/v1.rs (1)
51-51: Version registration for new batch_move operationThe addition of
batch_move: 0properly registers the new batch move operation in the versioning system. This ensures backward compatibility while enabling the new functionality needed for improved group action proofs.packages/rs-drive-abci/src/execution/platform_events/initialization/create_genesis_state/test/tokens.rs (2)
90-94: Enhanced group action metadata for token eventsYou've added contract ID and token position metadata to the GroupActionV0 structure. This is essential for the improved proof system, as it ensures that group actions can be properly associated with their originating contracts and token positions.
96-108: Added support for active/closed group action distinctionThe new
falseparameter indicates that this action doesn't close a group action. This distinction between active and closed group actions enhances the proof system's ability to track the lifecycle of group actions.packages/rs-dpp/src/group/group_action_status.rs (1)
3-3: Added Copy trait to improve ergonomicsAdding the
Copytrait to GroupActionStatus is appropriate as it's a small, stack-allocated enum. This improves ergonomics by allowing implicit copying rather than requiring explicit cloning, which aligns with Rust best practices for small value types.packages/rs-platform-version/src/version/drive_versions/drive_verify_method_versions/v1.rs (1)
39-39:Details
✅ Verification successful
Add new
verify_action_signers_total_powermethod version
The fieldverify_action_signers_total_power: 0has been added toDriveVerifyGroupMethodVersions. Ensure that the corresponding dispatch logic in thers-drivecrate maps version0to the new implementation (verify_action_signers_total_power_v0).Run the following to verify references:
🏁 Script executed:
#!/bin/bash # Search for all occurrences to confirm dispatch mappings exist rg "verify_action_signers_total_power" -n packages/rs-driveLength of output: 931
Dispatch mapping confirmed for
verify_action_signers_total_power
The dispatch logic inpackages/rs-drive/src/verify/group/verify_action_signers_total_power/mod.rsalready maps version0toverify_action_signers_total_power_v0. No further changes are required.packages/rs-drive/src/lib.rs (1)
58-59: Broadenverifymodule feature gating
Theverifymodule is now correctly gated by bothserverandverifyfeatures, matching other modules likedriveandquery. This ensures proof verification functions are available when theverifyfeature is enabled.packages/rs-drive/src/verify/state_transition/mod.rs (2)
1-3: Add module-level documentation
The new doc comment clearly explains the purpose ofverify_state_transition_was_executed_with_proof, improving readability and compliance with#![deny(missing_docs)].
4-6: Add module-level documentation
The doc comment forstate_transition_execution_path_queriesclarifies its role in constructing path queries for state transition verification. Good for maintainability.packages/rs-platform-version/src/version/drive_versions/drive_group_method_versions/v1.rs (1)
16-16:Details
✅ Verification successful
Add
fetch_action_is_closedfetch method version
The fieldfetch_action_is_closed: 0has been added toDriveGroupFetchMethodVersions. Confirm that version dispatch in thers-drivecrate correctly handles this new method (fetch_action_is_closed_v0).
🏁 Script executed:
#!/bin/bash # Search for implementations and dispatch references rg "fetch_action_is_closed" -n packages/rs-driveLength of output: 1122
✅
fetch_action_is_closedversion dispatch verifiedThe new
fetch_action_is_closed: 0entry is correctly handled in thers-drivecrate:
- packages/rs-drive/src/drive/group/fetch/mod.rs: imports
fetch_action_is_closed- packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/mod.rs: has the
0 => self.fetch_action_is_closed_v0(...)arm- packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/v0/mod.rs: implements
fetch_action_is_closed_v0No further changes required.
packages/rs-drive/src/drive/group/fetch/fetch_active_action_info/v0/mod.rs (3)
3-3: Rename import to active action path
The import ofgroup_active_action_pathreplaces the oldgroup_action_path, aligning with the active-action API. Good consistency with the updated path definitions.
28-32: Use active action path for fetching
Infetch_active_action_info_v0, callinggroup_active_action_pathensures data is read from the active-action tree. This correctly scopes the fetch to active actions.
63-69: Use active action path in add operations
Infetch_active_action_info_and_add_operations_v0, the call togroup_active_action_pathis correctly applied, ensuring operations target the active-action subtree.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_unfreeze_transition.rs (2)
62-63: LGTM: Added contract and token position identifiersThese new fields in the
GroupActionV0struct initialization provide important metadata for group action traceability, making the action proofs more robust.
77-77: LGTM: Added group action closing flagThe new
closes_group_actionparameter indicates whether this operation should close the group action. This is a key part of the improved proof system as it correctly tracks the lifecycle of group actions.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_mint_transition.rs (2)
63-64: LGTM: Added contract and token position identifiersThese new fields in the
GroupActionV0struct initialization provide important metadata for group action traceability, consistent with similar changes in other token transition actions.
78-78: LGTM: Added group action closing flagThe new
closes_group_actionparameter indicates whether this operation should close the group action, providing proper lifecycle tracking for the group action proof system.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_emergency_action_transition.rs (2)
58-59: LGTM: Added contract and token position identifiersThese new fields enhance the group action metadata with explicit contract and token position identifiers, consistent with the improvements to the group action proof system throughout the codebase.
73-73: LGTM: Added group action closing flagThe new
closes_group_actionparameter correctly determines the lifecycle state of the group action based on the base transition'sperform_action()result.packages/rs-sdk/tests/fetch/group_actions.rs (2)
153-154: LGTM: Updated test assertion with new fieldsThe test assertion has been properly updated to include the new
contract_idandtoken_contract_positionfields in theGroupActionV0struct, ensuring the test validates the extended group action metadata.
185-186: LGTM: Updated test assertion with new fieldsTest assertion correctly updated to match the new
GroupActionV0structure, maintaining test coverage for the enhanced group action proof system.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_burn_transition.rs (2)
59-60: Fields added to enhance group action metadata with contract and token position.The addition of
contract_idandtoken_contract_positionprovides better identification and traceability of group actions. This change properly enriches the group action metadata with explicit contract and token position identifiers.
74-74: Group action closure status tracking implemented.The new
closes_group_actionfield appropriately uses the base transition'sperform_action()method to determine whether the group action should be closed. This enables proper lifecycle management for group actions.packages/rs-drive/src/drive/group/fetch/fetch_action_id_info_keep_serialized/v0/mod.rs (3)
3-3: Import path updated to reflect active group actions.The import has been updated from
group_action_pathtogroup_active_action_pathto explicitly indicate that this path is associated with active group actions.
49-53: Path construction updated to use active group action path.Function call changed from
group_action_pathtogroup_active_action_pathto properly fetch from the active group actions subtree. This change aligns with the broader refactor to distinguish between active and closed group actions.
108-112: Path construction updated to use active group action path.Function call changed from
group_action_pathtogroup_active_action_pathto properly fetch from the active group actions subtree. This change maintains consistency with the other path construction update in this file.packages/rs-drive/src/util/batch/drive_op_batch/withdrawals.rs (2)
169-169: Added optional flag alterations parameter for batch move operations.The new
Noneargument corresponds to thealter_flags_to_new_flagsparameter, indicating that the storage flags of moved items should not be altered during the batch move operation.
211-211: Added optional flag alterations parameter for batch move operations.The new
Noneargument corresponds to thealter_flags_to_new_flagsparameter, indicating that the storage flags of moved items should not be altered during the batch move operation. This maintains consistency with the similar change at line 169.packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_freeze_transition.rs (2)
62-63: Fields added to enhance group action metadata with contract and token position.The addition of
contract_idandtoken_contract_positionprovides better identification and traceability of group actions. This change properly enriches the group action metadata with explicit contract and token position identifiers, consistent with similar changes in other token transition files.
77-77: Group action closure status tracking implemented.The new
closes_group_actionfield appropriately uses the base transition'sperform_action()method to determine whether the group action should be closed. This enables proper lifecycle management for group actions, consistent with similar changes in other token transition files.packages/rs-drive/src/error/drive.rs (1)
196-203: Two new error variants for enhanced group action handling.These new error variants extend the error handling capabilities for group action management:
InvalidActionwill be used for invalid group action operationsElementNotFoundwill help with handling missing elements during batch moves and other operationsBoth follow the established error pattern in this enum and are well-documented with appropriate error messages.
packages/rs-drive/src/drive/group/prove/prove_action_infos/v0/mod.rs (12)
178-182: Updated GroupActionV0 with contract_id and token_contract_position fields.These new fields provide important context for token-related group actions, ensuring that the group action contains all necessary information about which contract and token it applies to. This is part of the improved proof system for group actions mentioned in the PR objectives.
184-188: Updated GroupActionV0 with contract_id and token_contract_position fields.The second group action is also updated with the necessary contract and token position fields, maintaining consistency with the updated struct definition.
191-204: Added closes_group_action parameter to add_group_action.The new
falseparameter indicates that this action doesn't close the group action. This enhancement allows for distinguishing between active and closed group actions, which is crucial for the new proof system's reliability.
207-221: Added closes_group_action parameter to add_group_action.Similar to the first call, this action also doesn't close the group action (false). This is consistent with the improved handling of group action lifecycles.
415-419: Updated GroupActionV0 with contract_id and token_contract_position fields.The test data is consistently updated with the required fields for all group actions, ensuring proper test coverage of the enhanced struct.
421-425: Updated GroupActionV0 with contract_id and token_contract_position fields.Continues the pattern of adding the new fields to all group actions in the test.
427-437: Updated GroupActionV0 with contract_id and token_contract_position fields.This transfer action is properly updated with the new fields required by the enhanced GroupActionV0 struct.
439-449: Updated GroupActionV0 with contract_id and token_contract_position fields.The last group action in the test is also updated with the new fields, maintaining consistency across all test cases.
452-465: Added closes_group_action parameter to add_group_action.The closes_group_action flag is consistently set to false across all add_group_action calls, ensuring that the actions remain active.
468-481: Added closes_group_action parameter to add_group_action.The second call to add_group_action also includes the closes_group_action flag set to false, maintaining consistency.
484-497: Added closes_group_action parameter to add_group_action.All add_group_action calls are consistently updated with the new parameter throughout the test case.
500-513: Added closes_group_action parameter to add_group_action.The final call to add_group_action is also properly updated with the new closes_group_action parameter.
packages/rs-drive/src/drive/group/fetch/fetch_action_id_signers_power/v0/mod.rs (3)
1-1: Updated import to use the renamed group_active_action_path function.The import has been updated to use the more specific
group_active_action_pathfunction instead of the genericgroup_action_path. This change explicitly indicates that the operation targets active group actions.
47-51: Updated to use group_active_action_path for constructing the path.The code now uses
group_active_action_pathto construct the GroveDB path for fetching action signers, making it clear that this operation targets active group actions.
104-108: Updated to use group_active_action_path consistently.This ensures consistent use of the renamed path function throughout the file. This is part of the broader enhancement that distinguishes between active and closed group actions.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_set_price_for_direct_purchase_transition.rs (2)
60-68: Enhanced GroupActionV0 with contract_id and token_contract_position fields.The group action now includes two important new fields:
contract_id- Specifies which contract the action belongs totoken_contract_position- Identifies the token's position within the contractThese new fields provide critical context for token-related group actions, ensuring that the generated proof contains all necessary information.
70-78: Added closes_group_action flag to AddGroupAction operation.The
closes_group_actionflag now determines whether this operation should close the group action. It's derived from the base transition'sperform_action()method, establishing a clear connection between performing an action and closing it.This enhancement supports the improved proof system by tracking whether actions remain active or are closed.
packages/rs-dpp/src/group/group_action/mod.rs (3)
3-5: Appropriate imports added for the new trait.The new imports correctly support the
GroupActionAccessorstrait that's being added to this file, providing the necessary types for the accessors.Also applies to: 9-9
19-23: Well-designed accessor trait.This trait provides a clean interface for accessing key properties of group actions. The trait declaration clearly defines the contract with three essential accessor methods.
24-42: Good implementation of the accessor trait for GroupAction.The implementation properly delegates to the inner variant methods, providing a consistent interface regardless of the version. This pattern ensures that clients don't need to know about the internal structure of the enum.
packages/rs-drive/src/drive/group/paths.rs (2)
91-103: Function rename clarifies it's for active actions only.The function was renamed from
group_action_pathtogroup_active_action_pathto explicitly indicate its association with active group actions. This improves code clarity and aligns with the distinction between active and closed actions.
106-118: Vector version of path function also renamed consistently.Similarly,
group_action_path_vecwas renamed togroup_active_action_path_vecto maintain consistency with the non-vector version. This rename is part of a broader refactoring to explicitly distinguish between active and closed group actions.packages/rs-drive/src/util/batch/drive_op_batch/group.rs (2)
32-34: Added boolean flag to indicate group action closure.The new
closes_group_actionfield added toGroupOperationType::AddGroupActionenables distinguishing between initiating a new action and closing an existing one. The comment clearly explains its purpose.
56-62: Parameter properly passed to add_group_action_operations.The implementation correctly passes the new
closes_group_actionparameter to the drive method. This change aligns with the enhanced group action lifecycle management where actions can be explicitly marked as closed.packages/rs-drive/src/drive/group/prove/prove_action_signers/v0/mod.rs (7)
128-130: Simplified syntax for BTreeMap initialization.The code now uses a more concise syntax for initializing the BTreeMap of member powers, improving readability while maintaining the same functionality.
Also applies to: 136-139
174-178: Added new required fields to GroupActionV0.The test correctly includes the new required fields
contract_idandtoken_contract_positionwhen constructing the GroupActionV0 instance, ensuring the tests reflect the updated data structure.
180-184: Consistent structure for all test GroupActionV0 instances.This second test action is also correctly updated with the new required fields, maintaining consistency across all test cases.
191-193: Updated add_group_action call with new parameter.The test now includes the new
closes_group_actionparameter (set to false) when callingadd_group_action, reflecting the API changes for group action management.
207-209: All add_group_action calls consistently updated.All calls to
add_group_actionthroughout the test file have been updated to include thecloses_group_actionparameter, ensuring consistent testing of the new API.Also applies to: 223-225
444-448: Test for closed actions updated with new fields.The test for closed actions now correctly includes the required
contract_idandtoken_contract_positionfields in the GroupActionV0 instance, maintaining test validity with the updated data structures.
455-456: Consistent parameter update in final test case.The final test case also correctly includes the new
closes_group_actionparameter in theadd_group_actioncall, ensuring all tests consistently exercise the new API.packages/rs-drive/src/util/grove_operations/mod.rs (1)
140-140: Addition of a new batch_move module looks good!This addition follows the established pattern of modular organization in this file, where each Grove DB operation type has its own module.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_config_update_transition.rs (2)
63-64: Enriching GroupActionV0 with additional context data looks good!Adding the contract_id and token_contract_position fields to GroupActionV0 improves the traceability and contextual information available for group actions. This is a valuable enhancement for tracking and identifying token-related group actions.
78-78: Properly implementing group action lifecycle with closes_group_action flagAdding the closes_group_action flag derived from perform_action() enables proper lifecycle management of group actions. This allows the system to distinguish between active and closed group actions, which is essential for the improved proof system described in the PR objectives.
packages/rs-dpp/src/group/group_action/v0/mod.rs (3)
1-7: Imports for new fields look goodThe additional imports support the new fields being added to GroupActionV0 and its accessor implementation.
14-16: Enhanced GroupActionV0 with additional context fieldsAdding contract_id and token_contract_position to GroupActionV0 provides valuable context information that helps with group action identification and verification. This is well-aligned with the PR objective of improving the proof system for group actions.
19-31: Well-implemented accessor trait for GroupActionV0The implementation of GroupActionAccessors provides a clean, standardized interface for accessing the fields of GroupActionV0. This improves code consistency and follows good object-oriented design principles.
packages/rs-drive/src/state_transition_action/action_convert_to_operations/batch/token/token_destroy_frozen_funds_transition.rs (1)
73-74: Confirm semantic coupling betweencloses_group_actionandperform_action()You derive
closes_group_actionstraight fromself.base().perform_action().
For most token transitions this is correct, but if future versions introduce “dry-run” transitions (perform=false yet still closing the action) this direct tie may become brittle.Consider exposing an explicit accessor on
TokenBaseTransitionAction(e.g.closes_group_action()) or documenting thatperform_action()doubles as a “close” flag for destroy-frozen-funds.packages/rs-dpp/src/state_transition/proof_result.rs (1)
28-39: Inconsistent inclusion ofGroupActionStatusacross new variantsThe three new “token group action” variants that carry balances, identity info, and pricing schedules include a
GroupActionStatus, yet the…WithDocumentvariant does not. Was that intentional?If action status is relevant for those three cases it is probably also valuable when returning an associated document; omitting it may force downstream callers to fetch status separately.
-VerifiedTokenGroupActionWithDocument(GroupSumPower, Option<Document>), +VerifiedTokenGroupActionWithDocument(GroupSumPower, GroupActionStatus, Option<Document>),Confirm whether the extra field is required for consistency.
packages/rs-drive/src/util/grove_operations/batch_delete/mod.rs (2)
8-8: LGTM: New import for DeleteOptions.Clean addition of the DeleteOptions import from grovedb for supporting customizable delete operations.
55-96: Well-structured extension for customizable delete operations.This new method appropriately extends the existing batch delete functionality by accepting customizable DeleteOptions. The implementation follows the established versioning pattern and error handling approach used throughout the codebase.
packages/rs-drive/src/verify/state_transition/state_transition_execution_path_queries/mod.rs (3)
1-5: LGTM: Clean imports for the new module.The imports are appropriate for the functionality being introduced.
6-36: Well-documented trait introduction with clear purpose.The documentation comprehensively explains the trait's purpose, required methods, and expected behavior. It provides good context for implementers about when and how to use this trait.
37-48: Good trait design for converting transitions to path queries.The
TryTransitionIntoPathQuerytrait is well-designed with an associated error type and a clear method signature. This abstraction will help standardize how state transitions are converted to path queries across different transition types.packages/rs-drive/src/state_transition_action/batch/batched_transition/token_transition/token_base_transition_action/v0/transformer.rs (2)
70-91: Improved group action validation with early detection of closed actions.This addition properly checks if a group action is already closed before attempting to fetch signers' power, preventing unnecessary processing and providing appropriate error responses. The error handling uses the correct error type (
GroupActionAlreadyCompletedError) and follows the established pattern.
204-225: Consistent implementation of closed action check in borrowed version.The same improvement has been correctly applied to the borrowed base transition method, maintaining consistency between the two implementations. This ensures that both entry points have the same validation logic.
packages/rs-drive/src/drive/group/fetch/fetch_action_is_closed/mod.rs (2)
1-11: LGTM: Well-structured new module with appropriate imports.The module organization and imports follow the established patterns in the codebase.
12-48: Clear implementation of a method to check group action status.The
fetch_action_is_closedmethod is well-documented and follows the versioning pattern used throughout the codebase. It provides a clean API for determining whether a group action is closed or still active, which is essential for the enhanced group action proof system described in the PR objectives.packages/rs-drive/src/util/grove_operations/batch_move/mod.rs (1)
16-26: LGTM! Well-structured batch move operation implementationThe implementation follows the standard pattern for versioned operations in the codebase and has proper error handling.
packages/rs-drive/src/verify/group/verify_action_signers_total_power/mod.rs (2)
18-47: Excellent documentation and implementationThe documentation is thorough and clearly explains the purpose, parameters, return values, errors, and versioning. The implementation properly handles version dispatch.
49-82: Well-structured method implementationThe method follows the standard pattern for versioned dispatch and includes proper error handling for unknown versions.
packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/mod.rs (1)
12-12: LGTM! Well-structured parameter additionThe parameter
alter_flags_to_new_flagsis properly added to the method signature, documentation, and passed to the versioned implementation.Also applies to: 21-21, 36-36, 51-51
packages/rs-drive/src/drive/group/estimated_costs/for_add_group_action/v0/mod.rs (1)
174-218: Potential double-insert of identical layer-info entries when called twiceWhen
also_add_closed_treeistruethe function unconditionally inserts
entries for the closed-action root/path/signers. If the caller ever invokes
add_estimation_costs_for_add_group_action_v0first withfalse(for the
active tree) and later withtrue(e.g. for closing the same action in the
same estimation map) theHashMap::insertwill overwrite the previous records
silently, losing the active-tree estimates.Consider guarding each
insertwithentry().or_insert_with(...), or providing
separate maps for active / closed estimations so information cannot be lost.packages/rs-drive/src/util/grove_operations/batch_move_items_in_path_query/v0/mod.rs (1)
240-459: Unit tests do not exercise the new flag-alteration branchAll added tests pass
Noneforalter_flags_to_new_flags. Edge-cases such as
Some(None)(strip flags) andSome(Some(flags))(replace with new) are still
untested, so regressions could slip in unnoticed.Would you like help writing a small test that moves items while stripping their
flags to confirm the behaviour?packages/rs-drive/src/verify/group/verify_action_signers_total_power/v0/mod.rs (2)
91-103: Subset verification path may drop parent feature
verify_subset_query_get_parent_tree_info()only returns the immediate parent feature of the queried key.
Ifaction_signer_idis several levels below the summed root, the returnedtree_featurecan beNone, causing a false “expected a summed tree” error.Double-check that the query always targets the first level under the summed-Merk node or fetch the feature from
proved_key_valuesinstead of the parent info whentree_feature.is_none().
117-131:⚠️ Potential issueCasting aggregate power may truncate
aggregate_powercomes fromTreeFeatureType::SummedMerkNode(u64), butGroupSumPoweris an alias foru128.
Theascast is silent and lossy if values exceedu64::MAX.Either:
- Change
TreeFeatureType::SummedMerkNodeto hold a wider type, or- Return
u64here and up-cast later with explicit widening.-Ok((root_hash, action_status, aggregate_power as GroupSumPower)) +Ok((root_hash, action_status, GroupSumPower::from(aggregate_power)))Likely an incorrect or invalid review comment.
packages/rs-drive/src/util/grove_operations/batch_delete/v0/mod.rs (1)
24-31:validate_tree_at_path_exists: falsemay hide data-loss bugsSetting
validate_tree_at_path_existstofalsewill silently proceed when the
target path is mistyped or was pruned earlier, potentially deleting the wrong
element (because the resolved path collapses).Unless there is a strong performance reason, prefer enabling the check:
-validate_tree_at_path_exists: false, +validate_tree_at_path_exists: true,or expose the choice to the caller instead of hard-coding it.
packages/rs-drive/src/util/grove_operations/batch_move/v0/mod.rs (1)
302-331: Test covers tree move error — good catchThe negative test ensures we don’t accidentally move sub-trees.
No issues spotted here – thanks for the defensive test.packages/rs-drive/src/verify/state_transition/verify_state_transition_was_executed_with_proof/v0/mod.rs (1)
352-360: Use caller-suppliedblock_infowhen building the expected historical document
build_historical_documentcurrently receives&BlockInfo::default(), which
throws away the real block height / time provided by the caller and may cause
false negatives when the verifier compares system-populated fields (e.g.
blockHeight,blockTime). Passing the actualblock_infokeeps the check
honest.- &BlockInfo::default(), + block_info,packages/rs-drive/src/drive/group/insert/add_group_action/mod.rs (4)
27-27: Clear parameter addition for group action closing functionalityThe new boolean parameter
closes_group_actionis added to distinguish between active and closing group actions, which aligns with the PR objective of improving the group action proof system.
41-41: Parameter correctly passed to implementation methodThe
closes_group_actionparameter is correctly passed to the version-specific implementation, maintaining consistency with the public API.
65-65: Consistent parameter addition in operation building methodThe
closes_group_actionparameter is similarly added to the operation-building method and properly passed to its implementation, maintaining API consistency across the related methods.Also applies to: 80-80
104-104: Parameter consistently added to operations generation methodThe parameter addition to
add_group_action_operationscompletes the consistent implementation across all three related methods, properly supporting the distinction between active and closing group actions.Also applies to: 120-120
Issue being fixed or feature implemented
Group proofs were not working as intended. In this PR we fix this by introducing a better proof system for group actions where both the fact the signing occurred and the result is returned in provable form.
How Has This Been Tested?
Heavily tested with a lot more unit tests.
Breaking Changes
Not breaking vis a vis 1.8.
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Documentation
Refactor