Skip to content
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(svm): H-01 optimize claiming relayer refunds #847

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

Reinis-FRP
Copy link
Contributor

@Reinis-FRP Reinis-FRP commented Jan 8, 2025

OZ identified following issue:

When a relayer within a transaction leaf cannot receive direct refunds, such as due to a failed token transfer caused by blacklisting, the protocol uses claim account PDAs to handle the refund. However, this fallback mechanism imposes limitations on other relayers within the same transaction, forcing all of them to claim refunds via a ClaimAccount PDA, even if direct refunds are possible for them. This leads to the following issues:

  • Risk of Hitting Compute Budget: The accrue_relayer_refunds process involves find_program_address, which can be computationally expensive as it iteratively attempts to validate a valid bump. Since this process can be used for several accounts, there is a high likelihood that the transaction will revert. Consequently, relayers and users may not be able to retrieve their refunds from that leaf.
  • Refund Delays: A relayer might claim their refund from its ClaimAccount PDA right before the execution of a leaf, reducing the vault’s total available funds. This could delay refunds for other users as the protocol might lack sufficient liquidity at the time of execution.
  • Increased Execution Overhead: Executors are responsible for ensuring that all relayers have initialized claim accounts and are incurring additional computational and financial overhead, including the cost of covering rent exemption fees.
  • Liquidity-Related Reverts: Relayers can attempt to claim refunds when the spoke pool lacks sufficient liquidity. This scenario might result in transaction reversion without providing a meaningful error message, as the hub pool does not track outstanding spoke pool debts.
  • Prone to misuse: Since the function can be invoked by anyone, malicious actors could front-run the data worker, unnecessarily deferring refunds even when direct refund mechanisms are operational. In addition, a malicious relayer might force the use of claim accounts and delay its refund claim until just before the execution of a leaf, deliberately triggering a revert to disrupt system operations.

Consider implementing logic to route refunds only through ClaimAccount PDAs for relayers that are explicitly unable to receive direct transfers. This would optimize operations, prevent errors, and reduce overhead. Moreover, consider enhancing error messaging to clearly indicate insufficient liquidity in the spoke pool during claim attempts. This change would ensure smoother operations and improved clarity for users.

We have opted to apply only a limited fix by optimizing the way how relayer refund claim instructions are composed by moving its parameters to account data.

Signed-off-by: Reinis Martinsons <[email protected]>
Copy link

linear bot commented Jan 8, 2025

Signed-off-by: Reinis Martinsons <[email protected]>
tokenProgram: TOKEN_PROGRAM_ID,
program: program.programId,
};
// Will not claim for the first relayer as if its token account was blacklisted.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why do we simulate the first relayer being blacklisted?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, this was rather arbitrary. now simplified the tests by claiming to all accounts

md0x
md0x previously approved these changes Jan 8, 2025
Copy link
Contributor

@md0x md0x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Signed-off-by: Reinis Martinsons <[email protected]>

it("Execute Max multiple refunds with claims in separate phase versioned transactions", async () => {
// Larger amount would hit maximum instruction trace length limit.
const solanaDistributions = 21;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement!

@Reinis-FRP Reinis-FRP changed the title test(svm): max atomic refund claims test(svm): H-01 optimize claiming relayer refunds Jan 9, 2025
@Reinis-FRP Reinis-FRP changed the title test(svm): H-01 optimize claiming relayer refunds fix(svm): H-01 optimize claiming relayer refunds Jan 9, 2025
@Reinis-FRP Reinis-FRP marked this pull request as ready for review January 9, 2025 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants