-
Notifications
You must be signed in to change notification settings - Fork 22
fix: ordering of Callback accounts #42
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
base: main
Are you sure you want to change the base?
Conversation
feat: added docs
WalkthroughThis PR modifies the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
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 |
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
magic-actions/programs/magic-actions/src/lib.rs (1)
136-149: Critical: Test account ordering does not match updated struct definition.The Rust struct
UpdateLeaderboard(lines 136-149) expects accounts in this order:counter,escrow_auth,escrow. However, the test atmagic-actions/tests/magic-actions.ts:52-55provides them in the wrong order:counter,escrow,escrowAuth.Fix the test to match the new struct ordering:
.accounts({ counter: pda, escrowAuth: pda, escrow: pda, })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
magic-actions/programs/magic-actions/src/lib.rs(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
magic-actions/programs/magic-actions/src/lib.rs (1)
anchor-counter/programs/anchor-counter/src/lib.rs (1)
commit(50-58)
🔇 Additional comments (3)
magic-actions/programs/magic-actions/src/lib.rs (3)
41-50: LGTM - Formatting improvements.The added whitespace and multi-line formatting of the
msg!macro improve readability without changing functionality.
75-115: LGTM - Minor formatting adjustments.The whitespace additions and line break adjustments improve code organization without affecting behavior.
84-101: Verify CallHandler accounts list is complete.The
accountsvector inCallHandlerincludes onlyleaderboardandcounter, but theUpdateLeaderboardstruct now requires four accounts:leaderboard,counter,escrow_auth, andescrow.Based on the documentation comments indicating these accounts "will be moved to the end in the future, meaning you can omit this unless needed," it appears the Magic framework automatically appends the escrow accounts during callback execution. However, please confirm this is the correct behavior and that the framework properly derives and validates these accounts.
Consider adding a code comment here explaining that the Magic framework automatically provides the escrow accounts, for clarity.
| @@ -1,10 +1,10 @@ | |||
| use anchor_lang::prelude::*; | |||
| use anchor_lang::Discriminator; | |||
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.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Verify if the Discriminator import is needed.
The Discriminator trait is imported but doesn't appear to be used explicitly in the code. If it's not required by the macros or used elsewhere, consider removing it to keep imports clean.
Run the following script to check if Discriminator is actually used:
🏁 Script executed:
#!/bin/bash
# Check if Discriminator is used anywhere in the file
rg -n 'Discriminator' magic-actions/programs/magic-actions/src/lib.rsLength of output: 33
Remove the unused Discriminator import on line 2.
The Discriminator trait is imported from anchor_lang but is never used anywhere in the file. This is dead code that should be cleaned up.
🤖 Prompt for AI Agents
In magic-actions/programs/magic-actions/src/lib.rs around line 2, the file
imports the unused trait `Discriminator` from `anchor_lang`; remove the dead
import line `use anchor_lang::Discriminator;` so the file no longer contains an
unused import and rebuild to ensure no other references exist.
Summary by CodeRabbit
Chores
Refactor