-
Notifications
You must be signed in to change notification settings - Fork 42
Add BubblegumV2 plugin and UpdateCollectionInfo instruction #225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
* Add Bubblegum V1 plugin * Regenerate IDL and clients * Update Rust client advanced types * Add instruction for Bubblegum collection size bookkeeping * Regenerate IDL and clients * Add tests and fix functionality * Add more tests * Add test for UpdateCollectionInfoV1 * Add asset tests for BubblegumV1 plugin * Use enum for and amount for incrementing and decrementing * Regenerate IDL and clients * Fix rust test * chore: Release mpl-core version 0.10.0-alpha.1 * Update Bubblegum signer address * Add Attributes to BubblegumV1 plugin allow list * Deploy JS client v1.3.0-alpha.0 --------- Co-authored-by: danenbm <[email protected]>
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Summary by CodeRabbit
Summary by CodeRabbit
WalkthroughThis change set introduces the BubblegumV2 plugin across the Solana Core program and its associated client libraries. It updates type definitions, plugin handling, and validation logic to support BubblegumV2 as a new, stateless plugin for collections, enforcing strict rules on plugin composition and authority. The update includes a new instruction Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Client (JS/Rust)
participant CoreProgram
participant BubblegumProgram
User->>Client (JS/Rust): Request create collection with BubblegumV2 plugin
Client->>CoreProgram: Send create_collection instruction (BubblegumV2 plugin)
CoreProgram->>CoreProgram: Validate plugins (allow list, authority)
CoreProgram-->>Client (JS/Rust): Success or error (BlockedByBubblegumV2/InvalidPlugin)
User->>Client (JS/Rust): Request add plugin to collection with BubblegumV2
Client->>CoreProgram: Send add_plugin instruction
CoreProgram->>CoreProgram: Check if plugin is allowed with BubblegumV2
CoreProgram-->>Client (JS/Rust): Success or error (BlockedByBubblegumV2/InvalidAuthority)
User->>Client (JS/Rust): Request UpdateCollectionInfoV1 (BubblegumV2)
Client->>CoreProgram: Send UpdateCollectionInfoV1 instruction
CoreProgram->>CoreProgram: Validate Bubblegum signer and update type
CoreProgram-->>Client (JS/Rust): Success or error (InvalidAuthority)
sequenceDiagram
participant Test
participant Client (JS/Rust)
participant CoreProgram
Test->>Client (JS/Rust): Attempt asset creation with BubblegumV2 plugin
Client->>CoreProgram: createAsset (BubblegumV2 plugin)
CoreProgram->>CoreProgram: Reject with InvalidPlugin
CoreProgram-->>Client (JS/Rust): Error
Client (JS/Rust)-->>Test: Assert error thrown
Test->>Client (JS/Rust): Attempt to add BubblegumV2 to asset/collection
Client->>CoreProgram: addPluginV1 (BubblegumV2)
CoreProgram->>CoreProgram: Reject with InvalidAuthority
CoreProgram-->>Client (JS/Rust): Error
Client (JS/Rust)-->>Test: Assert error thrown
Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this 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 (6)
clients/rust/src/hooked/advanced_types.rs (1)
164-168
: Consider adding doc comments.
This helps maintain consistency, clarifies the plugin’s purpose, and documents how it interacts with Bubblegum.clients/rust/tests/update_collection_info.rs (2)
12-13
: Double-check the correctness ofBUBBLEGUM_PROGRAM_ADDRESS
.
If the Bubblegum program address changes in the future, consider referencing a centrally defined constant or environment variable.
15-114
: Effective negative test coverage.
The scenario of incorrectly signing with a fake PDA is well-handled. Consider adding a positive test to verify a correct Bubblegum PDA call path and check that the instruction succeeds as intended.programs/mpl-core/src/processor/create_collection.rs (2)
126-130
: Consider merging plugin checks into a single pass.
This snippet scans the plugin list to detect Bubblegum. You could merge this logic into the main loop if performance becomes critical.
131-138
: Initializebubblegum_v1_allow_list
once.
IfALLOW_LIST
is large or frequently accessed, you could store it in a static or lazily initializedHashSet
to avoid re-computation.programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs (1)
84-95
: Test coverage suggestion.
This test checks serialization length effectively. However, consider adding separate tests to cover corner cases like disallowing late addition, blocking external adapters, and rejecting removal. Expanding these tests would confirm thatBubblegumV1
enforces its constraints as intended.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (17)
Cargo.lock
is excluded by!**/*.lock
clients/js/src/generated/errors/mplCore.ts
is excluded by!**/generated/**
clients/js/src/generated/instructions/index.ts
is excluded by!**/generated/**
clients/js/src/generated/instructions/updateCollectionInfoV1.ts
is excluded by!**/generated/**
clients/js/src/generated/types/bubblegumV1.ts
is excluded by!**/generated/**
clients/js/src/generated/types/index.ts
is excluded by!**/generated/**
clients/js/src/generated/types/plugin.ts
is excluded by!**/generated/**
clients/js/src/generated/types/pluginType.ts
is excluded by!**/generated/**
clients/js/src/generated/types/updateType.ts
is excluded by!**/generated/**
clients/rust/src/generated/errors/mpl_core.rs
is excluded by!**/generated/**
clients/rust/src/generated/instructions/mod.rs
is excluded by!**/generated/**
clients/rust/src/generated/instructions/update_collection_info_v1.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/bubblegum_v1.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/mod.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/plugin.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/plugin_type.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/update_type.rs
is excluded by!**/generated/**
📒 Files selected for processing (21)
clients/js/package.json
(1 hunks)clients/js/src/plugins/types.ts
(5 hunks)clients/js/test/plugins/asset/bubblegumV1.test.ts
(1 hunks)clients/js/test/plugins/collection/bubblegumV1.test.ts
(1 hunks)clients/rust/Cargo.toml
(1 hunks)clients/rust/src/hooked/advanced_types.rs
(3 hunks)clients/rust/src/hooked/mod.rs
(2 hunks)clients/rust/src/hooked/plugin.rs
(2 hunks)clients/rust/tests/update_collection_info.rs
(1 hunks)idls/mpl_core.json
(7 hunks)programs/mpl-core/Cargo.toml
(1 hunks)programs/mpl-core/src/error.rs
(1 hunks)programs/mpl-core/src/instruction.rs
(2 hunks)programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs
(1 hunks)programs/mpl-core/src/plugins/internal/permanent/mod.rs
(1 hunks)programs/mpl-core/src/plugins/lifecycle.rs
(3 hunks)programs/mpl-core/src/plugins/mod.rs
(8 hunks)programs/mpl-core/src/processor/create.rs
(1 hunks)programs/mpl-core/src/processor/create_collection.rs
(6 hunks)programs/mpl-core/src/processor/mod.rs
(3 hunks)programs/mpl-core/src/processor/update_collection_info.rs
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Programs / Build
🔇 Additional comments (64)
clients/rust/Cargo.toml (1)
8-8
: Version bump to 0.10.0-alpha.1 aligns with major feature additionThe version update from 0.8.1-beta.1 to 0.10.0-alpha.1 is appropriate given the addition of the new BubblegumV1 plugin functionality. The alpha designation correctly indicates that these new features are still in development or testing phase.
clients/js/package.json (1)
3-3
: Version increment to 1.3.0-alpha.0 follows semantic versioningThe version update from 1.2.0 to 1.3.0-alpha.0 correctly follows semantic versioning principles for adding new features (minor version increment) and indicating pre-release status with the alpha tag.
programs/mpl-core/Cargo.toml (1)
16-16
:❓ Verification inconclusive
Addition of mpl-bubblegum dependency
The dependency on mpl-bubblegum 1.4.0 is correctly added to support the new BubblegumV1 plugin functionality. This precise version pinning will ensure consistent builds.
🏁 Script executed:
#!/bin/bash # Verify if the pinned version is the latest stable cargo search mpl-bubblegum --limit 1Length of output: 134
Confirm mpl-bubblegum Dependency Version
The dependency on mpl-bubblegum = "1.4.0" appears correctly added to enable the new BubblegumV1 plugin functionality. However, the initial verification using the
cargo search
command failed due to the absence of Cargo in the current environment. To be certain that the pinned version is the latest stable release, please execute the following script locally to query the crates.io API and confirm that the maximum stable version is indeed 1.4.0:#!/bin/bash curl -s https://crates.io/api/v1/crates/mpl-bubblegum | jq '.crate.max_stable_version'
- If the output matches
"1.4.0"
, the dependency pinning is confirmed and no further changes are required.- Otherwise, please update the version accordingly.
programs/mpl-core/src/plugins/internal/permanent/mod.rs (1)
1-1
:✅ Verification successful
New BubblegumV1 module integrates with plugin system
The addition of the bubblegum_v1 module and its public export follows the established module pattern in the codebase for plugin integration. This change enables the BubblegumV1 plugin functionality mentioned in the PR objectives.
Also applies to: 7-7
🏁 Script executed:
#!/bin/bash # Check if bubblegum_v1.rs file exists if [ -f "programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs" ]; then echo "bubblegum_v1.rs file exists" # Check implementation details grep -n "struct BubblegumV1" "programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs" grep -n "impl Plugin" "programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs" else echo "bubblegum_v1.rs file not found" fiLength of output: 399
Verification Complete: BubblegumV1 Module Integrates Successfully
The file
programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs
was found, and it contains the expected definitions:
pub struct BubblegumV1 {}
at line 17- An implementation
impl PluginValidation for BubblegumV1 { ... }
at line 38This confirms that the new BubblegumV1 module properly integrates with the plugin system as intended.
programs/mpl-core/src/error.rs (1)
211-214
: Well-structured error addition for BubblegumV1 plugin constraintsThe new error variant
BlockedByBubblegumV1
is appropriately placed in the error enum with a clear error message explaining its purpose. This error will be used to enforce limitations when using the BubblegumV1 plugin alongside other plugins.clients/js/test/plugins/asset/bubblegumV1.test.ts (2)
1-19
: Well-defined test case for BubblegumV1 plugin restrictions on assetsThis test correctly verifies that the BubblegumV1 plugin cannot be used when creating assets, expecting an
InvalidPlugin
error which aligns with the implementation constraints.
21-35
: Appropriate test for BubblegumV1 plugin addition constraintsThis test properly verifies that the BubblegumV1 plugin cannot be added to existing assets, expecting an
InvalidAuthority
error. This validates the plugin's intended usage pattern.programs/mpl-core/src/processor/create.rs (1)
188-190
: Correctly extended plugin validation to include BubblegumV1The conditional check now properly includes BubblegumV1 alongside MasterEdition as plugin types that cannot be used in asset creation, which aligns with the plugin's intended design.
programs/mpl-core/src/plugins/lifecycle.rs (3)
94-94
: Appropriate lifecycle check for BubblegumV1 in add_pluginAdding BubblegumV1 to the
check_add_plugin
method withCheckResult::CanReject
correctly allows the plugin to reject adding other plugins, enforcing its constraints.
107-107
: Correctly configured BubblegumV1 to reject plugin removalThe BubblegumV1 plugin is now properly set to reject plugin removal operations in the
check_remove_plugin
method, which is consistent with maintaining plugin integrity.
212-212
: Well-implemented validation for BubblegumV1 with external plugin adaptersThe BubblegumV1 plugin is correctly configured to reject external plugin adapter additions in the
check_add_external_plugin_adapter
method, which helps maintain the plugin's constraints.programs/mpl-core/src/processor/mod.rs (3)
16-16
: Added new module for collection updates.The new module addition follows the consistent pattern in the codebase and will handle the UpdateCollectionInfoV1 instruction processing.
36-36
: Added public use for new module.The export is properly scoped with pub(crate) to maintain encapsulation while allowing internal access.
181-184
: UpdateCollectionInfoV1 instruction handling added.The new instruction match arm follows the established pattern for instruction handling, with proper logging and function call.
clients/rust/src/hooked/mod.rs (1)
37-38
: Added conversion support for new plugin types.The implementation handles three new plugin types (AddBlocker, ImmutableMetadata, and BubblegumV1) in the From trait implementation, allowing them to be properly converted to their corresponding PluginType values.
Also applies to: 52-52
clients/js/src/plugins/types.ts (5)
28-28
: Added BubblegumV1 import.The import ensures the BubblegumV1 type is available for use in this module.
93-95
: Added BubblegumV1 to CreatePluginArgs type.This addition allows clients to specify BubblegumV1 as a plugin type when creating plugins.
114-116
: Added BubblegumV1 to CreateOnlyPluginArgsV2 type.This inclusion allows the BubblegumV1 plugin to be specified in the V2 plugin creation API, maintaining consistency with how other plugins are handled.
191-191
: Added BubblegumV1Plugin type.The new type definition follows the established pattern, combining BasePlugin with BubblegumV1 to create a complete plugin type.
215-215
: Added bubblegumV1 to CollectionPluginsList.This addition allows collections to include the BubblegumV1 plugin, making it accessible through the common plugin list structure.
clients/rust/src/hooked/plugin.rs (2)
16-17
: Added BubblegumV1Plugin to imports.The import ensures the BubblegumV1Plugin type is available for use in the registry_records_to_plugin_list function.
348-350
: Added match arm for BubblegumV1 plugin.This addition ensures that when a BubblegumV1 plugin is encountered in the registry records, it is properly handled and added to the plugin list's bubblegum_v1 field.
clients/rust/src/hooked/advanced_types.rs (2)
11-16
: No immediate issues with the newly imported entities.
187-187
: Check test coverage for thebubblegum_v1
field.
Ensure that downstream code or unit tests handle the plugin’s presence or absence correctly withinPluginsList
.programs/mpl-core/src/instruction.rs (2)
13-14
: Confirm that references to any replaced imports are also updated.
Ensure thatUpdateCollectionExternalPluginAdapterV1Args
is fully removed or replaced in the rest of the codebase if it’s no longer in use.
306-309
: Validate that the Bubblegum signer is strictly a PDA derived address.
Currently, the instruction implies thatbubblegum_signer
must be a Bubblegum-owned signer. If not already enforced in the processor, add appropriate checks to prevent unauthorized calls.clients/rust/tests/update_collection_info.rs (2)
1-2
: Configuration gating with#![cfg(feature = "test-sbf")]
.
This ensures tests only run in the correct environment. Looks good.
3-5
: No issues with new module imports.
They properly expose the methods and types needed for the test.programs/mpl-core/src/processor/create_collection.rs (6)
7-7
: HashSet usage for plugin allow-lists
UsingHashSet
is an efficient way to enforce membership checks.
14-15
: ImportedBubblegumV1
No issues spotted; ensures consistent plugin logic usage.
115-115
: Flagginghas_bubblegum_v1
Declaring a dedicated boolean flag is a straightforward approach to handle plugin constraints.
152-156
: Error handling for invalid plugins.
ReturningBlockedByBubblegumV1
when a plugin isn’t in the allow list is consistent with your specified constraints.
183-188
: Forcing the Bubblegum plugin authority.
The plugin manager is always used as the authority whenBubblegumV1
is detected. Confirm that any user-supplied authority is either disallowed or properly superseded.
205-207
: External adapters disallowed with Bubblegum V1.
This restriction aligns with your single-plugin approach. No issues noted.clients/js/test/plugins/collection/bubblegumV1.test.ts (9)
21-44
: Comprehensive positive test scenario.
This test covers the creation of a collection with the BubblegumV1 plugin and verifies the resulting collection’s fields thoroughly. No issues found.
46-67
: Good negative test ensuring BubblegumV1 remains immutable after creation.
The test correctly expects an 'InvalidAuthority' error, illustrating robust coverage of forbidden post-creation plugin additions.
69-115
: Thorough check for removal restrictions.
This scenario ensures that BubblegumV1 cannot be removed by the update authority, maintaining plugin immutability. No logical flaws noted.
117-160
: Allows multiple allow-listed plugins alongside BubblegumV1.
This test confirms compatibility with other permitted plugins. Looks solid and well-structured.
162-189
: Correct handling of non-allow-listed plugins.
The test triggers a 'BlockedByBubblegumV1' error, ensuring that disallowed plugins are rejected when BubblegumV1 is present.
191-233
: Successful addition of allow-listed plugin post-creation.
Good demonstration of incremental plugin addition while BubblegumV1 is present in the collection.
234-269
: Negative scenario validatesInvalidAuthority
error.
Ensures the system blocks adding non-allow-listed plugins to a BubblegumV1 collection. Implementation is straightforward and correct.
270-289
: Test covers mixing BubblegumV1 with external plugins.
Correctly verifies that using external plugins (e.g., 'AppData') with BubblegumV1 is blocked.
290-327
: Additional negative scenario covering external plugin addition post-creation.
Completes the coverage for BubblegumV1 restrictions. The approach is consistent with other negative tests.idls/mpl_core.json (4)
2084-2117
: Addition of theUpdateCollectionInfoV1
instruction.
Declaring new accounts (collection
,bubblegumSigner
) and linking them toUpdateCollectionInfoV1Args
aligns with the rest of the IDL. Validation logic for checking signer authority is consistent with program expectations.
3705-3722
: New structUpdateCollectionInfoV1Args
.
Defines the fields needed for collection updates. The usage ofUpdateType
andamount
is well-structured and clearly conveys the needed parameters.
4682-4697
: New enumUpdateType
.
Clear definitions for Mint, Add, and Remove variants. Straightforward naming that covers the update cases comprehensively.
5039-5043
: New error codeBlockedByBubblegumV1
.
Accurately named error code to prevent incompatible plugins from coexisting with Bubblegum V1. This fosters clear error handling.programs/mpl-core/src/plugins/mod.rs (8)
63-64
: Introducing theBubblegumV1
variant inPlugin
.
Seamlessly extends the enum to encompass the new plugin. The inline comment clarifies its purpose for compressed NFTs.
108-109
: Extendedinner()
to handleBubblegumV1
.
The match arm for the new variant is correct, returning the plugin’s inner interface.
140-141
: Size accounting inlen()
method.
Adds support forBubblegumV1
while maintaining the established pattern. Implementation is consistent.
193-194
: AddedBubblegumV1
inPluginType
.
The variant naming is aligned with the plugin. Straightforward extension of the enum.
233-234
: Mapping fromPlugin
toPluginType
.
Correctly identifies theBubblegumV1
variant. No issues.
257-259
: Defaults theBubblegumV1
plugin authority to the Bubblegum program ID.
Ensures the plugin is specifically controlled by a designated address for compressed NFTs. Good approach.
310-311
: Test plugin enumeration includesBubblegumV1
.
Ensures coverage across all plugin tests, matching the pattern for existing plugins.
426-427
: Additional test scenarios forBubblegumV1
.
Verifies potential variations in data serialization. Good thoroughness in coverage.programs/mpl-core/src/processor/update_collection_info.rs (1)
1-66
: Implementation of theupdate_collection_info
logic.
- Validates the Bubblegum signer via
BUBBLEGUM_SIGNER
.- Applies the requested
UpdateType
by adjustingcollection.num_minted
andcollection.current_size
safely withsaturating_add
/saturating_sub
.- Stores the updated structure back into the account.
All operations appear correct, with well-thought-out checks to prevent overflows or unauthorized calls.
programs/mpl-core/src/plugins/internal/permanent/bubblegum_v1.rs (8)
1-3
: Good use of Borsh and Solana imports.
The imports at lines 1-3 correctly bring in Borsh traits and Solana’sProgramError
, aligning with typical serialization and error-handling patterns.
12-15
: Concise documentation.
The doc comments clearly convey the purpose of theBubblegumV1
plugin and the requirement that only the Bubblegum program can be the authority.
16-17
: Empty struct usage.
Defining theBubblegumV1
struct as empty is a simple approach, consistent with its stateless design.
19-29
: ALLOW_LIST logic appears correct.
This explicit list clarifies which plugins may coexist withBubblegumV1
. It’s a clean, centralized way to manage plugin compatibility.
31-36
: Stateless data blob.
ImplementingDataBlob
withlen() = 0
confirms thatBubblegumV1
carries no serialized data. This approach minimizes on-chain footprint.
38-59
: validate_add_plugin behavior makes sense.
The logic to:
- Abstain if the target plugin is in the allow list.
- Reject if it’s
BubblegumV1
(ensuring addition must happen at creation only).- Reject for all other plugin types.
These control flows match the stated design requirements and ensure that no incompatible plugin conflicts are introduced.
61-72
: Non-removable plugin.
This code strictly disallows removal ofBubblegumV1
, as evidenced by always rejecting removal attempts of itself. Any other plugin removal is not impeded byBubblegumV1
. This logic is consistent with the plugin’s permanence requirement.
74-81
: Rejecting external plugin adapters.
The code consistently rejects external plugin adapters ifBubblegumV1
is present, reflecting a policy of strict exclusivity for extended behaviors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmark
Benchmark suite | Current: 3aaae60 | Previous: 661d5e0 | Ratio |
---|---|---|---|
CU: create a new, empty asset |
11212 Compute Units |
11206 Compute Units |
1.00 |
Space: create a new, empty asset |
91 Bytes |
91 Bytes |
1 |
CU: create a new, empty asset with empty collection |
22698 Compute Units |
22681 Compute Units |
1.00 |
Space: create a new, empty asset with empty collection |
91 Bytes |
91 Bytes |
1 |
CU: create a new asset with plugins |
35179 Compute Units |
35182 Compute Units |
1.00 |
Space: create a new asset with plugins |
194 Bytes |
194 Bytes |
1 |
CU: create a new asset with plugins and empty collection |
41210 Compute Units |
41202 Compute Units |
1.00 |
Space: create a new asset with plugins and empty collection |
194 Bytes |
194 Bytes |
1 |
CU: list an asset |
28100 Compute Units |
28101 Compute Units |
1.00 |
CU: sell an asset |
44532 Compute Units |
44473 Compute Units |
1.00 |
CU: list an asset with empty collection |
35656 Compute Units |
35663 Compute Units |
1.00 |
CU: sell an asset with empty collection |
56884 Compute Units |
56825 Compute Units |
1.00 |
CU: list an asset with collection royalties |
36427 Compute Units |
36434 Compute Units |
1.00 |
CU: sell an asset with collection royalties |
61972 Compute Units |
61909 Compute Units |
1.00 |
CU: transfer an empty asset |
6450 Compute Units |
6444 Compute Units |
1.00 |
CU: transfer an empty asset with empty collection |
9049 Compute Units |
9043 Compute Units |
1.00 |
CU: transfer an asset with plugins |
14768 Compute Units |
14769 Compute Units |
1.00 |
CU: transfer an asset with plugins and empty collection |
17367 Compute Units |
17368 Compute Units |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
clients/js/test/plugins/collection/bubblegumV2.test.ts (1)
1-327
: 🛠️ Refactor suggestionMissing tests for UpdateCollectionInfoV1 instruction.
While the BubblegumV2 plugin functionality is well-tested, there are no tests for the UpdateCollectionInfoV1 instruction which is a key part of the implementation.
Consider adding tests that verify:
- A valid Bubblegum signer can update collection information
- Different update types (Mint, Add, Remove) with various amounts
- Unauthorized accounts cannot update collection information
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
⛔ Files ignored due to path filters (10)
clients/js/src/generated/errors/mplCore.ts
is excluded by!**/generated/**
clients/js/src/generated/types/bubblegumV2.ts
is excluded by!**/generated/**
clients/js/src/generated/types/index.ts
is excluded by!**/generated/**
clients/js/src/generated/types/plugin.ts
is excluded by!**/generated/**
clients/js/src/generated/types/pluginType.ts
is excluded by!**/generated/**
clients/rust/src/generated/errors/mpl_core.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/bubblegum_v2.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/mod.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/plugin.rs
is excluded by!**/generated/**
clients/rust/src/generated/types/plugin_type.rs
is excluded by!**/generated/**
📒 Files selected for processing (14)
clients/js/src/plugins/types.ts
(5 hunks)clients/js/test/plugins/asset/bubblegumV2.test.ts
(1 hunks)clients/js/test/plugins/collection/bubblegumV2.test.ts
(1 hunks)clients/rust/src/hooked/advanced_types.rs
(3 hunks)clients/rust/src/hooked/mod.rs
(2 hunks)clients/rust/src/hooked/plugin.rs
(2 hunks)idls/mpl_core.json
(7 hunks)programs/mpl-core/src/error.rs
(1 hunks)programs/mpl-core/src/plugins/internal/permanent/bubblegum_v2.rs
(1 hunks)programs/mpl-core/src/plugins/internal/permanent/mod.rs
(1 hunks)programs/mpl-core/src/plugins/lifecycle.rs
(3 hunks)programs/mpl-core/src/plugins/mod.rs
(8 hunks)programs/mpl-core/src/processor/create.rs
(1 hunks)programs/mpl-core/src/processor/create_collection.rs
(6 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
clients/rust/src/hooked/mod.rs (1)
clients/js/src/generated/types/plugin.ts (1)
Plugin
(68-84)
clients/rust/src/hooked/plugin.rs (3)
clients/js/src/plugins/types.ts (2)
BasePlugin
(37-40)BubblegumV2Plugin
(191-191)clients/js/src/generated/types/plugin.ts (1)
Plugin
(68-84)clients/js/src/generated/types/bubblegumV2.ts (1)
BubblegumV2
(11-11)
clients/js/test/plugins/asset/bubblegumV2.test.ts (3)
clients/js/test/_setupRaw.ts (1)
createAsset
(55-80)clients/js/src/generated/instructions/addPluginV1.ts (1)
addPluginV1
(98-180)clients/js/src/plugins/lib.ts (1)
createPlugin
(45-61)
clients/js/src/plugins/types.ts (2)
clients/js/src/generated/types/edition.ts (1)
EditionArgs
(13-13)clients/js/src/generated/types/bubblegumV2.ts (1)
BubblegumV2
(11-11)
programs/mpl-core/src/plugins/mod.rs (2)
clients/js/src/generated/types/bubblegumV2.ts (1)
BubblegumV2
(11-11)clients/js/src/generated/types/plugin.ts (1)
Plugin
(68-84)
🔇 Additional comments (42)
programs/mpl-core/src/error.rs (1)
212-214
: Well-structured error variant for BubblegumV2 plugin restrictions.The new error variant is appropriately placed and follows the established pattern in the enum. The error message clearly indicates its purpose - to signify when plugin operations are blocked due to BubblegumV2 plugin restrictions.
clients/rust/src/hooked/mod.rs (2)
37-38
: Reordering of match arms is safe and doesn't affect functionality.Moving these match arms to the top of the pattern matching doesn't change behavior since the order of arms in a match statement is irrelevant when patterns don't overlap.
52-52
: Good addition of BubblegumV2 plugin type conversion.This new match arm properly handles the BubblegumV2 plugin variant, ensuring consistency between the Plugin enum and PluginType enum conversions.
programs/mpl-core/src/processor/create.rs (1)
188-190
: Appropriate restriction of BubblegumV2 plugin for assets.This change correctly restricts the BubblegumV2 plugin from being used in individual assets. Based on the PR description, BubblegumV2 is meant only for collections to indicate compatibility with compressed NFTs, so this validation ensures proper usage.
programs/mpl-core/src/plugins/internal/permanent/mod.rs (2)
1-1
: New bubblegum_v2 module added to permanent plugins.This addition properly introduces the new BubblegumV2 plugin module following the established pattern for permanent plugins.
7-7
: Appropriate public re-export of BubblegumV2 plugin.The public re-export ensures the BubblegumV2 plugin functionality is accessible through the permanent plugins namespace, consistent with other permanent plugins.
programs/mpl-core/src/plugins/lifecycle.rs (3)
94-94
: Added BubblegumV2 with ability to reject pluginsThe BubblegumV2 plugin has been correctly added to the
check_add_plugin
lifecycle method withCanReject
capability, which aligns with its role of restricting what plugins can be added to a collection.
107-107
: Added removal prevention for BubblegumV2 pluginBubblegumV2 has been added to the
check_remove_plugin
method withCanReject
capability, which ensures that this plugin cannot be removed once added. This is essential for maintaining the integrity of collections marked for storing compressed NFTs.
212-212
: Added external adapter restriction for BubblegumV2BubblegumV2 has been added to the
check_add_external_plugin_adapter
method withCanReject
capability, allowing it to control which external plugin adapters can be added to a collection. This completes the permission set needed for BubblegumV2 to properly restrict plugin composition.clients/rust/src/hooked/plugin.rs (2)
16-17
: Added BubblegumV2Plugin to importsBubblegumV2Plugin has been correctly added to the imports list, maintaining alphabetical order.
348-350
: Added BubblegumV2 handling to registry processingImplemented the appropriate case handling for BubblegumV2 plugin in the registry processing function. This maintains the pattern used for other plugins and properly updates the accumulator structure.
clients/rust/src/hooked/advanced_types.rs (3)
11-16
: Added BubblegumV2 to type importsBubblegumV2 has been added to the imports list, maintaining alphabetical order with the other plugin types.
164-168
: Added BubblegumV2Plugin struct definitionThe new BubblegumV2Plugin struct follows the established pattern for plugin types, with the correct fields and derive macros. This ensures consistent handling with other plugin types.
187-187
: Added BubblegumV2Plugin to PluginsListAdded the BubblegumV2Plugin as an optional field to the PluginsList struct, allowing it to be properly included in the plugin list data structure.
clients/js/src/plugins/types.ts (5)
28-28
: Added BubblegumV2 to TypeScript importsThe BubblegumV2 type has been correctly imported from the generated types.
93-95
: Added BubblegumV2 to CreatePluginArgs union typeBubblegumV2 has been properly added as a valid option in the CreatePluginArgs union type. Note that it follows the pattern of plugins without additional data parameters.
114-116
: Added BubblegumV2 to CreateOnlyPluginArgsV2 union typeBubblegumV2 has been correctly added to the CreateOnlyPluginArgsV2 union type, indicating that it's a create-only plugin that can't be added after creation.
191-191
: Added BubblegumV2Plugin type definitionThe BubblegumV2Plugin type has been defined as a combination of BasePlugin and BubblegumV2, following the pattern used for other plugin types.
215-215
: Added BubblegumV2Plugin to CollectionPluginsListBubblegumV2Plugin has been added as an optional field to the CollectionPluginsList type. This correctly places it as a collection-level plugin rather than an asset-level plugin.
programs/mpl-core/src/processor/create_collection.rs (1)
205-208
: 👍 External adapters correctly blocked when BubblegumV2 is present.Nice defensive check — this enforces the plugin’s “no external adapters” invariant early, saving wasted compute units later in the pipeline.
clients/js/test/plugins/asset/bubblegumV2.test.ts (1)
32-34
: Verify expected error type.The processor rejects
addPluginV1
for BubblegumV2 withBlockedByBubblegumV2
, notInvalidAuthority
.
If that invariant changes upstream, this assertion will hide the regression.Please double-check the runtime error (or loosen the assertion to the correct enum variant).
programs/mpl-core/src/plugins/mod.rs (1)
140-141
: Missinglen()
implementation can break size accounting.
BubblegumV2
is zero-sized today, but you still delegate tobubblegum_v2.len()
.
Ensure the struct implementsDataBlob
and returns a constant so that future field additions automatically adjust account-space calculations.If
impl DataBlob for BubblegumV2
is missing or returns0
unconditionally, add:impl DataBlob for BubblegumV2 { #[inline(always)] fn len(&self) -> usize { 0 } }idls/mpl_core.json (7)
2084-2116
: New instruction UpdateCollectionInfoV1 added.This instruction adds the ability for a Bubblegum PDA signer to update collection information, which is a key functionality for managing Bubblegum-compatible collections.
3082-3088
: New BubblegumV2 struct added.The BubblegumV2 plugin is correctly implemented as an empty struct, indicating its stateless nature.
3706-3722
: New UpdateCollectionInfoV1Args struct added.This struct enables specifying the type of update and the amount to modify collection counts by.
4046-4053
: BubblegumV2 added to Plugin enum.BubblegumV2 is properly integrated as a variant in the Plugin enum.
4106-4109
: BubblegumV2 added to PluginType enum.BubblegumV2 is properly integrated as a variant in the PluginType enum.
4681-4697
: New UpdateType enum added.This enum provides clear options for how collection counts should be modified: Mint, Add, or Remove.
5039-5043
: New error code for BubblegumV2 constraints.Error code 50 for "BlockedByBubblegumV2" is appropriately added to handle validation failures when BubblegumV2 plugin restricts operations.
programs/mpl-core/src/plugins/internal/permanent/bubblegum_v2.rs (4)
12-17
: Well-documented BubblegumV2 struct.The documentation clearly explains the purpose of the BubblegumV2 plugin and its authority constraints.
31-36
: Stateless DataBlob implementation.The plugin correctly returns 0 for length, confirming its stateless nature.
38-59
: Thorough validation for adding plugins.The validation logic handles all cases appropriately:
- Allow-listed plugins are permitted
- Attempts to add another BubblegumV2 plugin are rejected
- All other plugins are rejected
61-72
: Plugin removal protection.The implementation correctly prevents removal of the BubblegumV2 plugin, which is essential for maintaining collection integrity.
clients/js/test/plugins/collection/bubblegumV2.test.ts (9)
21-44
: Test for collection creation with BubblegumV2 plugin.The test properly verifies that a collection can be created with the BubblegumV2 plugin and confirms the plugin's authority is set to the Bubblegum program ID.
46-67
: Test for adding BubblegumV2 post-creation.The test correctly verifies that BubblegumV2 cannot be added to a collection after its initial creation.
69-115
: Test for BubblegumV2 removal protection.The test correctly verifies that the BubblegumV2 plugin cannot be removed from a collection once added.
117-160
: Test for allowed plugin combinations.The test verifies that collections can be created with BubblegumV2 alongside other allow-listed plugins.
162-189
: Test for disallowed plugin combinations.The test verifies that collections cannot be created with BubblegumV2 alongside non-allow-listed plugins, correctly checking for the BlockedByBubblegumV2 error.
191-232
: Test for adding allowed plugins to BubblegumV2 collections.The test verifies that allow-listed plugins can be added to collections that already have the BubblegumV2 plugin.
234-268
: Test for adding disallowed plugins to BubblegumV2 collections.The test verifies that non-allow-listed plugins cannot be added to collections that have the BubblegumV2 plugin.
270-288
: Test for BubblegumV2 with external plugins at creation.The test verifies that collections cannot be created with both BubblegumV2 and external plugins.
290-326
: Test for adding external plugins to BubblegumV2 collections.The test verifies that external plugins cannot be added to collections with the BubblegumV2 plugin.
programs/mpl-core/src/plugins/internal/permanent/bubblegum_v2.rs
Outdated
Show resolved
Hide resolved
programs/mpl-core/src/plugins/internal/permanent/bubblegum_v2.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
⛔ Files ignored due to path filters (1)
Cargo.lock
is excluded by!**/*.lock
📒 Files selected for processing (1)
clients/rust/Cargo.toml
(1 hunks)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (2)
programs/mpl-core/src/processor/create_collection.rs (1)
133-137
: 🧹 Nitpick (assertive)Avoid per-transaction
HashSet
allocation; slice lookup is enough
BubblegumV2::ALLOW_LIST
contains only six elements, so converting it into aHashSet
on every instruction execution allocates heap memory for negligible performance gain.
A simple slice lookup (.contains
) is O(6) and keeps the code allocation-free.- let bubblegum_v2_allow_list: Option<HashSet<PluginType>> = if has_bubblegum_v2 { - Some(BubblegumV2::ALLOW_LIST.iter().cloned().collect()) - } else { - None - }; + // We have at most six items; avoid an allocation by using the slice directly. + let allow_list = has_bubblegum_v2.then_some(&BubblegumV2::ALLOW_LIST[..]);Corresponding change inside the loop:
- if let Some(allow_list) = &bubblegum_v2_allow_list { - if plugin_type != PluginType::BubblegumV2 && !allow_list.contains(&plugin_type) { + if let Some(allow_list) = allow_list { + if plugin_type != PluginType::BubblegumV2 && !allow_list.contains(&plugin_type) { return Err(MplCoreError::BlockedByBubblegumV2.into()); } }This removes two heap allocations (the
Vec
and theHashSet
) from the critical path.clients/js/test/plugins/collection/bubblegumV2.test.ts (1)
18-20
: Hard-coded program ID acknowledgedLeaving the Bubblegum program ID inline keeps the test self-contained, matching the maintainer’s preference recorded earlier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Lite
📒 Files selected for processing (3)
clients/js/test/plugins/collection/bubblegumV2.test.ts
(1 hunks)programs/mpl-core/src/plugins/internal/permanent/bubblegum_v2.rs
(1 hunks)programs/mpl-core/src/processor/create_collection.rs
(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
programs/mpl-core/src/processor/create_collection.rs (1)
Learnt from: danenbm
PR: metaplex-foundation/mpl-core#225
File: programs/mpl-core/src/processor/create_collection.rs:133-138
Timestamp: 2025-05-07T18:44:30.293Z
Learning: Lazy-static patterns and other forms of cross-transaction caching don't work in Solana smart contracts because the execution model is stateless - programs are freshly loaded for each transaction and don't maintain state between executions.
clients/js/test/plugins/collection/bubblegumV2.test.ts (1)
Learnt from: danenbm
PR: metaplex-foundation/mpl-core#225
File: clients/js/test/plugins/collection/bubblegumV2.test.ts:18-20
Timestamp: 2025-05-07T18:56:19.607Z
Learning: For test files, danenbm prefers to avoid additional dependencies by keeping constants like program IDs hardcoded directly in the test file rather than importing them from shared files.
🔇 Additional comments (1)
programs/mpl-core/src/processor/create_collection.rs (1)
210-213
: 👍 External-adapter gate looks solidBlocking every external plugin adapter when
BubblegumV2
is present correctly enforces the spec and mirrors the Rust-side validation.
programs/mpl-core/src/plugins/internal/permanent/bubblegum_v2.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notes
BubblegumV2
permanent plugin which is used to indicate to the Bubblegum V2 program that this Core collection is suitable for storing cNFTs.BubblegumV2
plugin limits what other plugins and external plugin adapters can be added to the collection, both at creation and via add instructions.UpdateCollectionInfoV1
, which allows a specific PDA signer from the Bubblegum program to directly change the collectionnum_minted
andcurrent_size
fields.