Adding permanent freeze execute plugin #247
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
WalkthroughAdds a PermanentFreezeExecute plugin end-to-end: IDL, on-chain plugin implementation and validations, lifecycle wiring, JS/Rust client types and mappings, and comprehensive asset & collection tests enforcing freeze/unfreeze, add/remove, and execution blocking semantics. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as Client (JS/Rust)
participant Program as mpl-core Program
participant PM as Plugin Manager
participant PFE as PermanentFreezeExecute
User->>Client: Submit Execute instruction
Client->>Program: execute(...)
Program->>PM: validate_execute(...)
PM->>PFE: validate_execute(ctx)
alt frozen == true
PFE-->>PM: Reject (InvalidAuthority)
PM-->>Program: CanReject
Program-->>Client: Error (execute blocked)
else unfrozen
PFE-->>PM: Abstain
PM-->>Program: Allow/No Objection
Program-->>Client: Execute proceeds
end
sequenceDiagram
autonumber
actor Authority as UpdateAuthority
participant Client as Client
participant Program as mpl-core Program
participant PM as Plugin Manager
participant PFE as PermanentFreezeExecute
Note over Client,Program: Add/Remove PermanentFreezeExecute plugin
Authority->>Client: Request add/remove
Client->>Program: add_plugin/remove_plugin
Program->>PM: check_add/remove
alt Add when same type exists
PM->>PFE: validate_add_plugin
PFE-->>PM: Reject
Program-->>Client: Error
else Remove while frozen
PM->>PFE: validate_remove_plugin
PFE-->>PM: Reject
Program-->>Client: Error
else Allowed
Program-->>Client: Success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)programs/mpl-core/**📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (2)📓 Common learnings📚 Learning: 2025-09-16T17:12:49.489ZApplied to files:
🧬 Code graph analysis (1)programs/mpl-core/src/plugins/lifecycle.rs (1)
⏰ 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). (3)
🔇 Additional comments (2)
Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Benchmark
| Benchmark suite | Current: c0ef4b5 | Previous: 6c76673 | Ratio |
|---|---|---|---|
CU: create a new, empty asset |
7628 Compute Units |
7628 Compute Units |
1 |
Space: create a new, empty asset |
91 Bytes |
91 Bytes |
1 |
CU: create a new, empty asset with empty collection |
15706 Compute Units |
15706 Compute Units |
1 |
Space: create a new, empty asset with empty collection |
91 Bytes |
91 Bytes |
1 |
CU: create a new asset with plugins |
25922 Compute Units |
25916 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 |
30830 Compute Units |
30824 Compute Units |
1.00 |
Space: create a new asset with plugins and empty collection |
194 Bytes |
194 Bytes |
1 |
CU: list an asset |
19019 Compute Units |
19014 Compute Units |
1.00 |
CU: sell an asset |
24206 Compute Units |
24194 Compute Units |
1.00 |
CU: list an asset with empty collection |
23516 Compute Units |
23511 Compute Units |
1.00 |
CU: sell an asset with empty collection |
31593 Compute Units |
31581 Compute Units |
1.00 |
CU: list an asset with collection royalties |
22906 Compute Units |
22901 Compute Units |
1.00 |
CU: sell an asset with collection royalties |
34644 Compute Units |
34632 Compute Units |
1.00 |
CU: transfer an empty asset |
3611 Compute Units |
3611 Compute Units |
1 |
CU: transfer an empty asset with empty collection |
5171 Compute Units |
5171 Compute Units |
1 |
CU: transfer an asset with plugins |
7048 Compute Units |
7045 Compute Units |
1.00 |
CU: transfer an asset with plugins and empty collection |
8608 Compute Units |
8605 Compute Units |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (8)
clients/js/src/generated/types/index.tsis excluded by!**/generated/**clients/js/src/generated/types/permanentFreezeExecute.tsis excluded by!**/generated/**clients/js/src/generated/types/plugin.tsis excluded by!**/generated/**clients/js/src/generated/types/pluginType.tsis excluded by!**/generated/**clients/rust/src/generated/types/mod.rsis excluded by!**/generated/**clients/rust/src/generated/types/permanent_freeze_execute.rsis excluded by!**/generated/**clients/rust/src/generated/types/plugin.rsis excluded by!**/generated/**clients/rust/src/generated/types/plugin_type.rsis excluded by!**/generated/**
📒 Files selected for processing (9)
clients/js/src/plugins/types.ts(5 hunks)clients/js/test/plugins/asset/freezeExecute.test.ts(1 hunks)clients/js/test/plugins/asset/permanentFreezeExecute.test.ts(1 hunks)clients/js/test/plugins/collection/permanentFreezeExecute.test.ts(1 hunks)idls/mpl_core.json(3 hunks)programs/mpl-core/src/plugins/internal/permanent/mod.rs(1 hunks)programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs(1 hunks)programs/mpl-core/src/plugins/lifecycle.rs(1 hunks)programs/mpl-core/src/plugins/mod.rs(9 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
clients/js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the JavaScript client in clients/js/ (hand-written APIs under clients/js/src/)
Files:
clients/js/test/plugins/asset/freezeExecute.test.tsclients/js/test/plugins/asset/permanentFreezeExecute.test.tsclients/js/test/plugins/collection/permanentFreezeExecute.test.tsclients/js/src/plugins/types.ts
programs/mpl-core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep the on-chain Solana program under programs/mpl-core/
Files:
programs/mpl-core/src/plugins/lifecycle.rsprograms/mpl-core/src/plugins/internal/permanent/mod.rsprograms/mpl-core/src/plugins/mod.rsprograms/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
programs/mpl-core/src/plugins/internal/permanent/**
📄 CodeRabbit inference engine (CLAUDE.md)
Add immutable internal plugins under plugins/internal/permanent/
Files:
programs/mpl-core/src/plugins/internal/permanent/mod.rsprograms/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
🧠 Learnings (1)
📚 Learning: 2025-09-05T20:01:56.374Z
Learnt from: CR
PR: metaplex-foundation/mpl-core#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T20:01:56.374Z
Learning: Applies to programs/mpl-core/src/plugins/internal/permanent/** : Add immutable internal plugins under plugins/internal/permanent/
Applied to files:
programs/mpl-core/src/plugins/internal/permanent/mod.rsprograms/mpl-core/src/plugins/mod.rsprograms/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
🧬 Code graph analysis (6)
programs/mpl-core/src/plugins/lifecycle.rs (1)
clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)
clients/js/test/plugins/asset/permanentFreezeExecute.test.ts (10)
clients/js/src/instructions/create.ts (1)
create(32-109)clients/js/src/generated/accounts/assetV1.ts (1)
fetchAssetV1(46-57)clients/js/test/_setupRaw.ts (4)
assertAsset(137-174)DEFAULT_ASSET(45-48)createAsset(55-80)assertBurned(215-226)clients/js/src/generated/accounts/assetSigner.ts (1)
findAssetSignerPda(121-136)clients/js/src/generated/instructions/updatePluginV1.ts (1)
updatePluginV1(81-163)clients/js/src/plugins/lib.ts (1)
createPlugin(45-61)clients/js/src/generated/instructions/addPluginV1.ts (1)
addPluginV1(98-180)clients/js/src/generated/instructions/transferV1.ts (1)
transferV1(94-174)clients/js/src/generated/instructions/removePluginV1.ts (1)
removePluginV1(81-163)clients/js/src/generated/instructions/burnV1.ts (1)
burnV1(88-163)
programs/mpl-core/src/plugins/mod.rs (2)
clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)clients/js/src/generated/types/plugin.ts (1)
Plugin(74-92)
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (8)
clients/js/test/_setupRaw.ts (5)
createCollection(91-108)assertCollection(176-213)DEFAULT_COLLECTION(50-53)assertAsset(137-174)DEFAULT_ASSET(45-48)clients/js/src/generated/instructions/removeCollectionPluginV1.ts (1)
removeCollectionPluginV1(82-160)clients/js/src/generated/instructions/addCollectionPluginV1.ts (1)
addCollectionPluginV1(100-178)clients/js/src/plugins/lib.ts (1)
createPlugin(45-61)clients/js/src/generated/instructions/updateCollectionPluginV1.ts (1)
updateCollectionPluginV1(82-160)clients/js/src/generated/accounts/assetV1.ts (1)
fetchAssetV1(46-57)clients/js/src/generated/accounts/assetSigner.ts (1)
findAssetSignerPda(121-136)clients/js/src/generated/instructions/transferV1.ts (1)
transferV1(94-174)
clients/js/src/plugins/types.ts (1)
clients/js/src/generated/types/permanentFreezeExecute.ts (2)
PermanentFreezeExecuteArgs(13-13)PermanentFreezeExecute(11-11)
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs (3)
clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)programs/mpl-core/src/plugins/lifecycle.rs (10)
len(40-42)validate_execute(406-411)validate_execute(648-653)validate_add_plugin(238-243)validate_add_plugin(536-541)from(68-70)from(74-78)from(492-498)validate_remove_plugin(246-258)validate_remove_plugin(545-550)programs/mpl-core/src/plugins/mod.rs (3)
len(122-152)len(225-227)from(231-252)
⏰ 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). (3)
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Programs / Build
- GitHub Check: Cursor Bugbot
🔇 Additional comments (22)
clients/js/test/plugins/asset/freezeExecute.test.ts (1)
16-17: Import path adjustments look correct.Paths now correctly resolve to the package root src and shared test setup from the nested test location.
idls/mpl_core.json (3)
4152-4155: IDL: PluginType includes PermanentFreezeExecute.Matches program side; no further issues.
3132-3143: Approve: PermanentFreezeExecute IDL and generated TS types verified.
Generated TypeScript artifacts exist and match the IDL shape (frozen: boolean); present in clients/js/src/generated/types/permanentFreezeExecute.ts and referenced in clients/js/src/generated/types/plugin.ts.
4085-4093: Verified — Plugin enum extended with PermanentFreezeExecute and Rust updated.
programs/mpl-core/src/plugins/mod.rs defines and uses PermanentFreezeExecute (see lines ~68, 114, 148, 208, 221, 250, 278, 331, 449); mappings/uses updated and discriminant ordering is preserved — no changes required.programs/mpl-core/src/plugins/lifecycle.rs (1)
205-206: Execute checks updated to include PermanentFreezeExecute → CanReject.Parity with FreezeExecute achieved. No further changes needed here.
programs/mpl-core/src/plugins/internal/permanent/mod.rs (2)
5-5: Module wired: permanent_freeze_execute.Compilation unit added; good.
12-12: Resolve — re-export is correct; struct & impls present.programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs: pub struct PermanentFreezeExecute (line 15); impl DataBlob (line 35); impl PluginValidation (line 41).
clients/js/src/plugins/types.ts (5)
22-23: Import generated PermanentFreezeExecute types.Matches IDL; OK.
103-107: CreatePluginArgs: add PermanentFreezeExecute variant.Correctly mirrors generated args and keeps Create-only vs owner-managed separation elsewhere.
128-131: CreateOnlyPluginArgsV2: add PermanentFreezeExecute.Ensures create-time only addition on assets/collections; good.
211-211: Type alias for PermanentFreezeExecutePlugin added.Consistent with other plugin aliases.
225-226: Expose permanentFreezeExecute in CommonPluginsList.Naming matches existing style; OK.
programs/mpl-core/src/plugins/mod.rs (8)
67-69: Plugin enum: added PermanentFreezeExecute variant.Appended safely; matches IDL/clients.
114-115: inner(): dispatch includes PermanentFreezeExecute.Prevents unreachable path/panic; good.
148-151: DataBlob::len covers PermanentFreezeExecute.Serialization length stays accurate; tests updated accordingly.
206-209: PluginType: add PermanentFreezeExecute.Keeps enum in sync with Plugin and IDL.
249-251: From<&Plugin> → PluginType mapping includes PermanentFreezeExecute.Round‑trip mapping remains complete.
278-279: manager(): PermanentFreezeExecute → UpdateAuthority.Matches “permanent” semantics and JS typing; OK.
331-332: Tests: empty-size coverage includes PermanentFreezeExecute.Good guard for serialization size invariants.
449-451: Tests: different-size coverage includes PermanentFreezeExecute.Completes variant matrix; OK.
clients/js/test/plugins/asset/permanentFreezeExecute.test.ts (1)
31-130: Good end-to-end coverage.The scenarios (freeze, unfreeze, duplicate add, persist across transfer, removal gating, burn unaffected) are comprehensive and aligned with program behavior.
Also applies to: 132-351, 353-461
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (1)
31-134: Solid collection-level coverage and inheritance checks.Tests assert create-only semantics, removal gating by frozen state, inheritance, and asset-level overrides. Looks good.
Also applies to: 136-223, 225-531
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts
Outdated
Show resolved
Hide resolved
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
Show resolved
Hide resolved
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
Show resolved
Hide resolved
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
Show resolved
Hide resolved
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs
Outdated
Show resolved
Hide resolved
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)
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs(2 hunks)programs/mpl-core/src/plugins/lifecycle.rs(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
programs/mpl-core/src/plugins/internal/permanent/**
📄 CodeRabbit inference engine (CLAUDE.md)
Add immutable internal plugins under plugins/internal/permanent/
Files:
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs
programs/mpl-core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep the on-chain Solana program under programs/mpl-core/
Files:
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rsprograms/mpl-core/src/plugins/lifecycle.rs
🧠 Learnings (1)
📚 Learning: 2025-09-05T20:01:56.374Z
Learnt from: CR
PR: metaplex-foundation/mpl-core#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T20:01:56.374Z
Learning: Applies to programs/mpl-core/src/plugins/internal/permanent/** : Add immutable internal plugins under plugins/internal/permanent/
Applied to files:
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs
🧬 Code graph analysis (2)
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs (2)
clients/js/src/generated/types/plugin.ts (1)
Plugin(74-92)clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)
programs/mpl-core/src/plugins/lifecycle.rs (1)
clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)
⏰ 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). (3)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
programs/mpl-core/src/plugins/lifecycle.rs (2)
108-111: Permit PermanentFreezeExecute to veto removal — LGTMAdding PermanentFreezeExecute => CanReject for remove aligns with immutable semantics and matches other permanent delegates. Explicit arm is fine even though the default is CanReject.
205-207: Execute gating wired for PermanentFreezeExecute — confirmed (validate_execute rejects when frozen)PermanentFreezeExecute::validate_execute rejects Execute when frozen (impl at programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs).
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs (1)
5-7: Importing Plugin for enum pattern matching — LGTMNeeded for matching target_plugin against Plugin::PermanentFreezeExecute.
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs (2)
11-12: Doc accuracy: authority is UpdateAuthority, not “any authority”.Align docs with PluginType::manager returning UpdateAuthority for this variant.
-/// The permanent freeze execute plugin allows any authority to lock the asset so its **Execute** lifecycle -/// event can be conditionally blocked. The default authority for this plugin is the update authority. +/// The permanent freeze execute plugin allows the UpdateAuthority to lock the asset so its **Execute** +/// lifecycle event can be conditionally blocked. The manager for this plugin is the UpdateAuthority.
55-68: Comment contradicts behavior; avoid unwrap; clarify create‑only enforcement.This method only rejects duplicates; create‑only is enforced centrally. Also prefer if‑let to avoid unwrap.
- // This plugin can only be added at creation time, so we - // always reject it. - if ctx.target_plugin.is_some() - && PluginType::from(ctx.target_plugin.unwrap()) == PluginType::PermanentFreezeExecute - { - reject!() - } else { - abstain!() - } + // This plugin is create-only; framework enforces that. Here we only prevent duplicates. + if let Some(target) = ctx.target_plugin { + if PluginType::from(target) == PluginType::PermanentFreezeExecute { + return reject!(); + } + } + abstain!()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
clients/rust/src/hooked/advanced_types.rs(3 hunks)clients/rust/src/hooked/mod.rs(1 hunks)clients/rust/src/hooked/plugin.rs(2 hunks)programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
clients/rust/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the Rust client library in clients/rust/
Files:
clients/rust/src/hooked/mod.rsclients/rust/src/hooked/advanced_types.rsclients/rust/src/hooked/plugin.rs
programs/mpl-core/src/plugins/internal/permanent/**
📄 CodeRabbit inference engine (CLAUDE.md)
Add immutable internal plugins under plugins/internal/permanent/
Files:
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
programs/mpl-core/**
📄 CodeRabbit inference engine (CLAUDE.md)
Keep the on-chain Solana program under programs/mpl-core/
Files:
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
🧠 Learnings (1)
📚 Learning: 2025-09-05T20:01:56.374Z
Learnt from: CR
PR: metaplex-foundation/mpl-core#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T20:01:56.374Z
Learning: Applies to programs/mpl-core/src/plugins/internal/permanent/** : Add immutable internal plugins under plugins/internal/permanent/
Applied to files:
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
🧬 Code graph analysis (4)
clients/rust/src/hooked/mod.rs (2)
clients/js/src/generated/types/plugin.ts (1)
Plugin(74-92)clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs (5)
clients/js/src/generated/types/plugin.ts (1)
Plugin(74-92)clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_delegate.rs (5)
new(24-26)default(30-32)len(36-38)validate_add_plugin(64-77)validate_remove_plugin(80-89)programs/mpl-core/src/plugins/lifecycle.rs (10)
len(40-42)validate_execute(407-412)validate_execute(649-654)validate_add_plugin(239-244)validate_add_plugin(537-542)from(68-70)from(74-78)from(493-499)validate_remove_plugin(247-259)validate_remove_plugin(546-551)programs/mpl-core/src/plugins/mod.rs (3)
len(122-152)len(225-227)from(231-252)
clients/rust/src/hooked/advanced_types.rs (2)
clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)clients/js/src/plugins/types.ts (2)
PermanentFreezeExecutePlugin(211-211)BasePlugin(41-44)
clients/rust/src/hooked/plugin.rs (3)
clients/js/src/plugins/types.ts (1)
PermanentFreezeExecutePlugin(211-211)clients/js/src/generated/types/plugin.ts (1)
Plugin(74-92)clients/js/src/generated/types/permanentFreezeExecute.ts (1)
PermanentFreezeExecute(11-11)
⏰ 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). (6)
- GitHub Check: Rust Client / Test
- GitHub Check: JS Client / Lint
- GitHub Check: JS Client / Test (20.x)
- GitHub Check: Benchmark Summary / Performance regression summary
- GitHub Check: JS Client / Test (22.x)
- GitHub Check: Rust Client / Build
🔇 Additional comments (7)
clients/rust/src/hooked/advanced_types.rs (3)
15-17: Import of PermanentFreezeExecute is correct and scoped appropriately.
176-181: Wrapper type PermanentFreezeExecutePlugin matches existing pattern.Structure mirrors other plugin wrappers (BasePlugin + inner). No issues.
201-202: PluginsList field added correctly and follows naming conventions.clients/rust/src/hooked/plugin.rs (2)
19-23: Public export list updated to include PermanentFreezeExecutePlugin.
358-363: New match arm correctly stores PermanentFreezeExecute into PluginsList.Add/extend a round‑trip test that:
- creates an account with PermanentFreezeExecute,
- reads PluginsList and asserts permanent_freeze_execute.is_some().
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs (1)
70-82: Fix to read stored on‑chain state on remove looks good.Using ctx.target_plugin’s stored.frozen prevents bypass via ephemeral self.
Please add a test asserting remove is rejected when stored.frozen == true and allowed (abstain) when false.
clients/rust/src/hooked/mod.rs (1)
52-53: Approve: PermanentFreezeExecute → PluginType mapping present in both core and hooked layers.
Confirmed: programs/mpl-core/src/plugins/mod.rs and clients/rust/src/hooked/mod.rs both map Plugin::PermanentFreezeExecute → PluginType::PermanentFreezeExecute.
programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (1)
184-191: Pass authority explicitly to updateCollectionPluginV1 calls.Avoid relying on implicit defaults; set authority: umi.identity for clarity and correct signer usage.
await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: true }, }), + authority: umi.identity, }).sendAndConfirm(umi);await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);Also applies to: 204-211, 288-295, 489-496
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
clients/js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the JavaScript client in clients/js/ (hand-written APIs under clients/js/src/)
Files:
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: blockiosaurus
PR: metaplex-foundation/mpl-core#247
File: programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs:0-0
Timestamp: 2025-09-16T17:12:49.489Z
Learning: In mpl-core PluginValidation trait methods, the `self` parameter represents the current on-chain plugin state loaded from the registry, not an ephemeral instruction value. When validate_remove_plugin is called, `self.frozen` correctly reads the stored frozen state.
📚 Learning: 2025-09-16T17:12:49.489Z
Learnt from: blockiosaurus
PR: metaplex-foundation/mpl-core#247
File: programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs:0-0
Timestamp: 2025-09-16T17:12:49.489Z
Learning: In mpl-core PluginValidation trait methods, the `self` parameter represents the current on-chain plugin state loaded from the registry, not an ephemeral instruction value. When validate_remove_plugin is called, `self.frozen` correctly reads the stored frozen state.
Applied to files:
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts
🧬 Code graph analysis (1)
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (8)
clients/js/test/_setupRaw.ts (5)
createCollection(91-108)assertCollection(176-213)DEFAULT_COLLECTION(50-53)assertAsset(137-174)DEFAULT_ASSET(45-48)clients/js/src/generated/instructions/removeCollectionPluginV1.ts (1)
removeCollectionPluginV1(82-160)clients/js/src/generated/instructions/addCollectionPluginV1.ts (1)
addCollectionPluginV1(100-178)clients/js/src/plugins/lib.ts (1)
createPlugin(45-61)clients/js/src/generated/instructions/updateCollectionPluginV1.ts (1)
updateCollectionPluginV1(82-160)clients/js/src/instructions/create.ts (1)
create(32-109)clients/js/src/generated/accounts/assetV1.ts (1)
fetchAssetV1(46-57)clients/js/src/generated/accounts/assetSigner.ts (1)
findAssetSignerPda(121-136)
⏰ 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). (3)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (1)
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (1)
119-121: Confirm expected error types align with on-chain implementation.All failure cases assert name: 'InvalidAuthority'. Please verify the program returns this error (and not, e.g., ExecuteBlocked/PluginFrozen) for: removing while frozen, adding post-creation, and execute while frozen.
#!/bin/bash # Verify error variants for PermanentFreezeExecute in the program. rg -n -C2 -S "PermanentFreezeExecute|validate_remove_plugin|validate_execute|InvalidAuthority|Execute" programs/mpl-coreAlso applies to: 149-151, 285-285, 441-441, 473-473
danenbm
left a comment
danenbm
left a comment
There was a problem hiding this comment.
approved but did have a couple extra test requests (i think one of them actually covered by another test)
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
clients/js/test/plugins/asset/permanentFreezeExecute.test.ts (4)
95-101: Pass authority explicitly on updatePluginV1.Avoid relying on builder defaults; make the signer explicit for deterministic behavior and to match UpdateAuthority semantics.
await updatePluginV1(umi, { asset: asset.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);
370-375: Also pass authority when adding TransferDelegate.Keeps errors deterministic and documents who is authorizing the add (owner in this test).
await addPluginV1(umi, { asset: asset.publicKey, plugin: createPlugin({ type: 'TransferDelegate', }), + authority: umi.identity, }).sendAndConfirm(umi);
447-450: Burn should specify authority (owner).burnV1 expects an authority; passing it explicitly avoids reliance on implicit behavior.
await burnV1(umi, { asset: asset.publicKey, - payer: owner, + payer: owner, + authority: owner, }).sendAndConfirm(umi);
139-146: Add authority on addPluginV1 for deterministic InvalidAuthority.Without an authority, errors may stem from missing signers instead of plugin validation. Pass the update authority explicitly.
const result = addPluginV1(umi, { asset: asset.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: true }, }), + authority: umi.identity, }).sendAndConfirm(umi);clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (4)
78-81: Pass authority on removeCollectionPluginV1.Ensure the correct signer is provided so failures come from plugin validation (not account/signature issues).
await removeCollectionPluginV1(umi, { collection: collection.publicKey, pluginType: PluginType.PermanentFreezeExecute, + authority: umi.identity, }).sendAndConfirm(umi);const result = removeCollectionPluginV1(umi, { collection: collection.publicKey, pluginType: PluginType.PermanentFreezeExecute, + authority: umi.identity, }).sendAndConfirm(umi);Also applies to: 114-118
141-147: Pass authority on addCollectionPluginV1.Explicit authority avoids brittle defaults and yields deterministic errors.
const result = addCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: true }, }), + authority: umi.identity, }).sendAndConfirm(umi);
184-190: Pass authority on updateCollectionPluginV1 calls.These updates should be authorized by the collection’s update authority; set authority explicitly.
await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: true }, }), + authority: umi.identity, }).sendAndConfirm(umi);await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);await updateCollectionPluginV1(umi, { collection: collection.publicKey, plugin: createPlugin({ type: 'PermanentFreezeExecute', data: { frozen: false }, }), + authority: umi.identity, }).sendAndConfirm(umi);Also applies to: 204-210, 288-294, 489-495
262-265: Fix incorrect array destructuring of Pda from findAssetSignerPda (runtime/type error).findAssetSignerPda returns a Pda, not an iterable; destructuring will crash. Use direct assignment.
- const [assetSignerPda] = findAssetSignerPda(umi, { + const assetSignerPda = findAssetSignerPda(umi, { asset: asset.publicKey, });Also applies to: 361-364, 419-421
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
clients/js/test/plugins/asset/permanentFreezeExecute.test.ts(1 hunks)clients/js/test/plugins/collection/permanentFreezeExecute.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
clients/js/**
📄 CodeRabbit inference engine (CLAUDE.md)
Maintain the JavaScript client in clients/js/ (hand-written APIs under clients/js/src/)
Files:
clients/js/test/plugins/collection/permanentFreezeExecute.test.tsclients/js/test/plugins/asset/permanentFreezeExecute.test.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: blockiosaurus
PR: metaplex-foundation/mpl-core#247
File: programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs:0-0
Timestamp: 2025-09-16T17:12:49.489Z
Learning: In mpl-core PluginValidation trait methods, the `self` parameter represents the current on-chain plugin state loaded from the registry, not an ephemeral instruction value. When validate_remove_plugin is called, `self.frozen` correctly reads the stored frozen state.
📚 Learning: 2025-09-16T17:12:49.489Z
Learnt from: blockiosaurus
PR: metaplex-foundation/mpl-core#247
File: programs/mpl-core/src/plugins/internal/permanent/permanent_freeze_execute.rs:0-0
Timestamp: 2025-09-16T17:12:49.489Z
Learning: In mpl-core PluginValidation trait methods, the `self` parameter represents the current on-chain plugin state loaded from the registry, not an ephemeral instruction value. When validate_remove_plugin is called, `self.frozen` correctly reads the stored frozen state.
Applied to files:
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts
📚 Learning: 2025-09-05T20:01:56.374Z
Learnt from: CR
PR: metaplex-foundation/mpl-core#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-05T20:01:56.374Z
Learning: Applies to programs/mpl-core/src/plugins/internal/authority_managed/** : Add collection-level, authority-managed internal plugins under plugins/internal/authority_managed/
Applied to files:
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts
🧬 Code graph analysis (2)
clients/js/test/plugins/collection/permanentFreezeExecute.test.ts (5)
clients/js/test/_setupRaw.ts (5)
createCollection(91-108)assertCollection(176-213)DEFAULT_COLLECTION(50-53)assertAsset(137-174)DEFAULT_ASSET(45-48)clients/js/src/plugins/lib.ts (1)
createPlugin(45-61)clients/js/src/instructions/create.ts (1)
create(32-109)clients/js/src/generated/accounts/assetV1.ts (1)
fetchAssetV1(46-57)clients/js/src/generated/accounts/assetSigner.ts (1)
findAssetSignerPda(121-136)
clients/js/test/plugins/asset/permanentFreezeExecute.test.ts (10)
clients/js/src/instructions/create.ts (1)
create(32-109)clients/js/src/generated/accounts/assetV1.ts (1)
fetchAssetV1(46-57)clients/js/test/_setupRaw.ts (4)
assertAsset(137-174)DEFAULT_ASSET(45-48)createAsset(55-80)assertBurned(215-226)clients/js/src/generated/accounts/assetSigner.ts (1)
findAssetSignerPda(121-136)clients/js/src/generated/instructions/updatePluginV1.ts (1)
updatePluginV1(81-163)clients/js/src/plugins/lib.ts (1)
createPlugin(45-61)clients/js/src/generated/instructions/addPluginV1.ts (1)
addPluginV1(98-180)clients/js/src/generated/instructions/transferV1.ts (1)
transferV1(94-174)clients/js/src/generated/instructions/removePluginV1.ts (1)
removePluginV1(81-163)clients/js/src/generated/instructions/burnV1.ts (1)
burnV1(88-163)
⏰ 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). (3)
- GitHub Check: Programs / Build
- GitHub Check: Programs / Test (mpl-core)
- GitHub Check: Cursor Bugbot
No description provided.