feat(drive): add array element indexing support#2974

Draft

pauldelucia wants to merge 16 commits intov3.0-devfrom

feat/arrayElementIndexing

Draft

feat(drive): add array element indexing support#2974
pauldelucia wants to merge 16 commits intov3.0-devfrom
feat/arrayElementIndexing

Conversation

Copy link

Member

pauldelucia commented Jan 10, 2026 *

edited by coderabbitai bot

Loading

Issue being fixed or feature implemented

Adds support for indexing individual elements of array fields in document types. This enables efficient queries like "find all posts where hashtags contains 'dash'" by creating separate index entries for each array element, rather than treating the entire array as a single indexed value.

What was done?

Schema & Validation (rs-dpp):

Added support for parsing array properties with items definition in try_from_value_map
Added ITEMS constant to property_names module
Validation ensures arrays must be last in compound indexes and only one array per index

Encoding (rs-dpp):

Added ArrayItemType.encode_element_for_tree_keys() for encoding individual array elements as index keys
Added Document.get_raw_array_elements_for_document_type() to extract and encode all array elements with deduplication

Serialization (rs-dpp):

Added ArrayItemType.read_from() for deserializing array elements from storage
Implemented array deserialization in DocumentPropertyType::read_optionally_from

Index Operations (rs-drive):

Modified add_indices_for_top_index_level_for_contract_operations to detect array properties and create index entries for each element
Modified add_indices_for_index_level_for_contract_operations with same array handling pattern for nested index levels
Modified remove_indices_for_top_index_level_for_contract_operations with same array handling pattern
Modified remove_indices_for_index_level_for_contract_operations with same array handling pattern for nested index levels
Update operations apply remove operations first, then add operations to avoid GroveDB batch conflicts when array elements overlap
Added explicit error for array properties in contested indexes (not supported)

Query Support (rs-drive):

Added Contains query operator (aliased as in for SQL compatibility)
Query path construction handles array indexes by using the query value as the key

How Has This Been Tested?

Added 13 unit tests for array indexing in query_tests.rs:
Added test contract array-index-with-types-contract.json for testing different array element types
All existing test suites pass

Breaking Changes

None - this is additive functionality. Existing contracts and documents are unaffected.

Checklist:

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have added or updated relevant unit/integration/functional/e2e tests
I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
I have made corresponding changes to the documentation if needed

For repository code-owners and collaborators only

I have assigned this pull request to a milestone

Summary by CodeRabbit

New Features
- Per-element indexing for array properties and a method to retrieve raw encoded array elements for indexing/tree-keys; element encoding/decoding for array items added.
Query
- Added Contains operator to search for values inside indexed arrays.
Behavior & Validation
- Indexed-array rules enforced: allowed item types, bounded string/byte sizes, only one array per compound index, and array must be last.
Drive
- Index add/remove/update now processes array elements individually; contested indexes disallow array properties.
Tests
- Added fixtures and array-index tests (duplicate test module present).

_{Tip: You can customize this high-level summary in your review settings.}


 feat(drive): add array element indexing support

f66c5b8

pauldelucia requested a review from QuantumExplorer as a code owner

January 10, 2026 14:02

github-actions bot added this to the v3.0.0 milestone Jan 10, 2026

Copy link

Contributor

coderabbitai bot commented Jan 10, 2026 *

edited

Loading

Walkthrough

Adds indexed-array support: schema validation for indexed array properties, array item encoding/decoding, a Document API to return deduplicated encoded array elements, per-element index insert/remove/update logic in Drive, a new Query Contains operator, platform-version flags, tests, and visibility/API adjustments.

Changes

Cohort / File(s)	Summary
Data Contract Validation `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs`, `packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs`	Track index property positions; allow array properties only when items are indexable scalar types (bounded `string`/`byteArray` or integer/number/identifier/date); disallow boolean arrays; enforce single-array-per-index and require array property to be last; position-aware error reporting.
Array Item Encoding & Deserialization `packages/rs-dpp/src/data_contract/document_type/property/array.rs`, `packages/rs-dpp/src/data_contract/document_type/property/mod.rs`	Add `ArrayItemType::encode_element_for_tree_keys()` and `read_from()`; parse `items` schemas (including nested byte-array), enforce deserialization length caps, and use item encoders for tree-key encoding; publicly re-export `ArrayItemType` and add `MAX_ARRAY_ITEMS_FOR_DESERIALIZATION`.
Schema Const `packages/rs-dpp/src/data_contract/document_type/mod.rs`	Added crate-private constant `ITEMS`.
Document: raw array elements API `packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/*`, `packages/rs-dpp/src/document/mod.rs`, `packages/rs-dpp/src/document/v0/mod.rs`	New trait and method returning deduplicated encoded array elements for a key path; platform-version dispatch and `DocumentV0` impl added.
Drive: per-element index handling (insert/delete/update) `packages/rs-drive/src/drive/document/insert/`, `packages/rs-drive/src/drive/document/delete/`, `packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs`, `packages/rs-drive/src/drive/document/insert_contested/**`	Detect array properties via `DocumentPropertyType`; for arrays, fetch encoded elements via the new Document API and iterate per-element to insert/remove index entries and recurse; update update flow to remove+add when indexes contain arrays; contested indices now reject arrays.
Drive Function Visibility `packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs`	Visibility changed from `pub(super)` to `pub(crate)`.
DocumentInfo API `packages/rs-drive/src/util/object_size_info/document_info.rs`	Added `get_raw_array_elements_for_document_type` to `DocumentInfoV0Methods` and implementations delegating to document getters or returning empty for estimation mode.
Query: Contains operator `packages/rs-drive/src/query/conditions.rs`, `packages/rs-drive/src/query/filter.rs`	Added `WhereOperator::Contains` with parsing, eval, type/shape checks, path-query serialization support, and disallow rules in filters where appropriate.
Platform Versioning `packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/{mod,v1,v2,v3}.rs`	Added `get_raw_array_elements_for_document_type: FeatureVersion` to `DocumentMethodVersions` and initialized to `0` across versions.
Tests & Fixtures `packages/rs-drive/tests/query_tests.rs`, `packages/rs-drive/tests/supporting_files/contract/array-index/*`	Added array-index test fixtures and contracts; new `array_index_tests` test module appears (duplicate module present in test file).

Sequence Diagram(s)

sequenceDiagram participant DriveOp as Drive Operation participant Document as DocumentV0 participant DPP as DocumentType/ArrayItemType participant Drive as Drive Indexer DriveOp->>Document: get_raw_array_elements_for_document_type(key_path, doc_type) Document->>DPP: flatten properties, lookup array item type DPP-->>Document: ArrayItemType Document->>Document: encode each non-null element via ArrayItemType.encode_element_for_tree_keys() Document-->>DriveOp: Vec (deduplicated) DriveOp->>Drive: for each encoded element -> add/remove index entry (push key, create/remove subtree, recurse) Drive-->>DriveOp: result

sequenceDiagram participant Updater as Update Flow participant Drive as Drive participant Document as DocumentV0 Updater->>Drive: check if any index contains array Drive-->>Updater: yes Updater->>Document: get_raw_array_elements_for_document_type(old_key) Document-->>Updater: old elements Updater->>Drive: remove_indices_for_top_index_level per old element Updater->>Document: get_raw_array_elements_for_document_type(new_key) Document-->>Updater: new elements Updater->>Drive: add_indices_for_top_index_level per new element Drive-->>Updater: done

Estimated code review effort

4 (Complex) | ~50 minutes

Poem

I hop through schemas, nibbling each array,
I pack each element into keys neat and small,
Indexes bloom per hashtag and tag,
"Contains" snoops where items hide in a thrall,
A rabbit cheers: indices tidy for all.

Pre-merge checks | 3

Passed checks (3 passed)

Check name	Status	Explanation
Description Check	Passed	Check skipped - CodeRabbit's high-level summary is enabled.
Title check	Passed	The title 'feat(drive): add array element indexing support' accurately and concisely summarizes the main change: adding array element indexing functionality to the drive package.
Docstring Coverage	Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{Tip: You can configure your own custom pre-merge checks in the settings.}

Finishing touches

Generate docstrings

Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Commits

Reviewing files that changed from the base of the PR and between 20ee9b5 and 4cfdc7f.

Files selected for processing (3)

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs
packages/rs-drive/tests/query_tests.rs

Files skipped from review as they are similar to previous changes (1)

packages/rs-drive/tests/query_tests.rs

Additional context used

Path-based instructions (1)

**/*.rs

CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Rust code must pass cargo clippy --workspace linter checks
Rust code must be formatted using cargo fmt --all

**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants

Files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs

Learnings (13)

Learning: 2024-10-29T14:40:54.727Z

Learnt from: lklimek Repo: dashpay/platform PR: 2277 File: packages/rs-sdk/src/core/transaction.rs:0-0 Timestamp: 2024-10-29T14:40:54.727Z Learning: In `packages/rs-sdk/src/platform/document_query.rs` and `packages/rs-sdk/src/core/transaction.rs`, certain places don't implement `IntoInner`, so direct error mappings cannot be simplified using `IntoInner`. A TODO comment has been added to address this in a future PR.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs

Learning: 2025-06-18T03:44:14.385Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2673 File: packages/rs-drive-abci/src/execution/validation/state_transition/state_transitions/identity_update/mod.rs:1164-1197 Timestamp: 2025-06-18T03:44:14.385Z Learning: QuantumExplorer determined that a CodeRabbit suggestion about fixing signable_bytes calculation in identity update tests with contract-bound keys was incorrect - the code flow is working as intended without the suggested changes.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-20T20:43:41.185Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2257 File: packages/rs-drive-abci/tests/strategy_tests/masternodes.rs:212-220 Timestamp: 2024-11-20T20:43:41.185Z Learning: In `packages/rs-drive-abci/tests/strategy_tests/masternodes.rs` , the pattern of generating a `PrivateKey`, converting it to bytes, and reconstructing a `BlsPrivateKey` from those bytes is intentional. Avoid suggesting to simplify this code in future reviews.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-20T16:05:40.200Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2257 File: packages/rs-drive-abci/src/platform_types/signature_verification_quorum_set/v0/for_saving.rs:148-151 Timestamp: 2024-11-20T16:05:40.200Z Learning: In `packages/rs-drive-abci/src/platform_types/signature_verific ation_quorum_set/v0/for_saving.rs`, when converting public keys from `QuorumForSavingV0` to `VerificationQuorum`, it's acceptable to use `.expect()` for public key conversion, as the conversion has been verified and panics are acceptable in this context.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-05-28T16:22:26.334Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2644 File: packages/rs-drive/src/cache/system_contracts.rs:18-19 Timestamp: 2025-05-28T16:22:26.334Z Learning: In packages/rs-drive/src/cache/system_contracts.rs, the `active_since_protocol_version` field in `ActiveSystemDataContract` struct is intentionally added for future use, not current implementation. QuantumExplorer confirmed this is "meant for later" when questioned about the `#[allow(unused)]` attribute.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-25T01:17:02.001Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2347 File: packages/rs-drive/tests/query_tests.rs:438-460 Timestamp: 2024-11-25T01:17:02.001Z Learning: In Rust test files (`packages/rs-drive/tests/query_tests.rs`), when code is used only in tests, defining explicit enums for fields (like the `status` field in the `Withdrawal` struct) may not be necessary; using primitive types is acceptable.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-10-21T01:03:42.458Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2227 File: packages/rs-dpp/src/core_types/validator_set/v0/mod.rs:299-299 Timestamp: 2024-10-21T01:03:42.458Z Learning: In the `test_serialize_deserialize_validator_set_v0` test within `packages/rs-dpp/src/core_types/validator_set/v0/mod.rs`, deterministic BLS keys cannot be easily used; therefore, using `BlsPublicKey::generate()` is acceptable.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-10-09T15:59:28.329Z

Learnt from: lklimek Repo: dashpay/platform PR: 2716 File: packages/rs-dapi/src/services/platform_service/broadcast_state_transition.rs:181-187 Timestamp: 2025-10-09T15:59:28.329Z Learning: In `packages/rs-dapi/src/services/platform_service/broadcast_st ate_transition.rs`, the maintainer requires logging full state transition bytes (`tx_bytes = hex::encode(st_bytes)`) at debug level when a state transition passes CheckTx but is removed from the block by the proposer, to facilitate debugging of this rare edge case.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-10-22T10:53:12.111Z

Learnt from: shumkov Repo: dashpay/platform PR: 2259 File: packages/rs-dapi-client/src/dapi_client.rs:137-139 Timestamp: 2024-10-22T10:53:12.111Z Learning: In `packages/rs-dapi-client/src/dapi_client.rs`, when passing data into asynchronous code, ensure that data structures are `Send + Sync`. Using `Arc` is necessary for the retry counter.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs

Learning: 2024-11-03T10:39:11.242Z

Learnt from: shumkov Repo: dashpay/platform PR: 2305 File: packages/rs-drive-abci/src/abci/handler/finalize_block.rs:81-0 Timestamp: 2024-11-03T10:39:11.242Z Learning: In `packages/rs-drive-abci/src/abci/handler/finalize_block.rs`, the use of `.expect("commit transaction")` after `app.commit_transaction(platform_version)` is intentional due to the provided comment explaining its necessity. Do not flag this usage in future reviews.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-10-01T08:37:32.168Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2790 File: packages/rs-drive/src/drive/document/index_uniqueness/validate_document_purchase_transition_action_uniqueness/v1/mod.rs:65-0 Timestamp: 2025-10-01T08:37:32.168Z Learning: In purchase document transitions (packages/rs-drive/src/drive/document/index_uniqueness/valid ate_document_purchase_transition_action_uniqueness/v1/mod.rs ), the `changed_data_values` field in `UniquenessOfDataRequestUpdateType::ChangedDocument` should be set to an empty BTreeSet (`Cow::Owned(BTreeSet::new())`) because purchase transitions do not modify document data properties (like "price"), only ownership and transfer metadata. An empty set signals the uniqueness validation logic to skip checking unique indexes on data properties.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts (ensuring it's between 3 and 100 characters) is already handled at the data contract validation level in packages/rs-dpp/src/data_contract/methods/validate_update/v0 /mod.rs, making additional checks in the update operations redundant.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs

Code graph analysis (1)

packages/rs-dpp/src/data_contract/document_type/property/array.rs (4)

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs (2)

Value (195-195)

value (213-213)

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs (2)

Value (212-212)

value (230-230)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (7)

value (2479-2479)

value (2480-2480)

value (2495-2495)

get_field_type_matching_error (2469-2474)

encode_date_timestamp (1516-1518)

encode_i64 (1664-1687)

encode_float (1990-2027)

packages/rs-platform-value/src/btreemap_extensions/mod.rs (1)

vec (265-267)

Additional comments (7)

packages/rs-dpp/src/data_contract/document_type/property/array.rs (3)

1-15: LGTM! Well-structured deserialization bounds.

The constants MAX_STRING_LENGTH_FOR_DESERIALIZATION and MAX_BYTE_ARRAY_LENGTH_FOR_DESERIALIZATION at 64KB are reasonable DoS protection limits, and the imports are correctly used in the new methods.

264-331: LGTM! Encoding logic correctly uses the established encoding schemes.

The method properly delegates to DocumentPropertyType::encode_* methods which handle sign-bit flipping for lexicographic ordering. The special cases for null values (empty vec) and empty strings/byte arrays (single 0 byte) correctly avoid collisions.

333-444: LGTM! Deserialization correctly inverts encode_value_ref_with_size.

The implementation:

Correctly reads length-prefixed formats for String, ByteArray, and Identifier

Uses raw big-endian decoding (not sign-bit-flipped) matching the encoding path

Applies DoS protection bounds for variable-length types

Validates identifier length is exactly 32 bytes

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (4)

14-14: LGTM! Public export and deserialization bounds are appropriate.

The MAX_ARRAY_ITEMS_FOR_DESERIALIZATION at 256 correctly exceeds the contract maxItems <= 255 limit, providing DoS protection while allowing valid arrays to deserialize.

Also applies to: 29-32

762-786: LGTM! Array deserialization is correct and safe.

The implementation properly:

Validates array length before allocation to prevent DoS

Uses pre-allocation with bounded capacity

Correctly delegates to array_item_type.read_from() for each element

1254-1263: LGTM! Correct delegation for Contains query support.

The Array case correctly encodes a single query value (not the whole array) using the item type's encoding, which is the expected behavior for array element indexing and Contains queries.

2355-2431: LGTM! Comprehensive array items parsing with proper validation.

The implementation correctly:

Parses typed array items with appropriate type constraints (string with length bounds, integer, number, boolean, date)

Restricts nested arrays to byte arrays only, preventing arbitrary nesting

Falls back to existing byte array handling when no items is specified

Provides clear error messages for invalid configurations

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 @coderabbitai help to get the list of available commands and usage tips.}

fmt

313f024

coderabbitai bot reviewed

Jan 10, 2026

View reviewed changes

Copy link

Contributor

coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Fix all issues with AI agents

In @packages/rs-dpp/src/data_contract/document_type/property/array.rs: - Around line 274-278: The Date branch in ArrayItemType::Date currently uses value.to_integer(), causing encoding to i64 which is inconsistent with encode_value_with_size; change it to use value.into_float() (as encode_value_with_size does for DocumentPropertyType::encode_date_timestamp) and encode the f64 bytes (e.g., f64::to_be_bytes) so index keys and stored values use the same f64-based date encoding. - Around line 284-288: The ArrayItemType::Number branch in encode_element_for_tree_keys incorrectly serializes floats with value.to_float().to_be_bytes(), which breaks lexicographic ordering; replace that serialization by calling the existing DocumentPropertyType::encode_float(...) helper (the same function used by scalar F64 encoding) to produce the ordered byte representation, ensuring the branch uses value.to_float().map_err(ProtocolError::ValueError)? as input to DocumentPropertyType::encode_float and returns its Vec result so array element Number encoding matches scalar F64 tree-key ordering. - Around line 279-283: ArrayItemType::Integer currently encodes i64 values with plain to_be_bytes() which breaks lexicographic ordering for negative integers; replace that encoding with the same sign-bit-flipped big-endian encoding used by DocumentPropertyType::encode_i64 by calling DocumentPropertyType::encode_i64(value_as_i64) (or replicating its wtr[0] ^= 0b1000_0000 flip) instead of to_be_bytes(), ensuring ArrayItemType::Integer produces the same sortable byte representation as scalar integers. In @packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs: - Around line 110-121: The validation currently sets is_array_property by matching only DocumentPropertyType::Array(_), but DocumentPropertyType also has DocumentPropertyType::VariableTypeArray(_) which should be treated the same; update the match in the is_array_property computation (the call chain starting from document_type.flattened_properties().get(name).map(...)) to include DocumentPropertyType::VariableTypeArray(_) alongside Array(_) so that both array variants cause the NotSupported error when creating contested indexes.

Nitpick comments (9)

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

176-183: Consider extracting duplicated array detection logic.

The array property detection logic at lines 177-183 is duplicated at lines 501-507. Consider extracting this into a helper function or closure to improve maintainability.

Suggested refactor

// Extract helper closure at the start of the function let index_contains_array = |index: &Index| -> bool { index.properties.iter().any(|prop| { document_type .flattened_properties() .get(&prop.name) .map(|p| matches!(p.property_type, DocumentPropertyType::Array(_))) .unwrap_or(false) }) }; // Then use: if index_contains_array(index) { continue; } // And: let any_index_has_array = document_type.indexes().values().any(index_contains_array);

packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs (1)

139-142: Redundant operation: all_fields_null &= true has no effect.

The expression all_fields_null &= true is equivalent to all_fields_null = all_fields_null & true, which doesn't change the value. If the intent is to preserve all_fields_null unchanged, the line can be removed.

Suggested simplification

if array_elements.is_empty() { // Empty array - track as null any_fields_null = true; - all_fields_null &= true; } else {

packages/rs-drive/tests/query_tests.rs (6)

7003-7140: Hardcoded post IDs diverge from the new JSON fixtures; validate Identifier encoding + reduce drift.
The helper embeds $id/$ownerId values directly (Lines 7039-7129) while separate fixtures were added under tests/supporting_files/contract/array-index/.

Please confirm the $id/$ownerId strings in both the fixtures and these inline JSON values decode to the expected Identifier type (and are 32 bytes if that's the constraint). A quick alphabet check script is in the post1.json comment.

Consider loading post0.json/post1.json fixtures here (or deriving IDs from known 32-byte hex) to avoid fixture/code divergence.

7227-7275: Serialization test is fine; avoid unwrap() after is_ok() to keep failure output.
Right now the test checks is_ok() then serialized.unwrap() (Lines 7266-7274), which will lose the original context if it ever regresses.

Proposed tweak

- assert!( - serialized.is_ok(), - "Serialization should succeed, got: {:?}", - serialized.err() - ); - assert!( - serialized.unwrap().len() > 0, - "Serialized bytes should not be empty" - ); + let bytes = serialized.expect("serialization should succeed"); + assert!(!bytes.is_empty(), "Serialized bytes should not be empty");

7340-7388: Test naming doesn't follow repo guideline ("should ...").
New tests use test_* naming (Lines 7341+). If you want to follow **/tests/**/*.rs guidance, rename the new ones.

Based on coding guidelines, tests should start with "should ...".

7390-7500: contains query tests: add assertions on returned document IDs to prevent false positives.
Currently you only assert results.len() (Lines 7427-7431, 7460-7464, 7493-7497). A regression could return the wrong 2 docs and still pass.

Example follow-up assertion (pattern)

assert_eq!( results.len(), 2, "expected 2 posts containing 'dash' hashtag" ); + // TODO: also assert returned $id set matches expected (order-independent)

7502-7542: Array element encoding assertion may be brittle if "tree key encoding" changes.
The test assumes returned bytes equal b"alpha" etc. (Lines 7539-7541). If element encoding includes type markers/normalization, this will fail even if behavior is correct.

Consider asserting via the same encoding routine used for indexing (e.g., ArrayItemType.encode_element_for_tree_keys(...)) so the test tracks intended encoding semantics.

7544-7584: Dedup test: also assert which elements remain.
Right now it only checks len() == 2 (Lines 7578-7582). Verifying membership prevents accidental "dedup to wrong set" bugs.

packages/rs-drive/tests/supporting_files/contract/array-index/array-index-contract.json (1)

1-58: Contract fixture looks correct for array-element indexing constraints.
Array index is standalone or last in compound index, and items is a bounded scalar type.

Consider adding name fields for the indexes to make failures/debug output easier to interpret.

Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Commits

Reviewing files that changed from the base of the PR and between b172460 and 313f024.

Files selected for processing (28)

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs
packages/rs-dpp/src/data_contract/document_type/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs
packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/mod.rs
packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/v0/mod.rs
packages/rs-dpp/src/document/document_methods/mod.rs
packages/rs-dpp/src/document/mod.rs
packages/rs-dpp/src/document/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/query/conditions.rs
packages/rs-drive/src/query/filter.rs
packages/rs-drive/src/util/object_size_info/document_info.rs
packages/rs-drive/tests/query_tests.rs
packages/rs-drive/tests/supporting_files/contract/array-index/array-index-contract.json
packages/rs-drive/tests/supporting_files/contract/array-index/post0.json
packages/rs-drive/tests/supporting_files/contract/array-index/post1.json
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/mod.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v1.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v3.rs

Additional context used

Path-based instructions (2)

**/*.rs

CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Rust code must pass cargo clippy --workspace linter checks
Rust code must be formatted using cargo fmt --all

**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants

Files:

packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/mod.rs
packages/rs-dpp/src/data_contract/document_type/mod.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v1.rs
packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs
packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/mod.rs
packages/rs-dpp/src/document/mod.rs
packages/rs-drive/tests/query_tests.rs
packages/rs-dpp/src/document/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs
packages/rs-drive/src/query/filter.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/v0/mod.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs
packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs
packages/rs-dpp/src/document/document_methods/mod.rs
packages/rs-drive/src/query/conditions.rs
packages/rs-drive/src/util/object_size_info/document_info.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v3.rs

**/tests/**/*.{js,jsx,ts,tsx,rs}

CodeRabbit inference engine (AGENTS.md)

**/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should ...'
Unit and integration tests should not perform network calls; mock dependencies

Files:

packages/rs-drive/tests/query_tests.rs

Learnings (14)

Learning: 2025-10-01T08:37:32.168Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/query/filter.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs

Learning: 2024-12-10T10:56:26.215Z

Learnt from: lklimek Repo: dashpay/platform PR: 2030 File: packages/rs-sdk/tests/vectors/test_asset_lock_proof/quorum_pubkey-100-4ce7fd81273c2b394c0f32367374fc5b09ba912e017aacb366d2171e9ca6f9d5.json:1-1 Timestamp: 2024-12-10T10:56:26.215Z Learning: In the `packages/rs-sdk/tests/vectors/test_asset_lock_proof/` directory, files with the `.json` extension are mock data that may not follow standard JSON format; this is intentional and acceptable within the project's testing framework.

Applied to files:

packages/rs-drive/tests/supporting_files/contract/array-index/post1.json

Learning: 2024-10-29T14:40:54.727Z

Applied to files:

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs
packages/rs-dpp/src/document/mod.rs
packages/rs-dpp/src/document/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs
packages/rs-drive/src/query/filter.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/v0/mod.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs
packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/document/document_methods/mod.rs
packages/rs-drive/src/query/conditions.rs
packages/rs-drive/src/util/object_size_info/document_info.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs

Learning: 2024-11-28T13:49:17.301Z

Learnt from: shumkov Repo: dashpay/platform PR: 2317 File: packages/rs-dapi-client/src/address_list.rs:175-180 Timestamp: 2024-11-28T13:49:17.301Z Learning: In Rust code in `packages/rs-dapi-client/src/address_list.rs`, do not change the interface of deprecated methods like `add_uri`, even to fix potential panics.

Applied to files:

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs

Learning: 2024-11-22T08:19:14.448Z

Learnt from: shumkov Repo: dashpay/platform PR: 2345 File: packages/rs-drive-abci/src/execution/platform_events/protocol_upgrade/perform_events_on_first_block_of_protocol_change/v0/mod.rs:93-99 Timestamp: 2024-11-22T08:19:14.448Z Learning: In `packages/rs-drive-abci/src/execution/platform_events/protoc ol_upgrade/perform_events_on_first_block_of_protocol_change/ v0/mod.rs`, the `insert_contract` method requires an owned `BlockInfo`, so cloning `block_info` is necessary when calling it.

Applied to files:

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-10-08T13:28:03.529Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2227 File: packages/rs-drive-abci/src/platform_types/platform_state/mod.rs:141-141 Timestamp: 2024-10-08T13:28:03.529Z Learning: When converting `PlatformStateV0` to `PlatformStateForSavingV1` in `packages/rs-drive-abci/src/platform_types/platform_state/mo d.rs`, only version `0` needs to be handled in the match on `platform_state_for_saving_structure_default` because the changes are retroactive.

Applied to files:

packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/mod.rs
packages/rs-dpp/src/document/mod.rs
packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs

Learning: 2024-10-30T11:19:59.163Z

Learnt from: lklimek Repo: dashpay/platform PR: 2277 File: packages/rs-sdk/tests/fetch/config.rs:233-233 Timestamp: 2024-10-30T11:19:59.163Z Learning: In the Rust SDK's `rs-sdk/tests` integration tests (e.g., in `packages/rs-sdk/tests/fetch/config.rs`), we cannot create objects during tests because there is no support for object creation in this context. Therefore, hardcoded values for test identities must be used.

Applied to files:

packages/rs-drive/tests/query_tests.rs

Learning: 2024-11-25T01:17:02.001Z

Applied to files:

packages/rs-drive/tests/query_tests.rs

Learning: 2025-03-07T12:37:44.555Z

Learnt from: ogabrielides Repo: dashpay/platform PR: 2486 File: packages/rs-drive/src/drive/platform_state/store_reduced_platform_state_bytes/v0/mod.rs:1-29 Timestamp: 2025-03-07T12:37:44.555Z Learning: GroveDB operations like `insert()` return a tuple that includes a cost value which needs to be unwrapped with `.unwrap()` before error handling with `.map_err()`.

Applied to files:

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.

Applied to files:

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs

Learning: 2025-04-11T09:08:05.652Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs
packages/rs-drive/src/query/filter.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs

Learning: 2025-05-28T16:22:26.334Z

Applied to files:

packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs

Learning: 2025-02-03T23:34:43.081Z

Learnt from: QuantumExplorer Repo: dashpay/platform PR: 2450 File: packages/rs-drive/src/drive/tokens/distribution/queries.rs:57-57 Timestamp: 2025-02-03T23:34:43.081Z Learning: In Rust query implementations, the `..` operator after a value creates a `RangeFrom` bound rather than performing vector slicing, which is a safe operation for constructing range-based queries.

Applied to files:

packages/rs-drive/src/query/conditions.rs

Learning: 2024-11-20T14:14:54.721Z

Learnt from: lklimek Repo: dashpay/platform PR: 2337 File: packages/rs-dapi-client/src/executor.rs:161-212 Timestamp: 2024-11-20T14:14:54.721Z Learning: In `packages/rs-dapi-client/src/executor.rs`, the `WrapWithExecutionResult` trait supports on-the-fly type conversions using the `From` trait, which is useful when using the `?` operator to return errors.

Applied to files:

packages/rs-drive/src/query/conditions.rs

Code graph analysis (9)

packages/rs-dpp/src/data_contract/document_type/property/array.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (4)

value (2451-2451)

value (2452-2452)

value (2467-2467)

get_field_type_matching_error (2441-2446)

packages/rs-drive/tests/query_tests.rs (4)

packages/rs-drive/src/util/test_helpers/setup.rs (1)

setup_drive (39-45)

packages/rs-drive/src/drive/contract/test_helpers.rs (1)

add_init_contracts_structure_operations (8-10)

packages/rs-dpp/src/tests/json_document.rs (1)

json_document_to_contract (66-74)

packages/rs-dpp/src/document/serialization_traits/platform_serialization_conversion/mod.rs (1)

serialize (19-30)

packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

name (113-135)

packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

name (113-135)

packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

name (113-135)

packages/rs-dpp/src/document/document_methods/mod.rs (2)

packages/rs-drive/src/util/object_size_info/document_info.rs (4)

get_raw_array_elements_for_document_type (65-70)

get_raw_array_elements_for_document_type (261-282)

get_raw_for_document_type (54-61)

get_raw_for_document_type (156-258)

packages/rs-dpp/src/document/mod.rs (4)

get_raw_array_elements_for_document_type (147-175)

get_raw_for_contract (82-113)

get_raw_for_document_type (116-145)

hash (177-200)

packages/rs-drive/src/util/object_size_info/document_info.rs (2)

packages/rs-dpp/src/document/document_methods/mod.rs (1)

get_raw_array_elements_for_document_type (41-46)

packages/rs-dpp/src/document/mod.rs (1)

get_raw_array_elements_for_document_type (147-175)

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (2)

name (113-135)

max_size (260-291)

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

name (113-135)

Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: Rust packages (drive) / Tests
GitHub Check: Rust packages (dpp) / Check each feature
GitHub Check: Rust packages (drive-abci) / Tests
GitHub Check: Build JS packages / Build JS
GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image

Additional comments (49)

packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs (1)

13-13: LGTM! Import is necessary and correctly placed.

The DocumentPropertyType import is required for the array property type check introduced below and follows standard Rust conventions.

packages/rs-drive/src/query/conditions.rs (8)

76-77: LGTM: Contains operator properly documented.

The doc comment clearly explains the operator's purpose for array field queries.

95-95: LGTM: Flip behavior correctly disabled for Contains.

Contains cannot be meaningfully flipped (unlike comparison operators), so returning false in allows_flip() and an error in flip() is correct.

Also applies to: 125-127

138-139: LGTM: Contains correctly categorized as a range operator.

Since Contains requires scanning multiple index entries (one per array element), treating it as a range operator is appropriate.

241-244: LGTM: Contains evaluation logic is correct.

The implementation properly checks if the document's array field contains the query value, returning false if the field is not an array.

255-272: LGTM: Comprehensive type validation for Contains operator.

The implementation correctly validates that the query value's type matches the array's element type, covering all ArrayItemType variants. This ensures type safety at query construction time.

668-668: LGTM: Contains correctly classified as non-groupable.

Like StartsWith and Between variants, Contains cannot be meaningfully combined with other range operators, so the non-groupable classification is correct.

Also applies to: 686-686

1702-1703: LGTM: Contains operator correctly restricted to Array types.

The implementation properly limits Contains to DocumentPropertyType::Array, preventing misuse on scalar fields. Note that VariableTypeArray currently has no allowed operators, which may be a known limitation.

1202-1224: Clarify what "elsewhere" means for array element path construction in Contains queries.

The implementation correctly serializes the search element and inserts it as a key. However, the comment is imprecise about path adjustment happening "elsewhere." The array element subtree navigation actually occurs through the standard recursive_insert_on_query() processing, which handles remaining index properties sequentially. For clarity, the comment should explain that the field name becomes the subquery key in the path building logic (via query.set_subquery_key(first.name.as_bytes().to_vec())), allowing the serialized element value to naturally navigate to the correct element subtree within the index structure.

packages/rs-drive/src/query/filter.rs (1)

538-538: LGTM: Contains correctly rejected for price clause validation.

Since price is a scalar numeric field (U64), the Contains operator (designed for array fields) is correctly marked as invalid, consistent with how StartsWith is rejected for numeric contexts.

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/mod.rs (1)

32-32: LGTM: Visibility widened to support array indexing.

The visibility change from pub(super) to pub(crate) appropriately extends access to this function within the crate, aligning with the array indexing feature's requirements.

packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v2.rs (1)

24-31: LGTM: Consistent version initialization.

The addition of get_raw_array_elements_for_document_type: 0 properly tracks the new document method version, consistent with the initialization pattern for other method versions.

packages/rs-dpp/src/data_contract/document_type/mod.rs (1)

69-69: LGTM: Standard JSON Schema constant.

The addition of the ITEMS constant follows the established pattern and aligns with the JSON Schema standard for defining array item types.

packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v1.rs (1)

24-31: LGTM: Consistent version initialization.

The addition of get_raw_array_elements_for_document_type: 0 properly tracks the new document method version, maintaining consistency with the version initialization pattern.

packages/rs-dpp/src/data_contract/document_type/property/array.rs (1)

260-262: The collision concern is partially addressed; empty strings are already mitigated but empty byte arrays are not.

The code already handles null-to-empty-vec collisions for string types by encoding empty strings as [0] (see lines 1155-1157 in mod.rs and array.rs). However, ByteArray types in encode_element_for_tree_keys (array.rs line 289) return raw bytes without collision mitigation, meaning null values and empty byte arrays both encode to an empty vector. This should either be explicitly documented as safe or mitigated consistently with string handling.

packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/mod.rs (1)

24-24: LGTM!

The new version tracking field follows the established pattern and naming conventions for document method versions.

packages/rs-dpp/src/document/document_methods/mod.rs (3)

6-6: LGTM!

Module declaration follows the established pattern.

12-12: LGTM!

Re-export visibility is correctly scoped to crate::document.

38-46: LGTM!

The method signature is well-documented and follows the pattern of similar methods. The return type Vec> appropriately represents multiple encoded array elements, and the absence of owner_id parameter is reasonable since array elements are encoded for index keys independently.

packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/mod.rs (1)

1-3: LGTM!

Module structure and re-export pattern are consistent with sibling document method modules.

packages/rs-platform-version/src/version/dpp_versions/dpp_document_versions/v3.rs (1)

29-29: LGTM!

Version initialization to 0 is correct for the new method, and placement within DocumentMethodVersions is logical.

packages/rs-dpp/src/document/v0/mod.rs (2)

22-23: LGTM!

Import follows the established pattern and maintains the logical grouping of document method traits.

180-182: LGTM!

Trait implementation follows the established pattern used by sibling document method traits, with the "automatically done" comment indicating a default or blanket implementation.

packages/rs-dpp/src/document/mod.rs (1)

147-175: LGTM!

The new get_raw_array_elements_for_document_type method follows the established version-dispatch pattern consistently with other methods in this file. The error handling for unknown versions is correctly implemented.

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs (2)

111-114: Verify empty array handling in update scenarios.

When an array is empty, the removal loop is skipped entirely. This is correct if no index entries were created for an empty array. However, in update scenarios where an array transitions from non-empty to empty, the old index entries need to be removed.

Please verify that the update path (in update_document_for_contract_operations) correctly handles this case by ensuring the old document's array elements are passed for removal, not the new document's (potentially empty) array.

100-184: LGTM for array property removal logic.

The per-element removal flow correctly:

Retrieves array elements via get_raw_array_elements_for_document_type

Iterates each element to construct element-specific paths

Handles cost estimation per element

Recursively removes lower-level index entries

packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs (1)

167-203: LGTM for per-element array indexing.

The per-element insertion logic correctly:

Creates a tree for each element value

Pushes the element to the path

Recursively indexes sub-levels for each element

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (1)

113-211: LGTM for top-level array index insertion.

The array handling correctly:

Skips empty arrays (no index entries created)

Creates per-element trees and paths

Handles cost estimation per element

Recursively processes sub-levels for each element

The pattern is consistent with the removal logic in the corresponding module.

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v0/mod.rs (2)

457-540: LGTM for array item type validation.

The validation logic correctly:

Validates that array item types are indexable scalars (String, ByteArray, Integer, Number, Identifier, Date)

Enforces bounded sizes for String and ByteArray elements

Rejects Boolean arrays as not meaningful for indexing

Limits to one array property per compound index

600-616: LGTM for array position validation.

The check correctly enforces that array properties must be the last property in a compound index, which is essential for the per-element indexing strategy to work correctly with prefix-based queries.

packages/rs-dpp/src/document/document_methods/get_raw_array_elements_for_document_type/v0/mod.rs (1)

11-70: LGTM!

The implementation correctly:

Looks up the array item type from the document type definition

Returns empty vec for non-array or missing properties (graceful fallback)

Encodes each non-null element using the item type's encoder

Deduplicates elements while preserving first-occurrence order

Propagates encoding errors appropriately

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

497-540: The code correctly avoids double-processing. The main loop iterates over indexes (not individual properties) and skips any index that contains an array property entirely--not just the array properties themselves. The remove/add functions then handle all properties of these skipped indexes, processing both scalar and array properties as appropriate. No scalar properties are processed twice.

Likely an incorrect or invalid review comment.

packages/rs-dpp/src/data_contract/document_type/class_methods/try_from_schema/v1/mod.rs (3)

11-12: LGTM!

The conditional import of ArrayItemType under the validation feature flag is correctly placed alongside other validation-specific imports.

432-556: LGTM!

The array property validation logic is well-structured:

Properly validates each ArrayItemType variant with appropriate constraints

Correctly enforces single array property per index

Tracks array position for the "must be last" validation

Consistent with the v0 schema validation approach

The use of enumerate and position tracking is clean.

617-633: LGTM!

The "array must be last" constraint is correctly enforced after iterating through all index properties. This ensures the array property validation is complete before checking its position.

packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs (4)

7-8: LGTM!

The new imports for DocumentTypeV0Getters, DocumentPropertyType, and Key are correctly added to support array property detection and per-element key construction.

Also applies to: 18-18

92-97: LGTM!

The array property detection safely falls back to false when the property isn't found in flattened_properties(). This correctly handles system properties like $ownerId which won't be arrays.

146-168: LGTM!

The per-element iteration correctly:

Creates a Key from each encoded element value

Clones the path and appends the element key

Recurses with all_fields_null = false since each element represents a non-null value

This mirrors the add_indices logic for consistency.

170-204: LGTM!

The scalar property removal logic is correctly preserved in the else branch, maintaining the existing behavior for non-array properties.

packages/rs-drive/src/util/object_size_info/document_info.rs (2)

62-70: LGTM!

The trait method signature correctly mirrors the Document method from packages/rs-dpp/src/document/document_methods/mod.rs, enabling consistent array element retrieval across all DocumentInfo variants.

260-282: LGTM with a note on estimation behavior.

The implementation correctly:

Delegates to the document's method for all concrete document variants

Maps ProtocolError to Error::Protocol consistently with other methods

Returns an empty Vec for DocumentEstimatedAverageSize since actual array elements aren't available

Note: For fee estimation involving array-indexed properties, the empty return may underestimate costs since per-element operations won't be counted. If fee accuracy becomes a concern, consider documenting this limitation or adding estimated element counts.

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (3)

14-14: LGTM!

The public re-export of ArrayItemType enables external modules to access this type for array element handling.

1227-1236: LGTM!

The Array case now correctly delegates to item_type.encode_element_for_tree_keys(value) for Contains query support, while VariableTypeArray appropriately remains unsupported for tree key encoding.

2329-2403: LGTM!

The array parsing logic is comprehensive and well-structured:

Correctly parses typed arrays via the items map with support for String, Integer, Number, Boolean, and nested ByteArray

Falls back to ByteArray handling when items is not present

Properly validates that byteArray must be true if defined (lines 2388-2392)

Handles Identifier detection via contentMediaType

The fact that ArrayItemType::Boolean can be parsed here while being rejected during index validation (in try_from_schema) is appropriate - parsing represents the schema structure, while index validation enforces indexing constraints separately.

packages/rs-drive/tests/supporting_files/contract/array-index/post0.json (1)

1-6: LGTM: solid fixture for array-index tests.
Shape matches the intended contract (content string + hashtags string array).

packages/rs-drive/tests/query_tests.rs (3)

7142-7192: Good coverage: validates array property types in both flattened_properties() and properties().
This catches the common "flattened vs nested schema mismatch" failure mode.

7194-7225: Nice: asserts CBOR round-trip yields Value::Array with textual elements.

7277-7338: Good: reproduces serialization using a contract loaded via setup_contract.
This is a useful regression guard if contract-loading paths differ.

packages/rs-drive/tests/supporting_files/contract/array-index/post1.json (1)

1-6: Remove or update post1.json fixture--unused file with invalid base58 identifiers.

post1.json is an orphaned fixture file not referenced anywhere in the codebase. While the $id field does contain non-base58 characters (I, O, 0), these don't break tests since the file is never parsed. Either delete the unused fixture or replace the invalid identifiers if the file serves documentation purposes.

Likely an incorrect or invalid review comment.

packages/rs-dpp/src/data_contract/document_type/property/array.rs Show resolved Hide resolved

.../src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs Show resolved Hide resolved

...rive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs Outdated Show resolved Hide resolved

pauldelucia and others added 3 commits January 10, 2026 21:42


 Merge branch 'v3.0-dev' into feat/arrayElementIndexing

71d8fd4


 fix: coderabbit review fixes

acfa3fd


 Merge branch 'feat/arrayElementIndexing' of https://github.com/dashpa...

59a7378

...y/platform into feat/arrayElementIndexing

coderabbitai bot reviewed

Jan 10, 2026

View reviewed changes

Copy link

Contributor

coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Fix all issues with AI agents

u8::MAX as u16) is incorrect for the add_indices operation; update the message to refer to "add_indices" (e.g., "document field is too big for being an index on add_indices" or similar) in the branch with the condition checking document_top_field_estimated_size and make the identical change for the second occurrence around the other check (the similar FeeError::Overflow at the later location).">In @packages/rs-dpp/src/data_contract/document_type/property/array.rs: - Around line 275-279: ArrayItemType::Date conversion casts an i64 to u64 without checking for negative values, which can wrap and corrupt lexicographic ordering; update the array encoding path to validate the integer from value.to_integer() (the local value_as_i64) is non-negative and return a ProtocolError::ValueError when it's negative, otherwise call DocumentPropertyType::encode_date_timestamp with the safe cast to u64; reference ArrayItemType::Date, value.to_integer(), value_as_i64 and DocumentPropertyType::encode_date_timestamp when making the change. In @packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs: - Around line 163-167: The error message string used in the FeeError::Overflow when checking document_top_field_estimated_size (> u8::MAX as u16) is incorrect for the add_indices operation; update the message to refer to "add_indices" (e.g., "document field is too big for being an index on add_indices" or similar) in the branch with the condition checking document_top_field_estimated_size and make the identical change for the second occurrence around the other check (the similar FeeError::Overflow at the later location).

Nitpick comments (2)

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

176-195: Array detection logic is correct but duplicated.

The array property detection pattern is repeated at lines 177-188 and again at lines 505-518. Consider extracting this into a helper function or caching the result to avoid redundant iteration.

Suggested refactor

+ // Pre-compute which indexes contain array properties + let indexes_with_arrays: Vec<_> = document_type + .indexes() + .values() + .filter(|index| { + index.properties.iter().any(|prop| { + document_type + .flattened_properties() + .get(&prop.name) + .map(|p| { + matches!( + p.property_type, + DocumentPropertyType::Array(_) | DocumentPropertyType::VariableTypeArray(_) + ) + }) + .unwrap_or(false) + }) + }) + .collect(); + // fourth we need to store a reference to the document for each index for index in document_type.indexes().values() { - // Check if this index contains an array property - let index_has_array = index.properties.iter().any(|prop| { - document_type - .flattened_properties() - .get(&prop.name) - .map(|p| { - matches!( - p.property_type, - DocumentPropertyType::Array(_) | DocumentPropertyType::VariableTypeArray(_) - ) - }) - .unwrap_or(false) - }); - - // For indexes with array properties, skip the inline scalar update logic. - if index_has_array { + // Skip array-containing indexes - handled separately below + if indexes_with_arrays.iter().any(|i| std::ptr::eq(*i, index)) { continue; }

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (1)

151-181: Cost estimation inserted redundantly inside element loop.

The estimated_costs_only_with_layer_info.insert at line 169-180 uses index_path.clone() as the key, which is the same for all elements. This means the same entry is overwritten on each iteration. Consider moving this block before the element loop.

Suggested refactor

+ if let Some(estimated_costs_only_with_layer_info) = + estimated_costs_only_with_layer_info + { + let document_top_field_estimated_size = document_and_contract_info + .owned_document_info + .document_info + .get_estimated_size_for_document_type( + name, + document_type, + platform_version, + )?; + + if document_top_field_estimated_size > u8::MAX as u16 { + return Err(Error::Fee(FeeError::Overflow( + "document field is too big for being an index", + ))); + } + + estimated_costs_only_with_layer_info.insert( + KeyInfoPath::from_known_owned_path(index_path.clone()), + EstimatedLayerInformation { + tree_type: TreeType::NormalTree, + estimated_layer_count: PotentiallyAtMaxElements, + estimated_layer_sizes: AllSubtrees( + document_top_field_estimated_size as u8, + NoSumTrees, + storage_flags.map(|s| s.serialized_size()), + ), + }, + ); + } + // For each array element, create an index entry for element_value in array_elements { let element_key = Key(element_value); let path_key_info = element_key.clone().add_path::<0>(index_path.clone()); // Insert tree for this element value self.batch_insert_empty_tree_if_not_exists( path_key_info.clone(), TreeType::NormalTree, storage_flags, apply_type, transaction, previous_batch_operations, batch_operations, drive_version, )?; - if let Some(estimated_costs_only_with_layer_info) = - estimated_costs_only_with_layer_info - { - // ... move this block before the loop - } - let any_fields_null = false; // Not null since we have an element

Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Commits

Reviewing files that changed from the base of the PR and between 313f024 and 59a7378.

Files selected for processing (7)

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Files skipped from review as they are similar to previous changes (1)

packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs

Additional context used

Path-based instructions (1)

**/*.rs

CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Rust code must pass cargo clippy --workspace linter checks
Rust code must be formatted using cargo fmt --all

**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants

Files:

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learnings (15)

Learning: 2024-10-29T14:40:54.727Z

Applied to files:

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learning: 2025-10-01T08:37:32.168Z

Applied to files:

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learning: 2025-06-18T03:44:14.385Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-20T20:43:41.185Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-20T16:05:40.200Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-05-28T16:22:26.334Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-25T01:17:02.001Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-10-21T01:03:42.458Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-10-09T15:59:28.329Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-10-22T10:53:12.111Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-03T10:39:11.242Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-04-11T09:08:05.652Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-11-22T08:19:14.448Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-03-07T12:37:44.555Z

Applied to files:

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Code graph analysis (2)

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (3)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

name (113-135)

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs (2)

PathInfo (169-169)

PathInfo (250-250)

packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs (1)

PathInfo (99-99)

packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs (4)

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs (2)

PathInfo (169-169)

PathInfo (250-250)

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (3)

PathInfo (196-196)

PathInfo (291-291)

element_key (137-137)

packages/rs-drive/src/drive/document/insert_contested/add_contested_indices_for_contract_operations/v0/mod.rs (1)

PathInfo (99-99)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

name (113-135)

Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

GitHub Check: Swift SDK and Example build (warnings as errors)

Additional comments (7)

packages/rs-dpp/src/data_contract/document_type/property/array.rs (1)

255-308: Well-structured tree key encoding implementation.

The method correctly delegates to DocumentPropertyType encoding helpers for proper lexicographic ordering of numeric types, handles null values appropriately, and prevents empty string collision with null by using vec![0].

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

520-550: Remove+add approach for array indexes looks correct.

The pattern of removing old index entries first (with &None for previous batch operations) and then adding new entries (with proper previous_batch_operations) is appropriate. The asymmetric handling makes sense: removal doesn't need conflict checking against pending operations, but insertion does.

packages/rs-drive/src/drive/document/delete/remove_indices_for_top_index_level_for_contract_operations/v0/mod.rs (2)

156-157: Hardcoded null flags for array elements.

Setting any_fields_null = false and all_fields_null = false for array elements assumes that the presence of an element means the field is not null. This is semantically correct - if we have an element value to remove, the field itself isn't null.

105-189: Array element removal logic is well-structured.

The per-element iteration correctly:

Skips empty arrays (no elements to remove)

Creates proper path segments for each element value

Uses consistent PathInfo construction matching the scalar path

Recurses to handle sub-level indices

packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs (1)

153-208: Per-element tree insertion for arrays is correctly implemented.

The logic properly:

Retrieves all array elements (with deduplication handled by the source method)

Skips empty arrays (lines 164-169)

Creates a subtree for each element value (lines 176-189)

Pushes the element to the path and recurses (lines 191-207)

The comment on line 199 (// Not all fields null since we have an element) correctly explains the false value.

packages/rs-drive/src/drive/document/delete/remove_indices_for_index_level_for_contract_operations/v0/mod.rs (1)

133-173: Array element removal at index level is consistent with insertion logic.

The implementation mirrors add_indices_for_index_level_for_contract_operations_v0:

Same array detection pattern

Same empty array handling (continue without action)

Same per-element path construction and recursion

No tree insertion needed (removal only)

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (1)

118-216: Array indexing at top level is correctly implemented.

The implementation correctly:

Detects array properties using the consistent pattern

Skips empty arrays (lines 129-132)

Creates index trees for each element value

Constructs proper PathInfo based on stateful vs stateless operation mode

Recurses with appropriate null flags

packages/rs-dpp/src/data_contract/document_type/property/array.rs Outdated Show resolved Hide resolved

.../src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs Show resolved Hide resolved

fmt

f207c8d

coderabbitai bot reviewed

Jan 11, 2026

View reviewed changes

Copy link

Contributor

coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI agents

In @packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs: - Around line 175-197: The post-loop remove+add functions are re-processing scalar-only indexes (causing duplicate updates); modify the logic so post-loop removal/addition runs only for indexes that contain array properties: inside the first loop that checks index_has_array (the loop iterating document_type.indexes() and computing index_has_array), collect or mark which indexes have arrays and set a flag if any exist, then change the later calls to remove_indices_for_top_index_level_for_contract_operations and add_indices_for_top_index_level_for_contract_operations to either accept/receive only the array-containing index set or be invoked only when the "any index_has_array" flag is true so scalar-only indexes are not processed again (adjust function signatures or add a parameter to filter to array-only indexes if needed).

Nitpick comments (1)

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

176-189: DRY: array-property detection logic is duplicated

The same "is any indexed property an array?" predicate appears twice. Consider extracting a small helper (or compute once and reuse) to avoid divergence bugs and repeated lookups.

Also applies to: 506-520

Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Commits

Reviewing files that changed from the base of the PR and between 59a7378 and f207c8d.

Files selected for processing (2)

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Files skipped from review as they are similar to previous changes (1)

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Additional context used

Path-based instructions (1)

**/*.rs

CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Rust code must pass cargo clippy --workspace linter checks
Rust code must be formatted using cargo fmt --all

**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants

Files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learnings (5)

Learning: 2025-04-11T09:08:05.652Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-10-01T08:37:32.168Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-10-29T14:40:54.727Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-11-22T08:19:14.448Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)

GitHub Check: Rust packages (wasm-dpp) / Tests
GitHub Check: Rust packages (dpp) / Tests
GitHub Check: Rust packages (dpp) / Linting
GitHub Check: Rust packages (dpp) / Unused dependencies
GitHub Check: Rust packages (drive-abci) / Tests
GitHub Check: Rust packages (drive) / Detect immutable structure changes
GitHub Check: Rust packages (wasm-sdk) / Tests
GitHub Check: Rust packages (drive) / Tests
GitHub Check: Rust packages (dapi-grpc) / Tests
GitHub Check: Rust packages (drive) / Unused dependencies
GitHub Check: Rust packages (wasm-dpp2) / Tests
GitHub Check: Rust packages (drive) / Linting
GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image
GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
GitHub Check: Build JS packages / Build JS
GitHub Check: Rust crates security audit

Additional comments (1)

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

22-22: Import looks appropriate for the new behavior

No concerns with bringing DocumentPropertyType into scope here.

...s-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs Outdated Show resolved Hide resolved

pauldelucia and others added 3 commits January 12, 2026 18:24


 Merge branch 'v3.0-dev' into feat/arrayElementIndexing

71cc467


 coderabbit fixes

3becadc


 Merge branch 'feat/arrayElementIndexing' of https://github.com/dashpa...

8952edb

...y/platform into feat/arrayElementIndexing

coderabbitai bot reviewed

Jan 12, 2026

View reviewed changes

Copy link

Contributor

coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Nitpick comments (1)

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (1)

151-181: Consider moving cost estimation outside the element loop.

The cost estimation block (lines 169-180) uses index_path.clone() as the HashMap key, which is the same for all array elements. This means the same entry is being overwritten on each iteration with identical values. While functionally correct, this is redundant work.

Suggested refactor to move estimation before the loop

if array_elements.is_empty() { // Empty array - nothing to index at top level continue; } + if let Some(estimated_costs_only_with_layer_info) = + estimated_costs_only_with_layer_info + { + let document_top_field_estimated_size = document_and_contract_info + .owned_document_info + .document_info + .get_estimated_size_for_document_type( + name, + document_type, + platform_version, + )?; + + if document_top_field_estimated_size > u8::MAX as u16 { + return Err(Error::Fee(FeeError::Overflow( + "document field is too big for being an index", + ))); + } + + estimated_costs_only_with_layer_info.insert( + KeyInfoPath::from_known_owned_path(index_path.clone()), + EstimatedLayerInformation { + tree_type: TreeType::NormalTree, + estimated_layer_count: PotentiallyAtMaxElements, + estimated_layer_sizes: AllSubtrees( + document_top_field_estimated_size as u8, + NoSumTrees, + storage_flags.map(|s| s.serialized_size()), + ), + }, + ); + } + // For each array element, create an index entry for element_value in array_elements { let element_key = Key(element_value); let path_key_info = element_key.clone().add_path::<0>(index_path.clone()); // Insert tree for this element value self.batch_insert_empty_tree_if_not_exists( path_key_info.clone(), TreeType::NormalTree, storage_flags, apply_type, transaction, previous_batch_operations, batch_operations, drive_version, )?; - if let Some(estimated_costs_only_with_layer_info) = - estimated_costs_only_with_layer_info - { - // ... move this block before the loop - } let any_fields_null = false;

Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Commits

Reviewing files that changed from the base of the PR and between f207c8d and 8952edb.

Files selected for processing (4)

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Files skipped from review as they are similar to previous changes (1)

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Additional context used

Path-based instructions (1)

**/*.rs

CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Rust code must pass cargo clippy --workspace linter checks
Rust code must be formatted using cargo fmt --all

**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants

Files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learnings (8)

Learning: 2025-04-11T09:08:05.652Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-10-01T08:37:32.168Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learning: 2024-10-29T14:40:54.727Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-09-30T11:55:43.856Z

Learnt from: shumkov Repo: dashpay/platform PR: 2185 File: packages/rs-drive-abci/src/execution/platform_events/withdrawals/rebroadcast_expired_withdrawal_documents/v1/mod.rs:47-62 Timestamp: 2024-09-30T11:55:43.856Z Learning: In `rebroadcast_expired_withdrawal_documents_v1`, the variable `expired_withdrawal_indices` needs to be a `Vec` rather than a `BTreeSet`, because a vector is necessary for subsequent processing.

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs

Learning: 2024-11-22T08:19:14.448Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-11-03T10:39:11.242Z

Applied to files:

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs

Learning: 2025-03-07T12:37:44.555Z

Applied to files:

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs
packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs

Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)

GitHub Check: Rust packages (wasm-dpp2) / Tests
GitHub Check: Rust packages (rs-sdk-ffi) / Tests
GitHub Check: Rust packages (dash-sdk) / Tests
GitHub Check: Rust packages (dpp) / Tests
GitHub Check: Rust packages (dpp) / Unused dependencies
GitHub Check: Rust packages (dpp) / Linting
GitHub Check: Rust packages (dpp) / Check each feature
GitHub Check: Rust packages (wasm-dpp) / Tests
GitHub Check: Rust packages (rs-dapi-client) / Tests
GitHub Check: Rust packages (wasm-sdk) / Tests
GitHub Check: Rust packages (drive-abci) / Tests
GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image
GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
GitHub Check: Build JS packages / Build JS
GitHub Check: Rust crates security audit

Additional comments (13)

packages/rs-drive/src/drive/document/insert/add_indices_for_top_index_level_for_contract_operations/v0/mod.rs (3)

6-16: LGTM!

New imports for Key and DocumentPropertyType are correctly added and used in the array handling logic below.

106-116: LGTM!

Array property detection correctly identifies both Array and VariableTypeArray types, with safe fallback to false for missing properties.

217-312: LGTM!

Scalar property handling correctly preserves the existing logic, including proper null flag tracking and recursive index processing.

packages/rs-drive/src/drive/document/insert/add_indices_for_index_level_for_contract_operations/v0/mod.rs (5)

6-11: LGTM!

New imports for array handling are correctly added: Key/KeyRef for key construction, DocumentTypeV0Getters for property access, and DocumentPropertyType for type checking.

96-106: LGTM!

Array property detection is consistent with the top-level implementation.

108-151: LGTM!

Property name tree insertion and cost estimation are correctly placed before the array/scalar branching, ensuring the structure is created regardless of property type.

153-208: LGTM!

Array handling correctly:

Documents the semantic difference between empty arrays and null scalars (lines 165-167)

Preserves any_fields_null from parent scope while setting all_fields_null to false for elements

Creates per-element index entries and recurses appropriately

209-259: LGTM!

Scalar property handling correctly preserves existing behavior with proper null flag updates using |= and &= operators.

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (5)

22-22: LGTM!

Import is correctly added and used for the array property type matching.

206-226: LGTM - Remove+add approach for array indexes.

The approach of removing old index entries and adding new ones is sound for handling array-containing indexes. This correctly handles element-level changes in arrays without the complexity of inline diffing.

The use of &None for previous_batch_operations during removal is appropriate since the removal phase should operate unconditionally on the old document state.

173-190: LGTM - Array property detection logic.

The upfront check correctly identifies whether any index contains array-typed properties by looking up each indexed property in the flattened properties map. The unwrap_or(false) fallback is a safe default for properties that may not be found.

228-228: LGTM!

The comment clearly documents that the following optimized inline update logic only applies when no indexes contain array properties.

196-203: No action needed. The code at lines 196-203 correctly handles index removal by using owner_id: None, which causes $ownerId references in indexes to resolve to the document's embedded owner_id. The comment accurately describes this behavior.

pauldelucia added 3 commits January 12, 2026 18:40


 feat: more tests

1c4aa07


 feat(dpp): add array serialization support for document delete/update

6c0950d

fmt

6d6518b

coderabbitai bot reviewed

Jan 12, 2026

View reviewed changes

Copy link

Contributor

coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Fix all issues with AI agents

In @packages/rs-dpp/src/data_contract/document_type/property/mod.rs: - Around line 2347-2395: The array item type match for item_type_str in the items_map parsing is missing the "date" branch; add a case matching "date" that returns ArrayItemType::Date so schemas with items.type == "date" are accepted (update the match arm in the block that maps item_type_str to ArrayItemType inside the items_map handling where ArrayItemType variants like String, Integer, Number, Boolean, Array are matched, and ensure the function still returns DocumentPropertyType::Array(array_item_type)).

Nitpick comments (5)

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (1)

173-190: LGTM - array detection logic is sound.

The upfront check correctly identifies whether any index contains array properties by matching against DocumentPropertyType::Array and DocumentPropertyType::VariableTypeArray. Using unwrap_or(false) safely handles edge cases where properties might not be found in flattened_properties().

For future optimization, consider caching this check at the DocumentType level (e.g., a has_array_indexed_property() method) to avoid repeated iteration on every update. This is a minor concern for typical document types but could matter at scale.

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (1)

772-776: Remove redundant conditional.

Both branches return the same value. The if elements.is_empty() check serves no purpose here.

Proposed fix

- if elements.is_empty() { - Ok((Some(Value::Array(elements)), false)) - } else { - Ok((Some(Value::Array(elements)), false)) - } + Ok((Some(Value::Array(elements)), false))

packages/rs-drive/tests/query_tests.rs (3)

7003-8264: Rename new tests to follow "should ..." naming convention (guidelines).

All newly-added tests are test_*, but **/tests/**/*.rs should have descriptive names starting with should .... Please rename the tests in this module accordingly. As per coding guidelines, ...

Proposed rename pattern (example)

- fn test_query_documents_with_contains_operator() { + fn should_query_documents_with_contains_operator() {

(Apply similarly across the other test_* functions in this module.)

7009-7140: Consider extracting/reusing a shared "contracts tree + contract load" helper to reduce duplication.

setup_array_index_tests repeats the same boilerplate used elsewhere in this file (drive setup, transaction, init contracts tree, apply batch, setup_contract, commit). Given how many tests depend on it, a small helper (e.g., setup_drive_with_contract(path, platform_version)) would cut repetition and lower maintenance cost.

7277-7338: test_array_document_serialize_with_drive_contract: simplify scope + remove explicit drop(drive) unless it's solving a real leak.

The extra block scope and drop(drive) read like leftovers; in tests they typically don't add value, and can make future refactors noisier.

Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Commits

Reviewing files that changed from the base of the PR and between 8952edb and 6c0950d.

Files selected for processing (5)

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs
packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-drive/tests/query_tests.rs
packages/rs-drive/tests/supporting_files/contract/array-index/array-index-with-types-contract.json

Additional context used

Path-based instructions (2)

**/*.rs

CodeRabbit inference engine (CLAUDE.md)

**/*.rs: Rust code must pass cargo clippy --workspace linter checks
Rust code must be formatted using cargo fmt --all

**/*.rs: Use 4-space indent for Rust files
Follow rustfmt defaults and keep code clippy-clean for Rust modules
Use snake_case for Rust module names
Use PascalCase for Rust type names
Use SCREAMING_SNAKE_CASE for Rust constants

Files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/tests/query_tests.rs

**/tests/**/*.{js,jsx,ts,tsx,rs}

CodeRabbit inference engine (AGENTS.md)

**/tests/**/*.{js,jsx,ts,tsx,rs}: Name tests descriptively, starting with 'should ...'
Unit and integration tests should not perform network calls; mock dependencies

Files:

packages/rs-drive/tests/query_tests.rs

Learnings (18)

Learning: 2025-04-11T09:08:05.652Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-10-01T08:37:32.168Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-10-29T14:40:54.727Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-04-11T09:08:05.652Z

Learnt from: pauldelucia Repo: dashpay/platform PR: 2523 File: packages/rs-drive/src/drive/contract/update/update_contract/v1/update_description/v1/mod.rs:147-151 Timestamp: 2025-04-11T09:08:05.652Z Learning: Description length validation for data contracts is already handled in the data contract validation process, specifically in packages/rs-dpp/src/data_contract/methods/validate_update/v0/mod.rs.

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs
packages/rs-dpp/src/data_contract/document_type/property/mod.rs

Learning: 2024-09-30T11:55:43.856Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2024-11-22T08:19:14.448Z

Applied to files:

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs

Learning: 2025-06-18T03:44:14.385Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-20T20:43:41.185Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-20T16:05:40.200Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-05-28T16:22:26.334Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-25T01:17:02.001Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs
packages/rs-drive/tests/query_tests.rs

Learning: 2024-10-21T01:03:42.458Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2025-10-09T15:59:28.329Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-10-22T10:53:12.111Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-11-03T10:39:11.242Z

Applied to files:

packages/rs-dpp/src/data_contract/document_type/property/array.rs

Learning: 2024-12-10T10:56:26.215Z

Applied to files:

packages/rs-drive/tests/supporting_files/contract/array-index/array-index-with-types-contract.json

Learning: 2024-11-25T07:49:05.419Z

Learnt from: shumkov Repo: dashpay/platform PR: 2345 File: packages/wallet-utils-contract/schema/v1/wallet-utils-contract-documents.json:49-55 Timestamp: 2024-11-25T07:49:05.419Z Learning: In the schema definitions for the `wallet-utils-contract` (file `packages/wallet-utils-contract/schema/v1/wallet-utils-contr act-documents.json`), the `$createdAt` field is a system field augmented by DPP and does not need to be defined explicitly in the schema's properties.

Applied to files:

packages/rs-drive/tests/supporting_files/contract/array-index/array-index-with-types-contract.json

Learning: 2024-10-30T11:19:59.163Z

Applied to files:

packages/rs-drive/tests/query_tests.rs

Code graph analysis (1)

packages/rs-dpp/src/data_contract/document_type/property/array.rs (1)

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (7)

value (2469-2469)

value (2470-2470)

value (2485-2485)

get_field_type_matching_error (2459-2464)

encode_date_timestamp (1507-1509)

encode_i64 (1655-1678)

encode_float (1981-2018)

Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (20)

GitHub Check: Rust packages (dash-sdk) / Tests
GitHub Check: Rust packages (wasm-sdk) / Tests
GitHub Check: Rust packages (drive) / Linting
GitHub Check: Rust packages (drive) / Unused dependencies
GitHub Check: Rust packages (drive) / Tests
GitHub Check: Rust packages (drive-abci) / Tests
GitHub Check: Rust packages (wasm-dpp2) / Tests
GitHub Check: Rust packages (dpp) / Tests
GitHub Check: Rust packages (dapi-grpc) / Tests
GitHub Check: Rust packages (dpp) / Check each feature
GitHub Check: Rust packages (dpp) / Linting
GitHub Check: Rust packages (rs-sdk-ffi) / Tests
GitHub Check: Rust packages (dpp) / Unused dependencies
GitHub Check: Rust packages (wasm-dpp) / Tests
GitHub Check: Build Docker images (Dashmate helper, dashmate-helper, dashmate-helper) / Build Dashmate helper image
GitHub Check: Build Docker images (Drive, drive, drive-abci, SDK_TEST_DATA=true
) / Build Drive image
GitHub Check: Build Docker images (RS-DAPI, rs-dapi, rs-dapi) / Build RS-DAPI image
GitHub Check: Build JS packages / Build JS
GitHub Check: Rust crates security audit
GitHub Check: Swift SDK and Example build (warnings as errors)

Additional comments (12)

packages/rs-drive/src/drive/document/update/internal/update_document_for_contract_operations/v0/mod.rs (3)

22-22: LGTM!

The import of DocumentPropertyType is necessary for the pattern matching on Array and VariableTypeArray variants in the new array detection logic.

242-243: LGTM!

The comment clearly documents that the following code path is specifically for scalar-only indexes, providing good context for future maintainers.

192-240: Two-phase remove+add approach is sound.

The strategy of applying removes first, then adds, correctly handles overlapping array elements between old and new documents by preventing GroveDB conflicts when the same key is both removed and inserted.

Key implementation details verified:

Line 199: owner_id: None correctly signals "use owner from the document" (consistent with line 272)

Line 213: Passing &None for previous_batch_operations properly isolates removal logic from pending operations

Lines 221-227: apply_batch_low_level_drive_operations executes remove operations within the transaction context before add operations proceed, maintaining operation ordering and atomicity

packages/rs-dpp/src/data_contract/document_type/property/array.rs (4)

1-7: LGTM!

The new imports are appropriate for the added functionality: DocumentPropertyType for delegating tree-key encoding to scalar type helpers, VarIntReader for decoding length-prefixed data in read_from, and std::io::{BufRead, Read} for the buffer reading interface.

256-318: Well-designed tree-key encoding for array elements.

The implementation correctly:

Mirrors the scalar encoding in DocumentPropertyType::encode_value_for_tree_keys

Uses proper lexicographic ordering helpers (encode_i64, encode_float, encode_date_timestamp)

Handles edge cases (null - empty vec, empty string - [0] to avoid collision)

Validates date timestamps are non-negative

346-355: Verify Date type encoding/decoding symmetry.

The read_from method decodes Date as f64::from_be_bytes and returns Value::Float. However, looking at encode_value_ref_with_size (line 216-219), Date is encoded with value.to_float()?.to_be_bytes(). This appears consistent.

However, the AI summary and PR objectives mention that dates should be timestamps (integers). If dates are intended to be stored as u64 timestamps elsewhere (as seen in the tree-key encoding using encode_date_timestamp), there may be an inconsistency between serialization and tree-key encoding formats. Please verify this is the intended behavior.

410-418: Boolean decoding is correct.

The decoding logic (bool_byte[0] != 0 means true) correctly matches the encoding in encode_value_ref_with_size where true - [1] and false - [0].

packages/rs-dpp/src/data_contract/document_type/property/mod.rs (3)

14-14: LGTM!

The public re-export of ArrayItemType appropriately exposes the type for downstream crates that need to work with array element encoding for indexing.

1245-1254: LGTM!

The implementation correctly delegates single-element tree-key encoding to ArrayItemType::encode_element_for_tree_keys for Contains query support. The error handling for VariableTypeArray is appropriately explicit.

2396-2421: LGTM!

The fallback logic correctly handles the byte array case with clear error messages when the schema is malformed. The contentMediaType check for Identifier detection is consistent with the existing pattern.

packages/rs-drive/tests/query_tests.rs (1)

7502-7542: The test assertions are correct for String arrays.

For ArrayItemType::String, encode_element_for_tree_keys returns value_as_text.as_bytes().to_vec() for non-empty strings (lines 268-273 in packages/rs-dpp/src/data_contract/document_type/property/array.rs), which is identical to raw UTF-8 bytes. The assertion elements.iter().any(|e| e == b"alpha") correctly matches the encoding used.

Note: This is specific to String arrays. Non-string array types (Integer, Date, Number, Boolean) use different encodings (sign-bit flip, timestamp encoding, float encoding, etc.), but this test only uses String arrays, so the assertions are valid.

packages/rs-drive/tests/supporting_files/contract/array-index/array-index-with-types-contract.json (1)

1-82: Well-structured test contract covering multiple array types.

This test fixture provides good coverage for the new array indexing feature:

String arrays (tags)

Integer arrays (scores)

Byte array arrays (identifiers with 32-byte items)

All indices are single-property, satisfying the constraints that arrays must be last in compound indexes and only one array per index.

One minor observation: the indices don't have explicit name fields. If the system auto-generates names this is fine, but explicit names can aid debugging in test failures.

Conversation

pauldelucia commented Jan 10, 2026 * edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue being fixed or feature implemented

What was done?

How Has This Been Tested?

Breaking Changes

Checklist:

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 10, 2026 * edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pauldelucia commented Jan 13, 2026

Uh oh!

coderabbitai bot commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pauldelucia commented Jan 10, 2026 *

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 10, 2026 *

edited

Loading