feat: document creation restriction by group membership #3032
Conversation
📝 WalkthroughWalkthroughAdds support for creationRestrictionMode = 3 ("Any group member") with an optional creationRestrictionGroup index across schema, DocumentTypeV2, validation, tests, and platform-versioning; enforces presence/absence of the group index and checks group membership during document creation. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Validator as Validator (Document Create)
participant DocType as DocumentType V2
participant Contract as Data Contract
participant Group as Group
Client->>Validator: Submit CreateDocument(owner_id)
Validator->>DocType: creation_restriction_mode()
DocType-->>Validator: AnyGroupMember (3)
Validator->>DocType: creation_restriction_group()
DocType-->>Validator: group_position
Validator->>Contract: resolve_group_at_position(group_position)
alt group exists
Contract-->>Validator: Group ref
Validator->>Group: member_power(owner_id)
alt member_power > 0
Group-->>Validator: member_power > 0
Validator-->>Client: success (creation allowed)
else not member
Group-->>Validator: GroupMemberNotFound / ProtocolError
Validator-->>Client: IdentityNotMemberOfGroupError
end
else position invalid
Contract-->>Validator: not found
Validator-->>Client: GroupPositionDoesNotExistError
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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. Comment |
…rictionMode-3-group
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs`:
- Around line 3523-3524: Rename the test function
run_chain_high_throughput_address_operations so its identifier begins with
"should" to comply with the tests naming convention; update the function
declaration #[stack_size(4 * 1024 * 1024)] fn
run_chain_high_throughput_address_operations() to a descriptive name starting
with "should", e.g. #[stack_size(4 * 1024 * 1024)] fn
should_run_chain_high_throughput_address_operations(), and update any references
to this function in the test module accordingly (keep the stack_size attribute
and body unchanged).
In
`@packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs`:
- Around line 670-672: Rename the test function
run_chain_insert_one_new_identity_per_block_and_one_new_document to start with
"should" (e.g., should_insert_one_new_identity_per_block_and_one_new_document)
while preserving the #[test] and #[stack_size(4 * 1024 * 1024)] attributes and
any references; update any references or uses of the function name in the test
module so the new name compiles and continues to run under the same test
harness.
- Around line 760-762: The test function
run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change
should be renamed to follow the convention starting with "should"; update the
test function name (the #[test] function
run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change) to
a descriptive name beginning with "should" (e.g.,
should_insert_one_new_identity_per_block_and_document_with_epoch_change) and
update any references to that symbol in the test module; keep the
#[stack_size(...)] attribute and function body unchanged.
🧹 Nitpick comments (3)
packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_update/basic_structure/v0/mod.rs (1)
72-100: Consider using a named constant or theCreationRestrictionModeenum instead of magic number3.The value
3representsAnyGroupMembermode but is unclear without context. TheCreationRestrictionModeenum exists in thedppcrate and is already available through your imports. UsingCreationRestrictionMode::AnyGroupMember as u8or defining a constant would improve readability and prevent silent breakage if the enum values change. The same pattern appears indata_contract_create/basic_structure/v0/mod.rsat line 86.TODO.md (1)
15-26: TODO items reference V0/V1, but implementation uses V2.The tracking items suggest adding
creation_restriction_grouptoDocumentTypeV0andDocumentTypeV1, but based on the PR changes, the field is only being added to the newDocumentTypeV2. Consider updating the TODO to reflect the actual implementation approach (V2-only) to avoid confusion.packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/data_contract_create/basic_structure/v0/mod.rs (1)
328-385: Align the test name with the exercised scenario (nonexistent group).
The test setscreationRestrictionGroup: 1, so it validates a missing group position, not a missing field. Consider renaming and optionally adding a separate test that omits the field to exercise the “required” path.✏️ Rename suggestion
- fn should_return_invalid_result_when_creation_restriction_group_missing() { + fn should_return_invalid_result_when_creation_restriction_group_does_not_exist() {
| #[stack_size(4 * 1024 * 1024)] | ||
| fn run_chain_high_throughput_address_operations() { |
There was a problem hiding this comment.
Rename test to start with should ….
The test name doesn’t follow the required naming convention.
Suggested rename
- fn run_chain_high_throughput_address_operations() {
+ fn should_handle_high_throughput_address_operations() {As per coding guidelines, **/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should …'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[stack_size(4 * 1024 * 1024)] | |
| fn run_chain_high_throughput_address_operations() { | |
| #[stack_size(4 * 1024 * 1024)] | |
| fn should_handle_high_throughput_address_operations() { |
🤖 Prompt for AI Agents
In `@packages/rs-drive-abci/tests/strategy_tests/test_cases/address_tests.rs`
around lines 3523 - 3524, Rename the test function
run_chain_high_throughput_address_operations so its identifier begins with
"should" to comply with the tests naming convention; update the function
declaration #[stack_size(4 * 1024 * 1024)] fn
run_chain_high_throughput_address_operations() to a descriptive name starting
with "should", e.g. #[stack_size(4 * 1024 * 1024)] fn
should_run_chain_high_throughput_address_operations(), and update any references
to this function in the test module accordingly (keep the stack_size attribute
and body unchanged).
| #[test] | ||
| #[stack_size(4 * 1024 * 1024)] | ||
| fn run_chain_insert_one_new_identity_per_block_and_one_new_document() { |
There was a problem hiding this comment.
Rename test to start with should ….
The test name doesn’t follow the required naming convention.
Suggested rename
- fn run_chain_insert_one_new_identity_per_block_and_one_new_document() {
+ fn should_insert_one_new_identity_per_block_and_one_new_document() {As per coding guidelines, **/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should …'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| #[stack_size(4 * 1024 * 1024)] | |
| fn run_chain_insert_one_new_identity_per_block_and_one_new_document() { | |
| #[test] | |
| #[stack_size(4 * 1024 * 1024)] | |
| fn should_insert_one_new_identity_per_block_and_one_new_document() { |
🤖 Prompt for AI Agents
In
`@packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs`
around lines 670 - 672, Rename the test function
run_chain_insert_one_new_identity_per_block_and_one_new_document to start with
"should" (e.g., should_insert_one_new_identity_per_block_and_one_new_document)
while preserving the #[test] and #[stack_size(4 * 1024 * 1024)] attributes and
any references; update any references or uses of the function name in the test
module so the new name compiles and continues to run under the same test
harness.
| #[test] | ||
| #[stack_size(4 * 1024 * 1024)] | ||
| fn run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change() { |
There was a problem hiding this comment.
Rename test to start with should ….
The test name doesn’t follow the required naming convention.
Suggested rename
- fn run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change() {
+ fn should_insert_one_new_identity_per_block_and_a_document_with_epoch_change() {As per coding guidelines, **/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should …'.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #[test] | |
| #[stack_size(4 * 1024 * 1024)] | |
| fn run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change() { | |
| #[test] | |
| #[stack_size(4 * 1024 * 1024)] | |
| fn should_insert_one_new_identity_per_block_and_a_document_with_epoch_change() { |
🤖 Prompt for AI Agents
In
`@packages/rs-drive-abci/tests/strategy_tests/test_cases/identity_and_document_tests.rs`
around lines 760 - 762, The test function
run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change
should be renamed to follow the convention starting with "should"; update the
test function name (the #[test] function
run_chain_insert_one_new_identity_per_block_and_a_document_with_epoch_change) to
a descriptive name beginning with "should" (e.g.,
should_insert_one_new_identity_per_block_and_document_with_epoch_change) and
update any references to that symbol in the test module; keep the
#[stack_size(...)] attribute and function body unchanged.
Issue being fixed or feature implemented
What was done?
How Has This Been Tested?
Breaking Changes
Checklist:
For repository code-owners and collaborators only
Summary by CodeRabbit
New Features
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.