feat: magic fee vault as vault on ER for receiving payments#149
feat: magic fee vault as vault on ER for receiving payments#149
Conversation
WalkthroughThis PR introduces two new magic fee vault operations: initialization and delegation. It adds discriminator variants, instruction builders, PDA helpers, processor functions, and comprehensive tests for both operations. A serde dependency is also updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/test_delegate_magic_fee_vault.rs`:
- Around line 84-156: The wrong-validator test creates new PDAs for the wrong
validator instead of calling delegate_magic_fee_vault against the
already-initialized vault, and it only asserts "any error"; update
test_delegate_magic_fee_vault_fails_with_wrong_validator to reuse the existing
vault/accounts created by setup_vault (i.e., pass the same validator-owned
PDA/account pubkeys that setup_vault initialized) but sign the tx with
wrong_validator, then assert the specific InstructionError/program error
returned (unwrap_err and match the InstructionError or program-specific custom
error you expect) rather than assert!(res.is_err()).
In `@tests/test_init_magic_fee_vault.rs`:
- Around line 57-75: Update the assertion in
test_init_magic_fee_vault_fails_without_validator_fees_vault to verify the
specific program error instead of just assert!(res.is_err()); after
banks.process_transaction(tx).await unwrap the Err to an InstructionError (or to
a TransportError then downcast) and assert it equals the expected custom program
error returned by your program (use the error enum/variant from your crate, e.g.
compare to dlp::error::YourProgramError::MissingValidatorFeesVault or the
corresponding InstructionError::Custom(code)); locate the test function and the
init_magic_fee_vault call to change the assertion accordingly and replace the
placeholder variant with the actual error variant/code from your program.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 166d9e24-a260-42d1-bc0a-52d03974c9dd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
Cargo.tomlsrc/discriminator.rssrc/instruction_builder/delegate_magic_fee_vault.rssrc/instruction_builder/init_magic_fee_vault.rssrc/instruction_builder/mod.rssrc/lib.rssrc/pda.rssrc/processor/delegate_magic_fee_vault.rssrc/processor/init_magic_fee_vault.rssrc/processor/mod.rstests/test_commit_finalize.rstests/test_delegate_magic_fee_vault.rstests/test_init_magic_fee_vault.rs
| async fn test_delegate_magic_fee_vault_fails_without_fees_vault() { | ||
| let (banks, payer, _admin, validator, blockhash) = | ||
| setup_program_test_env().await; | ||
|
|
||
| // No validator fees vault or magic fee vault initialized — should fail | ||
| let ix = dlp::instruction_builder::delegate_magic_fee_vault( | ||
| payer.pubkey(), | ||
| validator.pubkey(), | ||
| ); | ||
| let tx = Transaction::new_signed_with_payer( | ||
| &[ix], | ||
| Some(&payer.pubkey()), | ||
| &[&payer, &validator], | ||
| blockhash, | ||
| ); | ||
| let res = banks.process_transaction(tx).await; | ||
| assert!(res.is_err()); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_delegate_magic_fee_vault_fails_without_magic_fee_vault() { | ||
| let (banks, payer, admin, validator, blockhash) = | ||
| setup_program_test_env().await; | ||
|
|
||
| // Init validator fees vault but NOT the magic fee vault | ||
| let ix = dlp::instruction_builder::init_validator_fees_vault( | ||
| payer.pubkey(), | ||
| admin.pubkey(), | ||
| validator.pubkey(), | ||
| ); | ||
| let tx = Transaction::new_signed_with_payer( | ||
| &[ix], | ||
| Some(&payer.pubkey()), | ||
| &[&payer, &admin], | ||
| blockhash, | ||
| ); | ||
| banks.process_transaction(tx).await.unwrap(); | ||
|
|
||
| let ix = dlp::instruction_builder::delegate_magic_fee_vault( | ||
| payer.pubkey(), | ||
| validator.pubkey(), | ||
| ); | ||
| let tx = Transaction::new_signed_with_payer( | ||
| &[ix], | ||
| Some(&payer.pubkey()), | ||
| &[&payer, &validator], | ||
| blockhash, | ||
| ); | ||
| let res = banks.process_transaction(tx).await; | ||
| assert!(res.is_err()); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn test_delegate_magic_fee_vault_fails_with_wrong_validator() { | ||
| let (banks, payer, admin, validator, blockhash) = | ||
| setup_program_test_env().await; | ||
|
|
||
| setup_vault(&banks, &payer, &admin, &validator, blockhash).await; | ||
|
|
||
| // A different validator tries to delegate the vault | ||
| let wrong_validator = Keypair::new(); | ||
| let ix = dlp::instruction_builder::delegate_magic_fee_vault( | ||
| payer.pubkey(), | ||
| wrong_validator.pubkey(), | ||
| ); | ||
| let tx = Transaction::new_signed_with_payer( | ||
| &[ix], | ||
| Some(&payer.pubkey()), | ||
| &[&payer, &wrong_validator], | ||
| blockhash, | ||
| ); | ||
| let res = banks.process_transaction(tx).await; | ||
| assert!(res.is_err()); |
There was a problem hiding this comment.
Tighten the negative-path assertions, especially the wrong-validator case.
All three failure tests pass on any error, and test_delegate_magic_fee_vault_fails_with_wrong_validator currently derives brand-new PDAs from wrong_validator, so it never exercises “wrong validator against the existing vault” at all. Build that case against the already-initialized vault/accounts and assert the specific InstructionError or program error you expect.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_delegate_magic_fee_vault.rs` around lines 84 - 156, The
wrong-validator test creates new PDAs for the wrong validator instead of calling
delegate_magic_fee_vault against the already-initialized vault, and it only
asserts "any error"; update
test_delegate_magic_fee_vault_fails_with_wrong_validator to reuse the existing
vault/accounts created by setup_vault (i.e., pass the same validator-owned
PDA/account pubkeys that setup_vault initialized) but sign the tx with
wrong_validator, then assert the specific InstructionError/program error
returned (unwrap_err and match the InstructionError or program-specific custom
error you expect) rather than assert!(res.is_err()).
| #[tokio::test] | ||
| async fn test_init_magic_fee_vault_fails_without_validator_fees_vault() { | ||
| let (banks, payer, _admin, validator, blockhash) = | ||
| setup_program_test_env().await; | ||
|
|
||
| // Skip initializing the validator fees vault — should fail | ||
| let ix = dlp::instruction_builder::init_magic_fee_vault( | ||
| payer.pubkey(), | ||
| validator.pubkey(), | ||
| ); | ||
| let tx = Transaction::new_signed_with_payer( | ||
| &[ix], | ||
| Some(&payer.pubkey()), | ||
| &[&payer, &validator], | ||
| blockhash, | ||
| ); | ||
| let res = banks.process_transaction(tx).await; | ||
| assert!(res.is_err()); | ||
| } |
There was a problem hiding this comment.
Assert the whitelist guard’s error, not just is_err().
This test currently passes for any transaction failure, so a setup or account-order regression would look identical to the intended “validator fees vault missing” rejection. Check the concrete InstructionError or program error here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/test_init_magic_fee_vault.rs` around lines 57 - 75, Update the
assertion in test_init_magic_fee_vault_fails_without_validator_fees_vault to
verify the specific program error instead of just assert!(res.is_err()); after
banks.process_transaction(tx).await unwrap the Err to an InstructionError (or to
a TransportError then downcast) and assert it equals the expected custom program
error returned by your program (use the error enum/variant from your crate, e.g.
compare to dlp::error::YourProgramError::MissingValidatorFeesVault or the
corresponding InstructionError::Custom(code)); locate the test function and the
init_magic_fee_vault call to change the assertion accordingly and replace the
placeholder variant with the actual error variant/code from your program.
GabrielePicco
left a comment
There was a problem hiding this comment.
LGTM!
Please let's also add an integration tests, so that we can use it to intialize/delegate the vaults for the existing validators
Problem
What problem are you trying to solve?
Solution
How did you solve the problem?
On ER we need to receive payments for Intent scheduling. With this on startup validator needs to make sure that vault exists and delegated, otherwise do it. If that is not done commits and undelegations would get rejected
Before & After Screenshots
Insert screenshots of example code output
BEFORE:
[insert screenshot here]
AFTER:
[insert screenshot here]
Other changes (e.g. bug fixes, small refactors)
Deploy Notes
Notes regarding deployment of the contained body of work. These should note any
new dependencies, new scripts, etc.
New scripts:
script: script detailsNew dependencies:
dependency: dependency detailsSummary by CodeRabbit
New Features
Tests
Chores