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 stack overflow in Anker::SendRewards #566

Merged
merged 2 commits into from
May 18, 2022
Merged

Fix stack overflow in Anker::SendRewards #566

merged 2 commits into from
May 18, 2022

Conversation

ruuda
Copy link
Contributor

@ruuda ruuda commented May 12, 2022

Problem

Currently, our maintainer is failing to call SendRewards on mainnet:

Error while performing maintenance.
Solana RPC client returned an error:

  Request:    Some(SendTransaction)
  Kind:       RPC response error
  Error code: -32002
  Message:    Transaction simulation failed: Error processing Instruction 0: Program failed to complete
  Reason:     Transaction preflight failure
  Error:     

    Raw:      InstructionError(0, ProgramFailedToComplete)
    Display:  Error processing Instruction 0: Program failed to complete


  Logs:      

    Program BNVB8pd4coHpY7MVcrtiHLCLst7fyDGzMtPmfJqFAhwj invoke [1]
    Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA invoke [2]
    Program log: Instruction: Approve
    Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA consumed 2026 of 153383 compute units
    Program TokenkegQfeZyiNwAJbNbGKPFXCWuBvf9Ss623VQ5DA success
    Program BNVB8pd4coHpY7MVcrtiHLCLst7fyDGzMtPmfJqFAhwj consumed 52383 of 200000 compute units
    Program failed to complete: Access violation in stack frame 3 at address 0x200003fe8 of size 8 by instruction #19763
    Program BNVB8pd4coHpY7MVcrtiHLCLst7fyDGzMtPmfJqFAhwj failed: Program failed to complete

We saw this access violation before, it is caused by a stack overflow. We ran into this in SendRewards previously and we already optimized it to reduce stack usage, but I suspect the late addition of some metrics increased the size of the Anker struct which pushed it over the edge again.

Changes

  • Add a test that reproduces the stack overflow and fails in case it happens. The test calls SendRewards which will fail either way in our test context because we don’t have the Wormhole program and all its dependent accounts available there. (Setting up a token swap instance was hard enough, I don’t think it is feasible to get a working Wormhole instance in the test context.) But we can tell apart the failure due to stack overflow from the failure due to trying to call a non-existing program.
  • Make the test pass by boxing a few more things to free up stack space.

ruuda added 2 commits May 12, 2022 12:18
We cannot really test this call without the full Wormhole programs and
all of their associated accounts and dependencies in place, which is a
pain. But we can at least test that can get one particular error. I am
hoping that failing to call the Wormhole program will result in a
different error than the "ProgramFailedToComplete" that we get now, and
that we can test for the stack overflow this way.

The new test currently fails.
Put more things on the heap to free up some stack space. This makes the
test pass, it now fails at trying to call the program, it no longer
overflows the stack.
@ruuda ruuda requested a review from enriquefynn May 12, 2022 10:34
Copy link
Member

@enriquefynn enriquefynn left a comment

Choose a reason for hiding this comment

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

Tested in-between commits.
LGTM!

@ruuda ruuda merged commit 64fd772 into main May 18, 2022
@ruuda ruuda deleted the send-rewards-stack branch May 18, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants