-
Notifications
You must be signed in to change notification settings - Fork 0
feat: charge escrow if present #3
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
Conversation
cc2cb87 to
845a6fe
Compare
bmuddha
left a 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.
have a few concerns, and a potentially an escrow account dissociation bug.
Verified correct behaviour with integration test: https://github.com/magicblock-labs/magicblock-validator/pull/528/files#diff-eee5d643b14736cae0b44b1bbf83a985a87987b65fcbd1f02a57cd4d3617a52eR15 (which can be merged once the new cloning pipeline is integrated) |
c9be044 to
3a5f94e
Compare
bmuddha
left a 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.
LGTM overall, with a few minor nits, I need to test the interaction with the accountsdb, that correct feepayer is charged and persisted. And it should be good to merge.
Co-authored-by: Babur Makhmudov <31780624+bmuddha@users.noreply.github.com>
Thanks @bmuddha, applied suggested nits |
bmuddha
left a 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.
LGTM, higher level integration tests are green
Ephemeral Escrow Fee Payer Support
ephemeral_balance_pda_from_payerfunction in the newescrowmodule to derive an ephemeral escrow account for a given payer, and integrated it into transaction fee validation so that if the main fee payer account does not exist, the system will attempt to charge fees from the escrow account instead (src/escrow.rs,src/transaction_processor.rs). [1] [2] [3]CI and Dependency Updates
Cargo.tomlwith new dependencies required for escrow and reserved account key support (.github/workflows/cargo-test.yml,Cargo.toml). [1] [2] [3] [4]