Skip to content

Commit 9873e61

Browse files
authored
feat(entropy): Add on-chain gas limits and mark out-of-gas failures on-chain (#2559)
* moving stuff around * getting tests in place * fixing stuff * fixing stuff * more stuff * rebase * cleaning up * cleaning up possible error case * improve tests * cleanup * cleanup * cleanup * final cleanup * address PR comments * test emit event
1 parent 871e7e8 commit 9873e61

File tree

9 files changed

+825
-35
lines changed

9 files changed

+825
-35
lines changed

target_chains/ethereum/contracts/contracts/entropy/Entropy.sol

+144-15
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,10 @@ import "@pythnetwork/entropy-sdk-solidity/EntropyStatusConstants.sol";
8080
abstract contract Entropy is IEntropy, EntropyState {
8181
using ExcessivelySafeCall for address;
8282

83+
uint32 public constant TEN_THOUSAND = 10000;
84+
uint32 public constant MAX_GAS_LIMIT =
85+
uint32(type(uint16).max) * TEN_THOUSAND;
86+
8387
function _initialize(
8488
address admin,
8589
uint128 pythFeeInWei,
@@ -207,7 +211,8 @@ abstract contract Entropy is IEntropy, EntropyState {
207211
address provider,
208212
bytes32 userCommitment,
209213
bool useBlockhash,
210-
bool isRequestWithCallback
214+
bool isRequestWithCallback,
215+
uint32 callbackGasLimit
211216
) internal returns (EntropyStructs.Request storage req) {
212217
EntropyStructs.ProviderInfo storage providerInfo = _state.providers[
213218
provider
@@ -222,11 +227,12 @@ abstract contract Entropy is IEntropy, EntropyState {
222227
providerInfo.sequenceNumber += 1;
223228

224229
// Check that fees were paid and increment the pyth / provider balances.
225-
uint128 requiredFee = getFee(provider);
230+
uint128 requiredFee = getFeeForGas(provider, callbackGasLimit);
226231
if (msg.value < requiredFee) revert EntropyErrors.InsufficientFee();
227-
providerInfo.accruedFeesInWei += providerInfo.feeInWei;
232+
uint128 providerFee = getProviderFee(provider, callbackGasLimit);
233+
providerInfo.accruedFeesInWei += providerFee;
228234
_state.accruedPythFeesInWei += (SafeCast.toUint128(msg.value) -
229-
providerInfo.feeInWei);
235+
providerFee);
230236

231237
// Store the user's commitment so that we can fulfill the request later.
232238
// Warning: this code needs to overwrite *every* field in the request, because the returned request can be
@@ -251,9 +257,26 @@ abstract contract Entropy is IEntropy, EntropyState {
251257

252258
req.blockNumber = SafeCast.toUint64(block.number);
253259
req.useBlockhash = useBlockhash;
260+
254261
req.callbackStatus = isRequestWithCallback
255262
? EntropyStatusConstants.CALLBACK_NOT_STARTED
256263
: EntropyStatusConstants.CALLBACK_NOT_NECESSARY;
264+
if (providerInfo.defaultGasLimit == 0) {
265+
// Provider doesn't support the new callback failure state flow (toggled by setting the gas limit field).
266+
// Set gasLimit10k to 0 to disable.
267+
req.gasLimit10k = 0;
268+
} else {
269+
// This check does two important things:
270+
// 1. Providers have a minimum fee set for their defaultGasLimit. If users request less gas than that,
271+
// they still pay for the full gas limit. So we may as well give them the full limit here.
272+
// 2. If a provider has a defaultGasLimit != 0, we need to ensure that all requests have a >0 gas limit
273+
// so that we opt-in to the new callback failure state flow.
274+
req.gasLimit10k = roundTo10kGas(
275+
callbackGasLimit < providerInfo.defaultGasLimit
276+
? providerInfo.defaultGasLimit
277+
: callbackGasLimit
278+
);
279+
}
257280
}
258281

259282
// As a user, request a random number from `provider`. Prior to calling this method, the user should
@@ -275,7 +298,8 @@ abstract contract Entropy is IEntropy, EntropyState {
275298
provider,
276299
userCommitment,
277300
useBlockHash,
278-
false
301+
false,
302+
0
279303
);
280304
assignedSequenceNumber = req.sequenceNumber;
281305
emit Requested(req);
@@ -292,6 +316,19 @@ abstract contract Entropy is IEntropy, EntropyState {
292316
function requestWithCallback(
293317
address provider,
294318
bytes32 userRandomNumber
319+
) public payable override returns (uint64) {
320+
return
321+
requestWithCallbackAndGasLimit(
322+
provider,
323+
userRandomNumber,
324+
0 // Passing 0 will assign the request the provider's default gas limit
325+
);
326+
}
327+
328+
function requestWithCallbackAndGasLimit(
329+
address provider,
330+
bytes32 userRandomNumber,
331+
uint32 gasLimit
295332
) public payable override returns (uint64) {
296333
EntropyStructs.Request storage req = requestHelper(
297334
provider,
@@ -300,7 +337,8 @@ abstract contract Entropy is IEntropy, EntropyState {
300337
// If we remove the blockHash from this, the provider would have no choice but to provide its committed
301338
// random number. Hence, useBlockHash is set to false.
302339
false,
303-
true
340+
true,
341+
gasLimit
304342
);
305343

306344
emit RequestedWithCallback(
@@ -310,7 +348,6 @@ abstract contract Entropy is IEntropy, EntropyState {
310348
userRandomNumber,
311349
req
312350
);
313-
314351
return req.sequenceNumber;
315352
}
316353

@@ -493,16 +530,27 @@ abstract contract Entropy is IEntropy, EntropyState {
493530

494531
address callAddress = req.requester;
495532

533+
// If the request has an explicit gas limit, then run the new callback failure state flow.
534+
//
496535
// Requests that haven't been invoked yet will be invoked safely (catching reverts), and
497536
// any reverts will be reported as an event. Any failing requests move to a failure state
498537
// at which point they can be recovered. The recovery flow invokes the callback directly
499538
// (no catching errors) which allows callers to easily see the revert reason.
500-
if (req.callbackStatus == EntropyStatusConstants.CALLBACK_NOT_STARTED) {
539+
if (
540+
req.gasLimit10k != 0 &&
541+
req.callbackStatus == EntropyStatusConstants.CALLBACK_NOT_STARTED
542+
) {
501543
req.callbackStatus = EntropyStatusConstants.CALLBACK_IN_PROGRESS;
502544
bool success;
503545
bytes memory ret;
546+
uint256 startingGas = gasleft();
504547
(success, ret) = callAddress.excessivelySafeCall(
505-
gasleft(), // TODO: providers need to be able to configure this in the future.
548+
// Warning: the provided gas limit below is only an *upper bound* on the gas provided to the call.
549+
// At most 63/64ths of the current context's gas will be provided to a call, which may be less
550+
// than the indicated gas limit. (See CALL opcode docs here https://www.evm.codes/?fork=cancun#f1)
551+
// Consequently, out-of-gas reverts need to be handled carefully to ensure that the callback
552+
// was truly provided with a sufficient amount of gas.
553+
uint256(req.gasLimit10k) * TEN_THOUSAND,
506554
256, // copy at most 256 bytes of the return value into ret.
507555
abi.encodeWithSelector(
508556
IEntropyConsumer._entropyCallback.selector,
@@ -522,8 +570,16 @@ abstract contract Entropy is IEntropy, EntropyState {
522570
randomNumber
523571
);
524572
clearRequest(provider, sequenceNumber);
525-
} else if (ret.length > 0) {
526-
// Callback reverted for some reason that is *not* out-of-gas.
573+
} else if (
574+
ret.length > 0 ||
575+
(startingGas * 31) / 32 >
576+
uint256(req.gasLimit10k) * TEN_THOUSAND
577+
) {
578+
// The callback reverted for some reason.
579+
// If ret.length > 0, then we know the callback manually triggered a revert, so it's safe to mark it as failed.
580+
// If ret.length == 0, then the callback might have run out of gas (though there are other ways to trigger a revert with ret.length == 0).
581+
// In this case, ensure that the callback was provided with sufficient gas. Technically, 63/64ths of the startingGas is forwarded,
582+
// but we're using 31/32 to introduce a margin of safety.
527583
emit CallbackFailed(
528584
provider,
529585
req.requester,
@@ -535,9 +591,13 @@ abstract contract Entropy is IEntropy, EntropyState {
535591
);
536592
req.callbackStatus = EntropyStatusConstants.CALLBACK_FAILED;
537593
} else {
538-
// The callback ran out of gas
539-
// TODO: this case will go away once we add provider gas limits, so we're not putting in a custom error type.
540-
require(false, "provider needs to send more gas");
594+
// Callback reverted by (potentially) running out of gas, but the calling context did not have enough gas
595+
// to run the callback. This is a corner case that can happen due to the nuances of gas passing
596+
// in calls (see the comment on the call above).
597+
//
598+
// (Note that reverting here plays nicely with the estimateGas RPC method, which binary searches for
599+
// the smallest gas value that causes the transaction to *succeed*. See https://github.com/ethereum/go-ethereum/pull/3587 )
600+
revert EntropyErrors.InsufficientGas();
541601
}
542602
} else {
543603
// This case uses the checks-effects-interactions pattern to avoid reentry attacks
@@ -590,7 +650,43 @@ abstract contract Entropy is IEntropy, EntropyState {
590650
function getFee(
591651
address provider
592652
) public view override returns (uint128 feeAmount) {
593-
return _state.providers[provider].feeInWei + _state.pythFeeInWei;
653+
return getFeeForGas(provider, 0);
654+
}
655+
656+
function getFeeForGas(
657+
address provider,
658+
uint32 gasLimit
659+
) public view override returns (uint128 feeAmount) {
660+
return getProviderFee(provider, gasLimit) + _state.pythFeeInWei;
661+
}
662+
663+
function getProviderFee(
664+
address providerAddr,
665+
uint32 gasLimit
666+
) internal view returns (uint128 feeAmount) {
667+
EntropyStructs.ProviderInfo memory provider = _state.providers[
668+
providerAddr
669+
];
670+
671+
// Providers charge a minimum of their configured feeInWei for every request.
672+
// Requests using more than the defaultGasLimit get a proportionally scaled fee.
673+
// This approach may be somewhat simplistic, but it allows us to continue using the
674+
// existing feeInWei parameter for the callback failure flow instead of defining new
675+
// configuration values.
676+
uint32 roundedGasLimit = uint32(roundTo10kGas(gasLimit)) * TEN_THOUSAND;
677+
if (
678+
provider.defaultGasLimit > 0 &&
679+
roundedGasLimit > provider.defaultGasLimit
680+
) {
681+
// This calculation rounds down the fee, which means that users can get some gas in the callback for free.
682+
// However, the value of the free gas is < 1 wei, which is insignificant.
683+
uint128 additionalFee = ((roundedGasLimit -
684+
provider.defaultGasLimit) * provider.feeInWei) /
685+
provider.defaultGasLimit;
686+
return provider.feeInWei + additionalFee;
687+
} else {
688+
return provider.feeInWei;
689+
}
594690
}
595691

596692
function getPythFee() public view returns (uint128 feeAmount) {
@@ -687,6 +783,24 @@ abstract contract Entropy is IEntropy, EntropyState {
687783
);
688784
}
689785

786+
// Set the default gas limit for a request.
787+
function setDefaultGasLimit(uint32 gasLimit) external override {
788+
EntropyStructs.ProviderInfo storage provider = _state.providers[
789+
msg.sender
790+
];
791+
if (provider.sequenceNumber == 0) {
792+
revert EntropyErrors.NoSuchProvider();
793+
}
794+
795+
// Check that we can round the gas limit into the 10k gas. This reverts
796+
// if the provided value exceeds the max.
797+
roundTo10kGas(gasLimit);
798+
799+
uint32 oldGasLimit = provider.defaultGasLimit;
800+
provider.defaultGasLimit = gasLimit;
801+
emit ProviderDefaultGasLimitUpdated(msg.sender, oldGasLimit, gasLimit);
802+
}
803+
690804
function constructUserCommitment(
691805
bytes32 userRandomness
692806
) public pure override returns (bytes32 userCommitment) {
@@ -703,6 +817,21 @@ abstract contract Entropy is IEntropy, EntropyState {
703817
);
704818
}
705819

820+
// Rounds the provided quantity of gas into units of 10k gas.
821+
// If gas is not evenly divisible by 10k, rounds up.
822+
function roundTo10kGas(uint32 gas) internal pure returns (uint16) {
823+
if (gas > MAX_GAS_LIMIT) {
824+
revert EntropyErrors.MaxGasLimitExceeded();
825+
}
826+
827+
uint32 gas10k = gas / TEN_THOUSAND;
828+
if (gas10k * TEN_THOUSAND < gas) {
829+
gas10k += 1;
830+
}
831+
// Note: safe cast here should never revert due to the if statement above.
832+
return SafeCast.toUint16(gas10k);
833+
}
834+
706835
// Create a unique key for an in-flight randomness request. Returns both a long key for use in the requestsOverflow
707836
// mapping and a short key for use in the requests array.
708837
function requestKey(

0 commit comments

Comments
 (0)