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

AutoRelayer: remove forwards #3287

Merged
merged 5 commits into from
Aug 21, 2023
Merged

Conversation

JoeHowarth
Copy link
Contributor

Forwards are currently broken due to charging the integrator's gas budget for temporary storage slots that can't be refunded properly to the integrator.

In this pr:

  • remove all forwards public functions
  • remove clearly forwards-only code paths
  • remove or rework forwards specific tests

The pr will be followed by a pr allowing cheap cross-chain refunds to make up for the lack of forwards. This will be achieved by not verifying the VAA.

// and there ends up not being enough funds,
// then we will revert this call)
// (Previously needed for reverts, but waiting to remove since this is sensitive code
// and would need a real audit to redeploy)
try
Copy link
Contributor

Choose a reason for hiding this comment

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

correct me if I'm wrong, but executeInstruction should never revert right? Can we not remove the failure case of this

Copy link
Contributor

Choose a reason for hiding this comment

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

as in, can we just change this into a normal function call rather than an external call

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I think you're right, but was doing the "only remove dead code" pr first like we discussed.

@derpy-duck
Copy link
Contributor

tryDecodeExecuteInstructionError can be removed too probably? (since there will be no errors)

}
if (!fs.existsSync(TS)) {
fs.mkdirSync(TS);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

do we remove this file's changes from this PR

@derpy-duck derpy-duck self-requested a review August 17, 2023 01:57
derpy-duck
derpy-duck previously approved these changes Aug 17, 2023
@JoeHowarth JoeHowarth changed the base branch from main to auto-relayer/v1.1 August 17, 2023 21:02
@JoeHowarth JoeHowarth merged commit 5e82607 into auto-relayer/v1.1 Aug 21, 2023
22 checks passed
@JoeHowarth JoeHowarth deleted the auto-relayer/rm-forwards branch August 21, 2023 16:54
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