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(CA): using the bundled simulation for allowance and bridging transactions #919

Merged
merged 1 commit into from
Jan 30, 2025

Conversation

geekbrother
Copy link
Contributor

@geekbrother geekbrother commented Jan 29, 2025

Description

This PR changes the approach of the gas estimation from using the eth_estimateGas for the approval transaction and simulation for the bridging transaction to the bundled simulation for the approval and bridging transaction together.

The following changes were made:

  • The bundled simulation provider trait was added and the implementation for the Tenderly provider;
  • Flow of the estimation changed to simulate both transactions in a bundle;
  • The proper check for the transaction simulation failure was added;
  • The proper error handling for the simulation provider error and the simulation failure added;
  • The estimated gas slippage decreased to 20% to cover the volatility between the estimation and the transaction execution.

How Has This Been Tested?

The current integration test for the gas estimation shouldn't be zero which is the default if the simulation was not fulfilled.

Due Diligence

  • Breaking change
  • Requires a documentation update
  • Requires a e2e/integration test update

@geekbrother geekbrother self-assigned this Jan 29, 2025
SimulationProviderUnavailable,

#[error("Simulation failed: {0}")]
SimulationFailed(String),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding this specific error will result in HTTP 500 and an alert for us.

@geekbrother geekbrother force-pushed the feat/adding_bundled_gas_estimation branch from d220e01 to 4f678c7 Compare January 29, 2025 21:25
@geekbrother geekbrother marked this pull request as ready for review January 29, 2025 21:28
@geekbrother geekbrother requested a review from jakubuid January 30, 2025 07:48
Copy link

@jakubuid jakubuid left a comment

Choose a reason for hiding this comment

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

Reading more and more Rust code is fun :D

@geekbrother geekbrother merged commit eb071ad into master Jan 30, 2025
15 checks passed
@geekbrother geekbrother deleted the feat/adding_bundled_gas_estimation branch January 30, 2025 08:00
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.

3 participants