Add MIP-11 Support to DAS Indexer #268
Conversation
Summary by CodeRabbitRelease Notes
WalkthroughAdds a Postgres migration replacing a unique constraint with two partial unique indexes for asset_grouping; updates transformer upserts/inserts to use partial-index-aware ON CONFLICT semantics and bulk inserts; prunes stale asset_grouping entries in DAO; bumps a workspace dependency; ignores Changes
Sequence Diagram(s)sequenceDiagram
participant Transformer
participant DB as Postgres
note right of Transformer #E8F6FF: High-level save/upsert flow
Transformer->>DB: BEGIN TRANSACTION
Transformer->>DB: INSERT MANY asset_grouping rows
alt collection conflict (partial index)
DB-->>Transformer: ON CONFLICT (asset_id, group_key) WHERE group_key='collection'\nDO UPDATE ... WHEN EXCLUDED.slot_updated >= asset_grouping.slot_updated
else other-group conflict (partial index)
DB-->>Transformer: ON CONFLICT (asset_id, group_key, group_value) WHERE group_key!='collection'\nDO UPDATE ... WHEN EXCLUDED.slot_updated >= asset_grouping.slot_updated
end
Transformer->>DB: COMMIT
DB-->>Transformer: OK / Error
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.toml(1 hunks)integration_tests/.gitignore(1 hunks)program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-api (ubuntu-22.04)
🔇 Additional comments (6)
integration_tests/.gitignore (1)
1-1: LGTM!Adding
.envto.gitignoreis a best practice to prevent accidental commits of sensitive environment variables and credentials.program_transformers/src/mpl_core_program/v1_asset.rs (5)
29-29: LGTM!The
Exprimport is correctly added to support query construction in the asset grouping logic.
369-380: LGTM!The logic correctly fetches the last slot_updated for the "group" key, with appropriate error handling and safe defaults.
369-423: Verify transaction isolation and error handling.The Groups plugin indexing logic is correctly wrapped within the existing transaction (started at line 108, committed at line 507), ensuring atomicity with other asset updates. Error handling properly propagates database errors through
map_err.One consideration: if the Groups plugin update fails, the entire asset save transaction rolls back. Confirm this is the desired behavior vs. allowing the asset to save without group indexing.
382-396: Verify slot ordering guarantees and reorg handling
Confirm that your upstream event source guarantees monotonically increasing slot values per asset (to satisfy thelast_group_slot_updated < slot_iassumption) or introduce reorg-safe logic to prevent stale or out-of-order deletes/inserts when slots are replayed.
398-422: Verify Groups plugin verification flag and duplicate handling
- Hardcoding
verified = trueassumes all Groups entries are always verified; confirm this aligns with the MIP-11 Groups spec.- Ensure
group.to_string()produces the correct on-chain identifier (pubkey) format, not a debug or alternate representation.- Although old groupings are cleared in the same transaction, duplicates within
groups.groupscould still cause insertion errors—verify input de-duplication.
| mpl-bubblegum = "2.0.0" | ||
| mpl-account-compression = "0.4.2" | ||
| mpl-core = {version = "0.10.0", features = ["serde"]} | ||
| mpl-core = {version = "=0.10.1-alpha.2", features = ["serde"]} |
There was a problem hiding this comment.
🧩 Analysis chain
Verify alpha version stability and consider version constraint.
Using an exact-pinned alpha version (=0.10.1-alpha.2) in the dependency may cause issues:
- Alpha version risk: Alpha releases are typically unstable and may contain bugs or breaking changes.
- Exact version pinning: The
=constraint prevents any updates, even patch fixes. Consider using~for patch-level flexibility once a stable version is available.
Verify the stability of this alpha version and check if a stable release is available:
🌐 Web query:
What is the latest stable version of mpl-core Rust crate?
💡 Result:
The latest stable mpl-core crate version is 0.10.1 (checked Sep 30, 2025). [1][2]
Sources:
[1] docs.rs — mpl-core 0.10.1.
[2] lib.rs — mpl-core (shows version 0.10.1 in Cargo features).
Update mpl-core to the stable 0.10.1 release and loosen the version constraint.
Replace the exact alpha pin (=0.10.1-alpha.2) with a caret requirement (^0.10.1) to use the latest stable release and receive future patch fixes.
🤖 Prompt for AI Agents
In Cargo.toml at line 72, the mpl-core dependency is pinned to the exact alpha
release "=0.10.1-alpha.2"; change this to a caret requirement "^0.10.1" so the
project uses the stable 0.10.1 release and allows compatible patch upgrades
(update the version string accordingly in that dependency entry).
| // Only perform updates if the last asset_grouping for group plugin slot_updated is less than the current slot for the asset update. | ||
| if last_group_slot_updated < slot_i { |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Document the different slot comparison strategies.
There's an intentional difference in slot comparison logic between Groups and Collection groupings:
- Line 383 (Groups): Uses strict less-than (
<) to gate the entire update block, preventing unnecessary deletes/inserts when slots are equal. - Line 452 (Collection): Uses greater-or-equal (
>=) in the WHERE clause, allowing updates when slots are equal.
This difference is likely intentional - Groups plugin requires delete-then-insert (no ON CONFLICT), while Collection uses upsert semantics. Consider adding a comment explaining this design choice for future maintainers.
- // Only perform updates if the last asset_grouping for group plugin slot_updated is less than the current slot for the asset update.
+ // Only perform updates if the last asset_grouping for group plugin slot_updated is less than the current slot for the asset update.
+ // Note: We use strict < comparison here (vs >= for collections) because Groups requires delete-then-insert,
+ // so we skip the entire operation when slots are equal to avoid unnecessary database operations.
if last_group_slot_updated < slot_i {Also applies to: 452-453
🤖 Prompt for AI Agents
In program_transformers/src/mpl_core_program/v1_asset.rs around lines 382-383
and 452-453, the slot comparison behavior differs: Groups uses strict less-than
(`<`) to skip updates when slots are equal, while Collection uses
greater-or-equal (`>=`) to allow updates when slots are equal; add a concise
comment at each location explaining this deliberate design choice — that Groups
performs delete-then-insert (no ON CONFLICT) so we must avoid touching records
when slots are equal, whereas Collection uses upsert semantics so equal-slot
updates are allowed — include the intent and the relation to delete-vs-upsert
behavior so future maintainers understand why the comparisons differ.
| txn.execute(query) | ||
| .await | ||
| .map_err(|db_err| ProgramTransformerError::AssetIndexError(db_err.to_string()))?; |
There was a problem hiding this comment.
We might want to have the delete and insert for this part of the same txn so if there's a failure they are reverted together
There was a problem hiding this comment.
I have updated the indexer to only fire off a single query for any of the cases - asset without group/empty group just deletes old groupings, while asset with groups now performs deletion and insertion using CTEs.
…ex constraint to allow multiple relations that are not collection relations
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
migration/src/lib.rs(2 hunks)migration/src/m20251008_145049_fix_asset_grouping_constraints.rs(1 hunks)program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check lock file, fmt, clippy
program_transformers/src/mpl_core_program/v1_asset.rs
[error] 368-368: rustfmt: formatting differences detected in block building the group variable; please run 'cargo fmt' to fix.
[error] 389-389: rustfmt: formatting differences detected around the formatted SQL value construction; please run 'cargo fmt' to fix.
[error] 430-430: rustfmt: formatting differences detected in statement execution; please run 'cargo fmt' to fix.
[error] 455-455: rustfmt: formatting differences detected in transaction error handling; please run 'cargo fmt' to fix.
migration/src/m20251008_145049_fix_asset_grouping_constraints.rs
[error] 29-29: rustfmt: formatting differences detected. The diff shows misplaced string concatenation around the WHERE clause; please run 'cargo fmt' to fix.
[error] 41-41: rustfmt: formatting differences detected. The diff shows misplaced string concatenation around the WHERE clause; please run 'cargo fmt' to fix.
⏰ 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: build-api (ubuntu-22.04)
🔇 Additional comments (3)
program_transformers/src/mpl_core_program/v1_asset.rs (1)
29-30: LGTM: Import additions support the new SQL-based grouping logic.The
Statementand additionalsea_queryimports are necessary for the raw SQL operations used in the asset_grouping handling block below.migration/src/lib.rs (1)
54-54: LGTM: Migration registration is correct.The new migration module is properly registered in both the module declaration and the migrations list.
Also applies to: 115-115
migration/src/m20251008_145049_fix_asset_grouping_constraints.rs (1)
51-85: LGTM: Rollback logic properly reverses the migration.The
down()method correctly drops the partial indexes and recreates the original unique constraint, providing a clean rollback path.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: Check lock file, fmt, clippy
program_transformers/src/mpl_core_program/v1_asset.rs
[error] 1-1: cargo fmt --all -- --check failed. Rust source files require formatting changes in this file.
⏰ 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: build-api (ubuntu-22.04)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
migration/src/m20251008_145049_fix_asset_grouping_constraints.rs(1 hunks)program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)
⏰ 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). (2)
- GitHub Check: build-api (ubuntu-22.04)
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-api (ubuntu-22.04)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)
⏰ 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). (2)
- GitHub Check: build-api (ubuntu-22.04)
- GitHub Check: test
There was a problem hiding this comment.
Actionable comments posted: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
digital_asset_types/src/dao/scopes/asset.rs(4 hunks)program_transformers/src/bubblegum/db.rs(1 hunks)program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)program_transformers/src/token_metadata/v1_asset.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
digital_asset_types/src/dao/scopes/asset.rs (1)
program_transformers/src/mpl_core_program/v1_asset.rs (2)
asset(727-732)groups(480-491)
program_transformers/src/mpl_core_program/v1_asset.rs (1)
integration_tests/tests/integration_tests/general_scenario_tests.rs (1)
vec(45-84)
🪛 GitHub Actions: Check lock file, fmt, clippy
program_transformers/src/bubblegum/db.rs
[error] 407-407: cargo fmt --check failed: Rust formatting changes detected in db.rs. Run 'cargo fmt' to fix.
digital_asset_types/src/dao/scopes/asset.rs
[error] 639-639: cargo fmt --check failed: Rust formatting changes detected in asset.rs. Run 'cargo fmt' to fix.
[error] 650-650: cargo fmt --check failed: Rust formatting changes detected in asset.rs. Run 'cargo fmt' to fix.
program_transformers/src/mpl_core_program/v1_asset.rs
[error] 491-491: cargo fmt --check failed: Rust formatting changes detected in v1_asset.rs. Run 'cargo fmt' to fix.
program_transformers/src/token_metadata/v1_asset.rs
[error] 342-342: cargo fmt --check failed: Rust formatting changes detected in v1_asset.rs. Run 'cargo fmt' to fix.
⏰ 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: build-api (ubuntu-22.04)
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
digital_asset_types/src/dao/scopes/asset.rs(4 hunks)program_transformers/src/bubblegum/db.rs(1 hunks)program_transformers/src/mpl_core_program/v1_asset.rs(2 hunks)program_transformers/src/token_metadata/v1_asset.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
digital_asset_types/src/dao/scopes/asset.rs (1)
program_transformers/src/mpl_core_program/v1_asset.rs (2)
asset(722-727)groups(477-488)
⏰ 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). (2)
- GitHub Check: test
- GitHub Check: build-api (ubuntu-22.04)
🔇 Additional comments (1)
digital_asset_types/src/dao/scopes/asset.rs (1)
646-662: Handle NULL slots when pruning stale groups.Rows inserted earlier can have
slot_updated = NULL.max()skips NULL, so once any non-NULL row exists we never drop the NULL entries, even though they’re stale. They’ll keep surfacing inasset.groups. Please treat NULL as “lowest” by retaining only rows whereslot_updatedis NULL and every non-collection row was NULL, or simply drop NULL rows once a concrete max exists. Example fix:- if let Some(max_slot_updated) = max_slot_updated { - asset_groupings_with_date - .retain(|ag| ag.0.group_key != "group" || ag.0.slot_updated == max_slot_updated); - } + if let Some(max_slot_updated) = max_slot_updated { + asset_groupings_with_date.retain(|ag| { + ag.0.group_key != "group" + || ag.0.slot_updated.map_or(false, |slot| slot == max_slot_updated) + }); + }This keeps only the freshest
'group'rows.Likely an incorrect or invalid review comment.
danenbm
left a comment
danenbm
left a comment
There was a problem hiding this comment.
Change LGTM just had the one question and then also I assume you will want to add testing at some point after its deployed to devnet
| fn filter_out_stale_asset_groupings( | ||
| asset_groupings_with_date: &mut Vec<(asset_grouping::Model, Option<asset_data::Model>)>, | ||
| ) { | ||
| // For core assets, any asset groupings that do not have the max | ||
| // `slot_updated` value are stale and should be removed. | ||
| // However, only filter out groupings where group_key is "group". | ||
| // All other groupings should be kept. | ||
| let max_slot_updated = asset_groupings_with_date | ||
| .iter() | ||
| .filter(|ag| ag.0.group_key == "group") | ||
| .map(|ag| ag.0.slot_updated) | ||
| .max(); | ||
| if let Some(max_slot_updated) = max_slot_updated { | ||
| asset_groupings_with_date | ||
| .retain(|ag| ag.0.group_key != "group" || ag.0.slot_updated == max_slot_updated); | ||
| } | ||
| } |
There was a problem hiding this comment.
Does this need to do any specific handling with the empty_groups empty Vec?
There was a problem hiding this comment.
You're right - since I'm inserting a grouping with a null value, I should filter out that grouping, so that nothing renders in cases where all groups were removed.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
digital_asset_types/src/dao/scopes/asset.rs(4 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pileks
PR: metaplex-foundation/digital-asset-rpc-infrastructure#268
File: program_transformers/src/mpl_core_program/v1_asset.rs:493-505
Timestamp: 2025-10-13T14:09:58.534Z
Learning: In the MIP-11 Groups plugin implementation for program_transformers/src/mpl_core_program/v1_asset.rs, the team has decided not to delete stale group rows before upserting because same-slot group changes are not expected to be common. They will adjust if analysis shows it becomes a problem.
📚 Learning: 2025-10-13T14:09:58.534Z
Learnt from: pileks
PR: metaplex-foundation/digital-asset-rpc-infrastructure#268
File: program_transformers/src/mpl_core_program/v1_asset.rs:493-505
Timestamp: 2025-10-13T14:09:58.534Z
Learning: In the MIP-11 Groups plugin implementation for program_transformers/src/mpl_core_program/v1_asset.rs, the team has decided not to delete stale group rows before upserting because same-slot group changes are not expected to be common. They will adjust if analysis shows it becomes a problem.
Applied to files:
digital_asset_types/src/dao/scopes/asset.rs
🧬 Code graph analysis (1)
digital_asset_types/src/dao/scopes/asset.rs (1)
program_transformers/src/mpl_core_program/v1_asset.rs (2)
asset(722-727)groups(477-488)
⏰ 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). (2)
- GitHub Check: build-api (ubuntu-22.04)
- GitHub Check: test
🔇 Additional comments (2)
digital_asset_types/src/dao/scopes/asset.rs (2)
403-407: LGTM! Stale grouping filtering correctly positioned.The call to
filter_out_stale_asset_groupingsnow correctly runs after groups have been populated (lines 384-401), ensuring stale "group" entries are actually pruned before returning assets. This addresses the issue raised in the previous review where filtering occurred too early whenasset.groupswas still empty.
498-513: LGTM! Consistent stale grouping filtering for single-asset queries.The stale grouping filter is correctly applied after groups are assembled (lines 498-510), maintaining consistency with the batch query path in
get_related_for_assets.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
digital_asset_types/src/dao/scopes/asset.rs(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pileks
PR: metaplex-foundation/digital-asset-rpc-infrastructure#268
File: program_transformers/src/mpl_core_program/v1_asset.rs:493-505
Timestamp: 2025-10-13T14:09:58.534Z
Learning: In the MIP-11 Groups plugin implementation for program_transformers/src/mpl_core_program/v1_asset.rs, the team has decided not to delete stale group rows before upserting because same-slot group changes are not expected to be common. They will adjust if analysis shows it becomes a problem.
📚 Learning: 2025-10-13T14:09:58.534Z
Learnt from: pileks
PR: metaplex-foundation/digital-asset-rpc-infrastructure#268
File: program_transformers/src/mpl_core_program/v1_asset.rs:493-505
Timestamp: 2025-10-13T14:09:58.534Z
Learning: In the MIP-11 Groups plugin implementation for program_transformers/src/mpl_core_program/v1_asset.rs, the team has decided not to delete stale group rows before upserting because same-slot group changes are not expected to be common. They will adjust if analysis shows it becomes a problem.
Applied to files:
digital_asset_types/src/dao/scopes/asset.rs
🧬 Code graph analysis (1)
digital_asset_types/src/dao/scopes/asset.rs (1)
program_transformers/src/mpl_core_program/v1_asset.rs (2)
asset(722-727)groups(477-488)
⏰ 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). (2)
- GitHub Check: build-api (ubuntu-22.04)
- GitHub Check: test
🔇 Additional comments (3)
digital_asset_types/src/dao/scopes/asset.rs (3)
407-410: LGTM! Correct placement of stale-group filtering.The filter is correctly invoked after group population completes, addressing the previous review concern. However, this will only become effective once the query filters at lines 367-371 are updated to fetch "group" entries (see separate comment).
Based on learnings: The team decided not to delete stale rows in the transformer, so filtering happens here at query time instead.
506-520: LGTM! Consistent stale-group filtering in both branches.The filter is correctly applied after groups are assembled, whether
show_collection_metadatais true or false. This will work properly once the query at lines 492-496 is fixed to fetch "group" entries.
654-672: LGTM! Stale-group filtering logic is sound.The function correctly:
- Filters only
group_key = "group"entries byslot_updated, preserving all other groupings- Retains the most recent "group" entries (max slot) and drops stale ones
- Filters out null-valued "group" entries (used to mark group removal)
Note: This function is currently unreachable because the queries at lines 367-371 and 492-496 exclude "group" entries. It will become functional once those filters are fixed.
Optional: The
ag.0.group_value.is_some()check on line 669 is redundant since upstream queries already filtergroup_value.is_not_null(). However, this defensive check is harmless and guards against future code paths that might skip the query filter.Based on learnings: The team decided not to delete stale group rows in the transformer because same-slot group changes aren't expected to be common, so pruning happens here at query time instead.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
digital_asset_types/src/dao/scopes/asset.rs(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: pileks
PR: metaplex-foundation/digital-asset-rpc-infrastructure#268
File: program_transformers/src/mpl_core_program/v1_asset.rs:493-505
Timestamp: 2025-10-13T14:09:58.534Z
Learning: In the MIP-11 Groups plugin implementation for program_transformers/src/mpl_core_program/v1_asset.rs, the team has decided not to delete stale group rows before upserting because same-slot group changes are not expected to be common. They will adjust if analysis shows it becomes a problem.
📚 Learning: 2025-10-13T14:09:58.534Z
Learnt from: pileks
PR: metaplex-foundation/digital-asset-rpc-infrastructure#268
File: program_transformers/src/mpl_core_program/v1_asset.rs:493-505
Timestamp: 2025-10-13T14:09:58.534Z
Learning: In the MIP-11 Groups plugin implementation for program_transformers/src/mpl_core_program/v1_asset.rs, the team has decided not to delete stale group rows before upserting because same-slot group changes are not expected to be common. They will adjust if analysis shows it becomes a problem.
Applied to files:
digital_asset_types/src/dao/scopes/asset.rs
🧬 Code graph analysis (1)
digital_asset_types/src/dao/scopes/asset.rs (1)
program_transformers/src/mpl_core_program/v1_asset.rs (2)
asset(722-727)groups(477-488)
⏰ 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). (2)
- GitHub Check: build-api (ubuntu-22.04)
- GitHub Check: test
🔇 Additional comments (1)
digital_asset_types/src/dao/scopes/asset.rs (1)
413-416: LGTM: stale-group pruning now occurs after population.Correct placement; this ensures stale rows are actually removed before returning assets.
| .filter( | ||
| Condition::any() | ||
| // Include collection groupings only if they have a non-null value | ||
| .add( | ||
| asset_grouping::Column::GroupKey | ||
| .eq("collection") | ||
| .and(asset_grouping::Column::GroupValue.is_not_null()), | ||
| ) | ||
| // Include all other groupings (regardless of value) | ||
| .add(asset_grouping::Column::GroupKey.ne("collection")), | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Whitelist group keys instead of using ne("collection") (clearer and more index-friendly).
Using GroupKey.ne("collection") may pull unrelated future keys and can prevent partial index use. If the intent is to return only legacy collections and MIP‑11 groups, explicitly match those keys.
Apply in both queries:
@@
- .filter(
- Condition::any()
- // Include collection groupings only if they have a non-null value
- .add(
- asset_grouping::Column::GroupKey
- .eq("collection")
- .and(asset_grouping::Column::GroupValue.is_not_null()),
- )
- // Include all other groupings (regardless of value)
- .add(asset_grouping::Column::GroupKey.ne("collection")),
- )
+ .filter(
+ Condition::any()
+ // Include collection groupings only if they have a non-null value
+ .add(
+ asset_grouping::Column::GroupKey
+ .eq("collection")
+ .and(asset_grouping::Column::GroupValue.is_not_null()),
+ )
+ // Include MIP-11 group rows (including nulls to detect removals)
+ .add(asset_grouping::Column::GroupKey.eq("group")),
+ )Repeat the same change in the single-asset query:
@@
- .filter(
- Condition::any()
- // Include collection groupings only if they have a non-null value
- .add(
- asset_grouping::Column::GroupKey
- .eq("collection")
- .and(asset_grouping::Column::GroupValue.is_not_null()),
- )
- // Include all other groupings (regardless of value)
- .add(asset_grouping::Column::GroupKey.ne("collection")),
- )
+ .filter(
+ Condition::any()
+ // Include collection groupings only if they have a non-null value
+ .add(
+ asset_grouping::Column::GroupKey
+ .eq("collection")
+ .and(asset_grouping::Column::GroupValue.is_not_null()),
+ )
+ // Include MIP-11 group rows (including nulls to detect removals)
+ .add(asset_grouping::Column::GroupKey.eq("group")),
+ )Also consider extracting this predicate into a small helper to keep both call sites in sync.
Also applies to: 498-508
🤖 Prompt for AI Agents
In digital_asset_types/src/dao/scopes/asset.rs around lines 367-377 (and
similarly at lines 498-508), the current predicate uses
GroupKey.ne("collection") which is broad and harms index usage; replace the
negative check with an explicit whitelist of allowed group keys (e.g., legacy
collection key and the MIP-11 group keys you expect) by changing the
.add(asset_grouping::Column::GroupKey.ne("collection")) clause to a
.add(Condition::any().add(asset_grouping::Column::GroupKey.eq("legacy_collection_key")).add(asset_grouping::Column::GroupKey.eq("mip11_key1")).add(...))
or equivalent whitelist checks, and extract that predicate into a small helper
function used by both query sites so the logic stays consistent.
| .filter( | ||
| Condition::any() | ||
| // Include collection groupings only if they have a non-null value | ||
| .add( | ||
| asset_grouping::Column::GroupKey | ||
| .eq("collection") | ||
| .and(asset_grouping::Column::GroupValue.is_not_null()), | ||
| ) | ||
| // Include all other groupings (regardless of value) | ||
| .add(asset_grouping::Column::GroupKey.ne("collection")), | ||
| ) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Deduplicate grouping-query construction to avoid drift between code paths.
Both the batch and single-asset paths build near-identical predicates and then apply the same stale pruning. Extract a helper (e.g., fn grouping_key_filter() -> Condition) and reuse it in both places to prevent future inconsistencies.
Example helper (outside these ranges):
fn grouping_key_filter() -> Condition {
Condition::any()
.add(
asset_grouping::Column::GroupKey
.eq("collection")
.and(asset_grouping::Column::GroupValue.is_not_null()),
)
.add(asset_grouping::Column::GroupKey.eq("group"))
}Also applies to: 518-518, 532-533
🤖 Prompt for AI Agents
In digital_asset_types/src/dao/scopes/asset.rs around lines 498-508 (and
similarly at 518 and 532-533), the grouping predicate is duplicated across the
batch and single-asset query paths; extract a helper function fn
grouping_key_filter() -> Condition that returns the shared Condition
construction (collection with non-null value OR the other grouping key), place
it outside these ranges (e.g., near other helper fns), and replace the inline
Condition::any() blocks at lines 498-508, 518, and 532-533 with calls to
grouping_key_filter() so both paths reuse the exact same predicate and avoid
drift.
| fn filter_out_stale_asset_groupings( | ||
| asset_groupings_with_date: &mut Vec<(asset_grouping::Model, Option<asset_data::Model>)>, | ||
| ) { | ||
| // For core assets, any asset groupings that do not have the max | ||
| // `slot_updated` value are stale and should be removed. | ||
| // However, only filter out groupings where group_key is "group". | ||
| // All other groupings should be kept. | ||
| let max_slot_updated = asset_groupings_with_date | ||
| .iter() | ||
| .filter(|ag| ag.0.group_key == "group") | ||
| .map(|ag| ag.0.slot_updated) | ||
| .max(); | ||
| if let Some(max_slot_updated) = max_slot_updated { | ||
| asset_groupings_with_date.retain(|ag| { | ||
| ag.0.group_key != "group" | ||
| || (ag.0.slot_updated == max_slot_updated && ag.0.group_value.is_some()) | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Simplify stale-group filtering; avoid Option<Option<_>> max while preserving behavior.
Minor readability refactor that keeps semantics identical (including the “null value at max slot means removal” rule).
fn filter_out_stale_asset_groupings(
asset_groupings_with_date: &mut Vec<(asset_grouping::Model, Option<asset_data::Model>)>,
) {
- // For core assets, any asset groupings that do not have the max
- // `slot_updated` value are stale and should be removed.
- // However, only filter out groupings where group_key is "group".
- // All other groupings should be kept.
- let max_slot_updated = asset_groupings_with_date
- .iter()
- .filter(|ag| ag.0.group_key == "group")
- .map(|ag| ag.0.slot_updated)
- .max();
- if let Some(max_slot_updated) = max_slot_updated {
- asset_groupings_with_date.retain(|ag| {
- ag.0.group_key != "group"
- || (ag.0.slot_updated == max_slot_updated && ag.0.group_value.is_some())
- });
- }
+ // Keep only the latest "group" rows; allow null at max slot to signal removal.
+ let max_slot_updated = asset_groupings_with_date
+ .iter()
+ .filter(|ag| ag.0.group_key == "group")
+ .filter_map(|ag| ag.0.slot_updated)
+ .max();
+ if let Some(max_slot_updated) = max_slot_updated {
+ asset_groupings_with_date.retain(|ag| {
+ ag.0.group_key != "group"
+ || (ag.0.slot_updated == Some(max_slot_updated) && ag.0.group_value.is_some())
+ });
+ } else {
+ // No slot info; keep only non-null "group" values.
+ asset_groupings_with_date
+ .retain(|ag| ag.0.group_key != "group" || ag.0.group_value.is_some());
+ }
}Based on learnings
🤖 Prompt for AI Agents
digital_asset_types/src/dao/scopes/asset.rs lines 666-684: simplify the
stale-group filtering by computing the maximum slot_updated across group_key ==
"group" entries directly and then retaining only non-"group" entries or "group"
entries that both match that max slot and have a non-None group_value; replace
any mapping that produces an Option<Option<_>> for the max with a direct max on
slot_updated and then use a retain closure that checks ag.0.group_key != "group"
|| (ag.0.slot_updated == max_slot && ag.0.group_value.is_some()) so behavior
(including dropping null group_value at the max slot) is preserved.
Summary
mpl-coredependency to0.10.1-alpha.2to support MIP-11 Groups pluginasset_groupingtableChanges
mpl-coreversion inCargo.tomlandCargo.lockv1_asset.rsto handle Groups plugin indexing with proper slot-based update logic.gitignorefor integration tests.envfileFuture work