feat(entropy): Add on-chain gas limits and mark out-of-gas failures on-chain #2559
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 6 Skipped Deployments
|
|
|
||
| function testRequestWithRevertingCallback() public { | ||
| uint32 defaultGasLimit = 100000; | ||
| vm.prank(provider1); | ||
| random.setDefaultGasLimit(defaultGasLimit); |
There was a problem hiding this comment.
these tests now require the provider to opt in to the new flow by setting their gas limit
| @@ -703,6 +808,16 @@ abstract contract Entropy is IEntropy, EntropyState { | |||
| ); | |||
| } | |||
|
|
|||
| // Rounds the provided quantity of gas into units of 10k gas. | |||
| // If gas is not evenly divisible by 10k, rounds up. | |||
| function roundGas(uint32 gas) internal pure returns (uint16) { | |||
There was a problem hiding this comment.
naming doesn't imply it's dividing it by 10k. Maybe something like convertGastoGas10k?
| if (gas10k * 10000 < gas) { | ||
| gas10k += 1; | ||
| } | ||
| return SafeCast.toUint16(gas10k); |
There was a problem hiding this comment.
As a dev, I might set the gas limit to a gazillion just for testing and that would cause a revert here.
I don't think the revert error is easily parsable, so I'd rather test manually and revert with a better reason.
| } | ||
|
|
||
| uint32 oldGasLimit = provider.defaultGasLimit; | ||
| provider.defaultGasLimit = gasLimit; |
There was a problem hiding this comment.
maybe we should pass this to roundGas and check if it is within the limits and not too high.
| @@ -61,6 +63,9 @@ contract EntropyStructs { | |||
| bool useBlockhash; | |||
| // Status flag for requests with callbacks. See EntropyConstants for the possible values of this flag. | |||
| uint8 callbackStatus; | |||
| // 2 bytes of space left in this struct. | |||
| // The gasLimit in units of 10k gas. (i.e., 2 = 20k gas). We're using units of 10k in order to fit this | |||
| // field into the remaining 2 bytes of this storage slot. The dynamic range here is 10k - ~65M, which should | |||
| // Rounds the provided quantity of gas into units of 10k gas. | ||
| // If gas is not evenly divisible by 10k, rounds up. | ||
| function roundGas(uint32 gas) internal pure returns (uint16) { | ||
| uint32 gas10k = gas / 10000; |
There was a problem hiding this comment.
can we put this 10000 as a constant somewhere in this file? It's being used 4-5 times across multiple functions.
| ) { | ||
| // This calculation rounds down the fee, which means that users can get some gas in the callback for free. | ||
| // However, the value of the free gas is < 1 wei, which is insignificant. | ||
| uint128 additionalFee = ((roundedGasLimit - |
There was a problem hiding this comment.
can't this lead to an underflow? if roundedGasLimit < provider.defaultGasLimit?
Summary
Follow up to the last PR around allowing providers to mark on-chain callbacks as failed. The previous PR covers cases where the contract manually reverts. This PR covers the case where the callback runs out of gas.
This PR gives each callback a gas limit which is stored in the on-chain struct. Providers can set a default gas limit, or users can manually specify a different gas limit when making a request. We mark callbacks as failed if they revert with an out-of-gas error and the transaction had enough gas to forward -- there are some EVM nuances here that we need to be careful about. See inline comments for details.
Rationale
Explicitly marking on-chain requests as failed will make the state transitions of entropy much clearer to end users.
How has this been tested?