-
Notifications
You must be signed in to change notification settings - Fork 8
fix: allow delegation of not existing accounts #133
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
fix: allow delegation of not existing accounts #133
Conversation
WalkthroughAdds a new public constant Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-10-15T11:45:25.802ZApplied to files:
⏰ 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)
🔇 Additional comments (1)
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: 2
🤖 Fix all issues with AI Agents
In @src/consts.rs:
- Line 39: Replace the hardcoded RENT_EXCEPTION_ZERO_BYTES_LAMPORTS used in the
rent-exemption check for the delegated account with a dynamic calculation based
on the account's actual size: compute the minimum balance using
Rent::default().minimum_balance(delegated_account.data_len()) and use that value
when verifying or funding rent exemption (instead of the zero-byte constant) in
the rent-check logic that handles the delegated_account after data is copied
from the buffer.
In @src/processor/fast/delegate.rs:
- Around line 229-237: The rent-exemption flow is incorrect: don’t
unconditionally use RENT_EXCEPTION_ZERO_BYTES_LAMPORTS or capture
delegated_account.lamports() before determining final data size; instead, move
the rent calculation and potential top-up before updating the delegation record
and after you know the final data length (i.e., after copying data into the
buffer but before persisting the delegation state), compute required_rent using
the Rent sysvar (Rent::from_account_info or Rent::get()/fallback for tests)
based on delegated_account.data_len(), invoke a transfer only for (required_rent
- delegated_account.lamports()) when positive, and finally store the actual
lamports balance (delegated_account.lamports() post-top-up) into the delegation
record instead of the pre-top-up value to avoid state inconsistency.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/consts.rssrc/processor/fast/delegate.rs
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-29T10:43:00.068Z
Learnt from: Dodecahedr0x
Repo: magicblock-labs/delegation-program PR: 90
File: tests/test_protocol_claim_fees.rs:30-30
Timestamp: 2025-10-29T10:43:00.068Z
Learning: In Solana test files using solana_program_test, use Rent::default() instead of Rent::get() because the Rent sysvar is not available in the test context. Rent::get() is only available in on-chain program execution.
Applied to files:
src/consts.rs
📚 Learning: 2025-10-15T11:45:25.802Z
Learnt from: snawaz
Repo: magicblock-labs/delegation-program PR: 107
File: src/entrypoint.rs:141-144
Timestamp: 2025-10-15T11:45:25.802Z
Learning: In the delegation-program codebase, prefer using `log!` (from pinocchio_log) over `msg!` for error and panic scenarios in the entrypoint code, as per maintainer preference.
Applied to files:
src/processor/fast/delegate.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: lint
🔇 Additional comments (1)
src/processor/fast/delegate.rs (1)
10-10: LGTM: Imports added for rent exemption feature.The imports of system instructions and the rent exemption constant are correctly added to support the new functionality.
Also applies to: 13-13
Description
Allow delegation of not existing accounts by making them rent exempt
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.