feat: magic fee vault as vault on ER for receiving payments#149
Merged
feat: magic fee vault as vault on ER for receiving payments#149
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Magic Fee Vault support across the codebase: two new DlpDiscriminator variants ( 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)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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
GabrielePicco
approved these changes
Mar 12, 2026
Contributor
GabrielePicco
left a comment
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
# Conflicts: # Cargo.toml # dlp-api/src/instruction_builder/delegate_magic_fee_vault.rs # dlp-api/src/instruction_builder/init_magic_fee_vault.rs # dlp-api/src/instruction_builder/mod.rs # src/discriminator.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/test_init_magic_fee_vault.rs (1)
57-75:⚠️ Potential issue | 🟡 MinorAssert the exact program error, not just
is_err().
Line 74currently passes on any transaction failure, so unrelated regressions can be misclassified as the expected guard failure.Suggested tightening
- let res = banks.process_transaction(tx).await; - assert!(res.is_err()); + let err = banks.process_transaction(tx).await.unwrap_err(); + // Assert the precise custom error returned when validator fees vault is missing. + // Replace `EXPECTED_ERROR_CODE` with the actual program error code/variant. + assert!(matches!( + err, + solana_sdk::transport::TransportError::TransactionError( + solana_sdk::transaction::TransactionError::InstructionError( + _, + solana_sdk::instruction::InstructionError::Custom(code) + ) + ) if code == EXPECTED_ERROR_CODE + ));🤖 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, The test test_init_magic_fee_vault_fails_without_validator_fees_vault should assert the exact program error instead of just res.is_err(); update the assertion to unwrap the transaction error and match it against the specific program error returned by your program (e.g., convert your custom error enum variant like dlp_api::error::DlpError::MissingValidatorFeesVault to the program error code and assert TransactionError::InstructionError(_, InstructionError::Custom(code)) or use assert_matches! to compare to that exact variant); modify the assertion in the test to check for that specific error rather than a general is_err().
🤖 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 83-101: The tests
test_delegate_magic_fee_vault_fails_without_fees_vault and
test_delegate_magic_fee_vault_fails_without_magic_fee_vault should assert the
specific transaction/InstructionError rather than only checking is_err(); update
each to call let err = res.unwrap_err() and assert that err matches
solana_sdk::transaction::TransactionError::InstructionError(0,
solana_program::instruction::InstructionError::InvalidAccountData) (or the
correct InstructionError your program returns) using matches! with a clear
failure message so the test verifies the exact expected rejection for the
dlp_api::instruction_builder::delegate_magic_fee_vault invocation.
---
Duplicate comments:
In `@tests/test_init_magic_fee_vault.rs`:
- Around line 57-75: The test
test_init_magic_fee_vault_fails_without_validator_fees_vault should assert the
exact program error instead of just res.is_err(); update the assertion to unwrap
the transaction error and match it against the specific program error returned
by your program (e.g., convert your custom error enum variant like
dlp_api::error::DlpError::MissingValidatorFeesVault to the program error code
and assert TransactionError::InstructionError(_, InstructionError::Custom(code))
or use assert_matches! to compare to that exact variant); modify the assertion
in the test to check for that specific error rather than a general is_err().
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3e651b27-df2d-4515-ae92-7050ebd9b017
📒 Files selected for processing (9)
dlp-api/src/instruction_builder/delegate_magic_fee_vault.rsdlp-api/src/instruction_builder/init_magic_fee_vault.rsdlp-api/src/instruction_builder/mod.rssrc/discriminator.rssrc/lib.rssrc/processor/delegate_magic_fee_vault.rstests/test_commit_finalize.rstests/test_delegate_magic_fee_vault.rstests/test_init_magic_fee_vault.rs
Contributor
Author
Added. Right now validator checks for vault and tries to create + delegate vault, otherwise we crash |
Contributor
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/integration/tests/test-delegation.ts`:
- Around line 418-423: Replace the manual PDA derivation for delegation metadata
with the SDK helper: instead of calling
web3.PublicKey.findProgramAddressSync(...) to compute delegationMetadata, call
delegationMetadataPdaFromDelegatedAccount(magicFeeVault) (matching how
delegationRecordPdaFromDelegatedAccount(magicFeeVault) is used) and use its
returned public key; update any variable name if needed to delegationMetadata to
keep usages consistent.
In `@tests/test_init_magic_fee_vault.rs`:
- Around line 50-56: The test currently only checks existence of
magic_fee_vault_account; add a stronger assertion that the account's owner
equals the delegation program ID by unwrapping the Option returned by
banks.get_account and asserting its owner field equals DELEGATION_PROGRAM_ID
(use the existing magic_fee_vault_pda_from_validator and magic_fee_vault_account
variables to locate the check and reference the DELEGATION_PROGRAM_ID constant).
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 020b06b7-029b-42ff-a698-ed0f71d19318
📒 Files selected for processing (2)
tests/integration/tests/test-delegation.tstests/test_init_magic_fee_vault.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/test_delegate_magic_fee_vault.rs (1)
160-191:⚠️ Potential issue | 🟡 MinorThis still doesn't test “wrong validator against the existing vault.”
delegate_magic_fee_vault(payer, wrong_validator.pubkey())derives a brand-new fees vault and magic-fee vault fromwrong_validator, so this is just another uninitialized-vault failure. If you want the authorization path, build the instruction against the already-initialized PDAs forvalidatorand only swap the validator signer/account towrong_validator, then assert the specific error.🤖 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 160 - 191, The test test_delegate_magic_fee_vault_fails_with_wrong_validator is currently deriving PDAs for wrong_validator (so it tests an uninitialized-vault failure) instead of exercising the authorization path; change the test to build the delegate_magic_fee_vault instruction using the already-initialized PDAs created by setup_vault (derive the fees vault and magic-fee vault from the original validator/keypair used in setup_vault) but sign/submit the transaction with wrong_validator as the signer/account, then assert the expected authorization-specific error (not InvalidAccountOwner) to verify wrong_validator is rejected when operating on an existing vault.
🤖 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/integration/tests/test-delegation.ts`:
- Around line 380-454: The tests use payer as the validator which masks bugs;
create a distinct validator Keypair (e.g., const validatorKeypair =
web3.Keypair.generate(); const validator = validatorKeypair.publicKey) and fund
it (airdrop or transfer) before these cases, update references in the
"Initialize magic fee vault" and "Delegate magic fee vault" tests to use that
validator public key (retain magicFeeVaultPda,
validatorFeesVaultPdaFromValidator, delegationRecordPdaFromDelegatedAccount,
etc.), ensure the validator keypair is marked as a signer in the transaction
submission (pass validatorKeypair to the routine that submits signatures via
processInstruction or equivalent) so the tests exercise a real distinct
validator signature rather than relying on payer === validator.
---
Duplicate comments:
In `@tests/test_delegate_magic_fee_vault.rs`:
- Around line 160-191: The test
test_delegate_magic_fee_vault_fails_with_wrong_validator is currently deriving
PDAs for wrong_validator (so it tests an uninitialized-vault failure) instead of
exercising the authorization path; change the test to build the
delegate_magic_fee_vault instruction using the already-initialized PDAs created
by setup_vault (derive the fees vault and magic-fee vault from the original
validator/keypair used in setup_vault) but sign/submit the transaction with
wrong_validator as the signer/account, then assert the expected
authorization-specific error (not InvalidAccountOwner) to verify wrong_validator
is rejected when operating on an existing vault.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7670cc55-b5f5-4081-9e10-7429d0fa4925
📒 Files selected for processing (2)
tests/integration/tests/test-delegation.tstests/test_delegate_magic_fee_vault.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
API
Tests