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

Estimate gas for RRP fulfillments #1693

Merged
merged 7 commits into from
Mar 29, 2023
Merged

Conversation

bdrhn9
Copy link
Contributor

@bdrhn9 bdrhn9 commented Mar 23, 2023

Closes #1589.

Since the issue was described well, I have implemented what it says. With unit-tests, covered all scenarios that can occur. Whole workflow is following:

  • If fulfillmentGasLimit is defined in config.json,
    • It works like this PR wasn't added
  • If fulfillmentGasLimit isn't defined in config.json, try to estimate gas,
    • If estimation was successful, submit fulfillment.
    • If estimation wasn’t successful, try to call statically to get revert string, if static call was successful unexpectedly, submit fail along with the gas estimation error. This scenario is very unlikely and probably it’ll occur because of RPC issues.
    • If estimation wasn’t successful, try to call statically to get revert string, if static call wasn’t successful as expected, submit fail along with revert string

Additionally, I tested manually by deploying on AWS without specifying fulfillmentGasLimit parameter in config.json. Tested on following chains:

  • ethereum-goerli-testnet
  • ethereum-sepolia-testnet
  • rsk-testnet
  • bsc-testnet
  • optimism-testnet
  • moonbeam-testnet
  • fantom-testnet
  • avalanche-testnet
  • polygon-testnet
  • milkomeda-testnet
  • arbitrum-testnet

@bdrhn9 bdrhn9 self-assigned this Mar 23, 2023
Copy link
Contributor

@dcroote dcroote left a comment

Choose a reason for hiding this comment

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

I tested locally with hardhat and the api3/airnode-client-dev:b78233ac011b696ae8b9a84942f69b6bd79bd6a9 image and it worked 👍

packages/airnode-node/src/evm/fulfillments/api-calls.ts Outdated Show resolved Hide resolved
@bdrhn9 bdrhn9 requested a review from Siegrift March 29, 2023 13:24
@bdrhn9 bdrhn9 requested a review from Siegrift March 29, 2023 16:01
Copy link
Contributor

@Siegrift Siegrift left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@bdrhn9 bdrhn9 merged commit 1e24c89 into master Mar 29, 2023
@bdrhn9 bdrhn9 deleted the estimate-gas-for-fulfillments branch March 29, 2023 18:42
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.

Have RRP fulfillments estimate gas
3 participants