Support undefined fulfillment gas limit #481
Conversation
| retries: 1, | ||
| }); | ||
| if (estimateGasMulticall.success) { | ||
| // Adding a extra 10% because multicall consumes less gas than tryMulticall |
There was a problem hiding this comment.
I came here to suggest adding a % overhead 👍
There was a problem hiding this comment.
The only thing that worries me now is that these changes should be an improvement over the fulfillmentGasLimit magic number but I'm now adding a different magic number which might even be configurable by chain 😅
On tests against the local hardhat network even 1% was enough but I added 10% just to be more confident it will be enough on all chains.
There was a problem hiding this comment.
Yeah I very much doubt this will be a problem
…ted to the beaconIds passed to updateBeaconSetWithBeacons
No worries and thanks for checking this PR out @bdrhn9 🙏🏻 |
Closes #447
I'm basically doing the following:
fulfillmentGasLimitexist in the config2.1. estimate based on Api3ServerV1.multicall (tryMulticall estimate is usually wrong). If successful then add an extra since tryMulticall is a bit more expensive
2.2. if previous failed, estimate based on a single beacon update using dummy data (for beacon set update we add up the estimate of updating all beacon plus the beaconSet)
2.3. if both previous failed then just return a hardcoded gas limit of 2_000_000
This is still a draft because I need to fix/add some tests and do some minor cleanup/refactor