diff --git a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol index 589fa7eeee..8e9986715d 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol @@ -80,6 +80,10 @@ import "@pythnetwork/entropy-sdk-solidity/EntropyStatusConstants.sol"; abstract contract Entropy is IEntropy, EntropyState { using ExcessivelySafeCall for address; + uint32 public constant TEN_THOUSAND = 10000; + uint32 public constant MAX_GAS_LIMIT = + uint32(type(uint16).max) * TEN_THOUSAND; + function _initialize( address admin, uint128 pythFeeInWei, @@ -207,7 +211,8 @@ abstract contract Entropy is IEntropy, EntropyState { address provider, bytes32 userCommitment, bool useBlockhash, - bool isRequestWithCallback + bool isRequestWithCallback, + uint32 callbackGasLimit ) internal returns (EntropyStructs.Request storage req) { EntropyStructs.ProviderInfo storage providerInfo = _state.providers[ provider @@ -222,11 +227,12 @@ abstract contract Entropy is IEntropy, EntropyState { providerInfo.sequenceNumber += 1; // Check that fees were paid and increment the pyth / provider balances. - uint128 requiredFee = getFee(provider); + uint128 requiredFee = getFeeForGas(provider, callbackGasLimit); if (msg.value < requiredFee) revert EntropyErrors.InsufficientFee(); - providerInfo.accruedFeesInWei += providerInfo.feeInWei; + uint128 providerFee = getProviderFee(provider, callbackGasLimit); + providerInfo.accruedFeesInWei += providerFee; _state.accruedPythFeesInWei += (SafeCast.toUint128(msg.value) - - providerInfo.feeInWei); + providerFee); // Store the user's commitment so that we can fulfill the request later. // 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 { req.blockNumber = SafeCast.toUint64(block.number); req.useBlockhash = useBlockhash; + req.callbackStatus = isRequestWithCallback ? EntropyStatusConstants.CALLBACK_NOT_STARTED : EntropyStatusConstants.CALLBACK_NOT_NECESSARY; + if (providerInfo.defaultGasLimit == 0) { + // Provider doesn't support the new callback failure state flow (toggled by setting the gas limit field). + // Set gasLimit10k to 0 to disable. + req.gasLimit10k = 0; + } else { + // This check does two important things: + // 1. Providers have a minimum fee set for their defaultGasLimit. If users request less gas than that, + // they still pay for the full gas limit. So we may as well give them the full limit here. + // 2. If a provider has a defaultGasLimit != 0, we need to ensure that all requests have a >0 gas limit + // so that we opt-in to the new callback failure state flow. + req.gasLimit10k = roundTo10kGas( + callbackGasLimit < providerInfo.defaultGasLimit + ? providerInfo.defaultGasLimit + : callbackGasLimit + ); + } } // 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 { provider, userCommitment, useBlockHash, - false + false, + 0 ); assignedSequenceNumber = req.sequenceNumber; emit Requested(req); @@ -292,6 +316,19 @@ abstract contract Entropy is IEntropy, EntropyState { function requestWithCallback( address provider, bytes32 userRandomNumber + ) public payable override returns (uint64) { + return + requestWithCallbackAndGasLimit( + provider, + userRandomNumber, + 0 // Passing 0 will assign the request the provider's default gas limit + ); + } + + function requestWithCallbackAndGasLimit( + address provider, + bytes32 userRandomNumber, + uint32 gasLimit ) public payable override returns (uint64) { EntropyStructs.Request storage req = requestHelper( provider, @@ -300,7 +337,8 @@ abstract contract Entropy is IEntropy, EntropyState { // If we remove the blockHash from this, the provider would have no choice but to provide its committed // random number. Hence, useBlockHash is set to false. false, - true + true, + gasLimit ); emit RequestedWithCallback( @@ -310,7 +348,6 @@ abstract contract Entropy is IEntropy, EntropyState { userRandomNumber, req ); - return req.sequenceNumber; } @@ -493,16 +530,27 @@ abstract contract Entropy is IEntropy, EntropyState { address callAddress = req.requester; + // If the request has an explicit gas limit, then run the new callback failure state flow. + // // Requests that haven't been invoked yet will be invoked safely (catching reverts), and // any reverts will be reported as an event. Any failing requests move to a failure state // at which point they can be recovered. The recovery flow invokes the callback directly // (no catching errors) which allows callers to easily see the revert reason. - if (req.callbackStatus == EntropyStatusConstants.CALLBACK_NOT_STARTED) { + if ( + req.gasLimit10k != 0 && + req.callbackStatus == EntropyStatusConstants.CALLBACK_NOT_STARTED + ) { req.callbackStatus = EntropyStatusConstants.CALLBACK_IN_PROGRESS; bool success; bytes memory ret; + uint256 startingGas = gasleft(); (success, ret) = callAddress.excessivelySafeCall( - gasleft(), // TODO: providers need to be able to configure this in the future. + // Warning: the provided gas limit below is only an *upper bound* on the gas provided to the call. + // At most 63/64ths of the current context's gas will be provided to a call, which may be less + // than the indicated gas limit. (See CALL opcode docs here https://www.evm.codes/?fork=cancun#f1) + // Consequently, out-of-gas reverts need to be handled carefully to ensure that the callback + // was truly provided with a sufficient amount of gas. + uint256(req.gasLimit10k) * TEN_THOUSAND, 256, // copy at most 256 bytes of the return value into ret. abi.encodeWithSelector( IEntropyConsumer._entropyCallback.selector, @@ -522,8 +570,16 @@ abstract contract Entropy is IEntropy, EntropyState { randomNumber ); clearRequest(provider, sequenceNumber); - } else if (ret.length > 0) { - // Callback reverted for some reason that is *not* out-of-gas. + } else if ( + ret.length > 0 || + (startingGas * 31) / 32 > + uint256(req.gasLimit10k) * TEN_THOUSAND + ) { + // The callback reverted for some reason. + // If ret.length > 0, then we know the callback manually triggered a revert, so it's safe to mark it as failed. + // 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). + // In this case, ensure that the callback was provided with sufficient gas. Technically, 63/64ths of the startingGas is forwarded, + // but we're using 31/32 to introduce a margin of safety. emit CallbackFailed( provider, req.requester, @@ -535,9 +591,13 @@ abstract contract Entropy is IEntropy, EntropyState { ); req.callbackStatus = EntropyStatusConstants.CALLBACK_FAILED; } else { - // The callback ran out of gas - // TODO: this case will go away once we add provider gas limits, so we're not putting in a custom error type. - require(false, "provider needs to send more gas"); + // Callback reverted by (potentially) running out of gas, but the calling context did not have enough gas + // to run the callback. This is a corner case that can happen due to the nuances of gas passing + // in calls (see the comment on the call above). + // + // (Note that reverting here plays nicely with the estimateGas RPC method, which binary searches for + // the smallest gas value that causes the transaction to *succeed*. See https://github.com/ethereum/go-ethereum/pull/3587 ) + revert EntropyErrors.InsufficientGas(); } } else { // This case uses the checks-effects-interactions pattern to avoid reentry attacks @@ -590,7 +650,43 @@ abstract contract Entropy is IEntropy, EntropyState { function getFee( address provider ) public view override returns (uint128 feeAmount) { - return _state.providers[provider].feeInWei + _state.pythFeeInWei; + return getFeeForGas(provider, 0); + } + + function getFeeForGas( + address provider, + uint32 gasLimit + ) public view override returns (uint128 feeAmount) { + return getProviderFee(provider, gasLimit) + _state.pythFeeInWei; + } + + function getProviderFee( + address providerAddr, + uint32 gasLimit + ) internal view returns (uint128 feeAmount) { + EntropyStructs.ProviderInfo memory provider = _state.providers[ + providerAddr + ]; + + // Providers charge a minimum of their configured feeInWei for every request. + // Requests using more than the defaultGasLimit get a proportionally scaled fee. + // This approach may be somewhat simplistic, but it allows us to continue using the + // existing feeInWei parameter for the callback failure flow instead of defining new + // configuration values. + uint32 roundedGasLimit = uint32(roundTo10kGas(gasLimit)) * TEN_THOUSAND; + if ( + provider.defaultGasLimit > 0 && + roundedGasLimit > provider.defaultGasLimit + ) { + // 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 - + provider.defaultGasLimit) * provider.feeInWei) / + provider.defaultGasLimit; + return provider.feeInWei + additionalFee; + } else { + return provider.feeInWei; + } } function getPythFee() public view returns (uint128 feeAmount) { @@ -687,6 +783,24 @@ abstract contract Entropy is IEntropy, EntropyState { ); } + // Set the default gas limit for a request. + function setDefaultGasLimit(uint32 gasLimit) external override { + EntropyStructs.ProviderInfo storage provider = _state.providers[ + msg.sender + ]; + if (provider.sequenceNumber == 0) { + revert EntropyErrors.NoSuchProvider(); + } + + // Check that we can round the gas limit into the 10k gas. This reverts + // if the provided value exceeds the max. + roundTo10kGas(gasLimit); + + uint32 oldGasLimit = provider.defaultGasLimit; + provider.defaultGasLimit = gasLimit; + emit ProviderDefaultGasLimitUpdated(msg.sender, oldGasLimit, gasLimit); + } + function constructUserCommitment( bytes32 userRandomness ) public pure override returns (bytes32 userCommitment) { @@ -703,6 +817,21 @@ 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 roundTo10kGas(uint32 gas) internal pure returns (uint16) { + if (gas > MAX_GAS_LIMIT) { + revert EntropyErrors.MaxGasLimitExceeded(); + } + + uint32 gas10k = gas / TEN_THOUSAND; + if (gas10k * TEN_THOUSAND < gas) { + gas10k += 1; + } + // Note: safe cast here should never revert due to the if statement above. + return SafeCast.toUint16(gas10k); + } + // Create a unique key for an in-flight randomness request. Returns both a long key for use in the requestsOverflow // mapping and a short key for use in the requests array. function requestKey( diff --git a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol index 720fd092d0..a2638e6de1 100644 --- a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol +++ b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol @@ -805,7 +805,8 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { blockNumber: 1234, requester: user1, useBlockhash: false, - callbackStatus: EntropyStatusConstants.CALLBACK_NOT_STARTED + callbackStatus: EntropyStatusConstants.CALLBACK_NOT_STARTED, + gasLimit10k: 0 }) ); vm.roll(1234); @@ -939,9 +940,6 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { ); } - // TODO: restore this test once providers have custom gas limits, which allow us to toggle between - // the old and new failure behavior. - /* function testRequestWithCallbackAndRevealWithCallbackFailing() public { bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); @@ -961,9 +959,69 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { provider1Proofs[assignedSequenceNumber] ); } - */ + + function testRequestWithCallbackGasLimit() public { + uint32 defaultGasLimit = 100000; + vm.prank(provider1); + random.setDefaultGasLimit(defaultGasLimit); + + bytes32 userRandomNumber = bytes32(uint(42)); + uint fee = random.getFee(provider1); + EntropyConsumer consumer = new EntropyConsumer(address(random), false); + vm.deal(user1, fee); + vm.prank(user1); + uint64 assignedSequenceNumber = consumer.requestEntropy{value: fee}( + userRandomNumber + ); + EntropyStructs.Request memory req = random.getRequest( + provider1, + assignedSequenceNumber + ); + + // Verify the gas limit was set correctly + assertEq(req.gasLimit10k, 10); + + vm.expectEmit(false, false, false, true, address(random)); + emit RevealedWithCallback( + req, + userRandomNumber, + provider1Proofs[assignedSequenceNumber], + random.combineRandomValues( + userRandomNumber, + provider1Proofs[assignedSequenceNumber], + 0 + ) + ); + random.revealWithCallback( + provider1, + assignedSequenceNumber, + userRandomNumber, + provider1Proofs[assignedSequenceNumber] + ); + + assertEq(consumer.sequence(), assignedSequenceNumber); + assertEq(consumer.provider(), provider1); + assertEq( + consumer.randomness(), + random.combineRandomValues( + userRandomNumber, + provider1Proofs[assignedSequenceNumber], + 0 + ) + ); + + EntropyStructs.Request memory reqAfterReveal = random.getRequest( + provider1, + assignedSequenceNumber + ); + assertEq(reqAfterReveal.sequenceNumber, 0); + } function testRequestWithRevertingCallback() public { + uint32 defaultGasLimit = 100000; + vm.prank(provider1); + random.setDefaultGasLimit(defaultGasLimit); + bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); EntropyConsumer consumer = new EntropyConsumer(address(random), true); @@ -1056,7 +1114,9 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { } function testRequestWithCallbackUsingTooMuchGas() public { - uint64 defaultGasLimit = 100000; + uint32 defaultGasLimit = 100000; + vm.prank(provider1); + random.setDefaultGasLimit(defaultGasLimit); bytes32 userRandomNumber = bytes32(uint(42)); uint fee = random.getFee(provider1); @@ -1074,20 +1134,75 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { assignedSequenceNumber ); + // Verify the gas limit was set correctly + assertEq(req.gasLimit10k, 10); + // The transaction reverts if the provider does not provide enough gas to forward // the gasLimit to the callback transaction. vm.expectRevert(); - random.revealWithCallback{gas: defaultGasLimit}( + random.revealWithCallback{gas: defaultGasLimit - 10000}( + provider1, + assignedSequenceNumber, + userRandomNumber, + provider1Proofs[assignedSequenceNumber] + ); + + // If called with enough gas, the transaction should succeed, but the callback should fail because + // it uses too much gas. + vm.expectEmit(false, false, false, true, address(random)); + emit CallbackFailed( + provider1, + address(consumer), + assignedSequenceNumber, + userRandomNumber, + provider1Proofs[assignedSequenceNumber], + random.combineRandomValues( + userRandomNumber, + provider1Proofs[assignedSequenceNumber], + 0 + ), + // out-of-gas reverts have an empty bytes array as the return value. + "" + ); + random.revealWithCallback( + provider1, + assignedSequenceNumber, + userRandomNumber, + provider1Proofs[assignedSequenceNumber] + ); + + // Verify request is still active after failure + EntropyStructs.Request memory reqAfterFailure = random.getRequest( + provider1, + assignedSequenceNumber + ); + assertEq(reqAfterFailure.sequenceNumber, assignedSequenceNumber); + assertEq( + reqAfterFailure.callbackStatus, + EntropyStatusConstants.CALLBACK_FAILED + ); + + // A subsequent attempt passing insufficient gas should also revert + vm.expectRevert(); + random.revealWithCallback{gas: defaultGasLimit - 10000}( provider1, assignedSequenceNumber, userRandomNumber, provider1Proofs[assignedSequenceNumber] ); + // Again, request stays active after failure + reqAfterFailure = random.getRequest(provider1, assignedSequenceNumber); + assertEq(reqAfterFailure.sequenceNumber, assignedSequenceNumber); + assertEq( + reqAfterFailure.callbackStatus, + EntropyStatusConstants.CALLBACK_FAILED + ); + // Calling without a gas limit should succeed vm.expectEmit(false, false, false, true, address(random)); emit RevealedWithCallback( - req, + reqAfterFailure, userRandomNumber, provider1Proofs[assignedSequenceNumber], random.combineRandomValues( @@ -1111,6 +1226,38 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { assertEq(reqAfterReveal.sequenceNumber, 0); } + // Test the corner case caused by the CALL opcode passing at most 63/64ths of the current gas + // to the sub-call. + function testRequestWithCallbackUsingTooMuchGas2() public { + // With a 64M gas limit, we will pass ~63M gas to the callback (which is insufficient), but still + // have ~1M gas to execute code within the revealWithCallback method, which should be enough to + // run all of the logic subsequent to the excessivelySafeCall. + uint32 defaultGasLimit = 64000000; + vm.prank(provider1); + random.setDefaultGasLimit(defaultGasLimit); + + bytes32 userRandomNumber = bytes32(uint(42)); + uint fee = random.getFee(provider1); + EntropyConsumer consumer = new EntropyConsumer(address(random), false); + consumer.setTargetGasUsage(defaultGasLimit); + + vm.deal(user1, fee); + vm.prank(user1); + uint64 assignedSequenceNumber = consumer.requestEntropy{value: fee}( + userRandomNumber + ); + + // The transaction reverts if the provider does not provide enough gas to forward + // the gasLimit to the callback transaction. + vm.expectRevert(EntropyErrors.InsufficientGas.selector); + random.revealWithCallback{gas: defaultGasLimit - 10000}( + provider1, + assignedSequenceNumber, + userRandomNumber, + provider1Proofs[assignedSequenceNumber] + ); + } + function testLastRevealedTooOld() public { for (uint256 i = 0; i < provider1MaxNumHashes; i++) { request(user1, provider1, 42, false); @@ -1299,15 +1446,282 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents { vm.expectRevert(); random.setProviderFeeAsFeeManager(provider1, 1000); } + + function testSetDefaultGasLimit() public { + uint32 newGasLimit = 100000; + + vm.prank(provider1); + vm.expectEmit(false, false, false, true, address(random)); + emit ProviderDefaultGasLimitUpdated(provider1, 0, newGasLimit); + random.setDefaultGasLimit(newGasLimit); + + EntropyStructs.ProviderInfo memory info = random.getProviderInfo( + provider1 + ); + assertEq(info.defaultGasLimit, newGasLimit); + + // Can reset back to 0. + vm.prank(provider1); + random.setDefaultGasLimit(0); + info = random.getProviderInfo(provider1); + assertEq(info.defaultGasLimit, 0); + + // Can set to maximum value. + uint32 maxLimit = random.MAX_GAS_LIMIT(); + vm.prank(provider1); + random.setDefaultGasLimit(maxLimit); + info = random.getProviderInfo(provider1); + assertEq(info.defaultGasLimit, random.MAX_GAS_LIMIT()); + + // Reverts if max value is exceeded + uint32 exceedsGasLimit = random.MAX_GAS_LIMIT() + 1; + vm.prank(provider1); + vm.expectRevert(EntropyErrors.MaxGasLimitExceeded.selector); + random.setDefaultGasLimit(exceedsGasLimit); + } + + function testSetDefaultGasLimitRevertIfNotFromProvider() public { + vm.expectRevert(EntropyErrors.NoSuchProvider.selector); + random.setDefaultGasLimit(100000); + } + + function testRequestWithCallbackUsesDefaultGasLimit() public { + uint32 defaultGasLimit = 100000; + vm.prank(provider1); + random.setDefaultGasLimit(defaultGasLimit); + + bytes32 userRandomNumber = bytes32(uint(42)); + uint fee = random.getFee(provider1); + + vm.deal(user1, fee); + vm.prank(user1); + uint64 sequenceNumber = random.requestWithCallback{value: fee}( + provider1, + userRandomNumber + ); + + EntropyStructs.Request memory req = random.getRequest( + provider1, + sequenceNumber + ); + assertEq(req.gasLimit10k, 10); + } + + function testGasLimitsAndFeeRounding() public { + vm.prank(provider1); + random.setDefaultGasLimit(20000); + vm.prank(provider1); + random.setProviderFee(1); + + // Test exact multiples of 10,000 + assertGasLimitAndFee(0, 2, 1); + assertGasLimitAndFee(10000, 2, 1); + assertGasLimitAndFee(20000, 2, 1); + assertGasLimitAndFee(100000, 10, 5); + + // Test values just below multiples of 10,000 + assertGasLimitAndFee(9999, 2, 1); + assertGasLimitAndFee(19999, 2, 1); + assertGasLimitAndFee(39999, 4, 2); + assertGasLimitAndFee(99999, 10, 5); + + // Test values just above multiples of 10,000 + assertGasLimitAndFee(10001, 2, 1); + assertGasLimitAndFee(20001, 3, 1); + assertGasLimitAndFee(100001, 11, 5); + assertGasLimitAndFee(110001, 12, 6); + + // Test middle values + assertGasLimitAndFee(5000, 2, 1); + assertGasLimitAndFee(15000, 2, 1); + assertGasLimitAndFee(25000, 3, 1); + + // Test maximum value + assertGasLimitAndFee( + uint32(type(uint16).max) * 10000, + type(uint16).max, + uint128(type(uint16).max) / 2 + ); + + // Test larger than max value reverts with expected error + uint32 exceedsGasLimit = uint32(type(uint16).max) * 10000 + 1; + vm.expectRevert(EntropyErrors.MaxGasLimitExceeded.selector); + random.getFeeForGas(provider1, exceedsGasLimit); + vm.expectRevert(EntropyErrors.MaxGasLimitExceeded.selector); + random.requestWithCallbackAndGasLimit{value: 10000000000000}( + provider1, + bytes32(uint(42)), + exceedsGasLimit + ); + + // A provider with a 0 gas limit is opted-out of the failure state flow, indicated by + // a 0 gas limit on all requests. + vm.prank(provider1); + random.setDefaultGasLimit(0); + + assertGasLimitAndFee(0, 0, 1); + assertGasLimitAndFee(10000, 0, 1); + assertGasLimitAndFee(20000, 0, 1); + assertGasLimitAndFee(100000, 0, 1); + } + + // Helper method to create a request with a specific gas limit and check the gasLimit10k / provider fees + function assertGasLimitAndFee( + uint32 gasLimit, + uint16 expectedGasLimit10k, + uint128 expectedProviderFee + ) internal { + // Create a request with callback + bytes32 userRandomNumber = bytes32(uint(42)); + uint fee = random.getFeeForGas(provider1, gasLimit); + assertEq(fee - random.getPythFee(), expectedProviderFee); + + // Passing 1 wei less than the expected fee causes a revert. + vm.deal(user1, fee); + vm.prank(user1); + vm.expectRevert(EntropyErrors.InsufficientFee.selector); + random.requestWithCallbackAndGasLimit{value: fee - 1}( + provider1, + userRandomNumber, + gasLimit + ); + + uint128 startingAccruedProviderFee = random + .getProviderInfo(provider1) + .accruedFeesInWei; + vm.prank(user1); + uint64 sequenceNumber = random.requestWithCallbackAndGasLimit{ + value: fee + }(provider1, userRandomNumber, gasLimit); + + assertEq( + random.getProviderInfo(provider1).accruedFeesInWei - + startingAccruedProviderFee, + expectedProviderFee + ); + + // Check the gasLimit10k field in the request + EntropyStructs.Request memory req = random.getRequest( + provider1, + sequenceNumber + ); + assertEq(req.gasLimit10k, expectedGasLimit10k); + } + + function testCallbackProvidedGas() public { + vm.prank(provider1); + random.setDefaultGasLimit(200000); + + assertCallbackResult(0, 190000, true); + assertCallbackResult(0, 210000, false); + assertCallbackResult(300000, 290000, true); + assertCallbackResult(300000, 310000, false); + + // A provider that hasn't upgraded to the callback failure flow + // can never cause a callback to fail because it runs out of gas. + vm.prank(provider1); + random.setDefaultGasLimit(0); + + assertCallbackResult(0, 190000, true); + assertCallbackResult(0, 210000, true); + assertCallbackResult(300000, 290000, true); + assertCallbackResult(300000, 310000, true); + } + + // Helper method to assert whether a request with a specific gas limit / a callback with a specific gas cost + // should be successful or not. + function assertCallbackResult( + uint32 gasLimit, + uint32 callbackGasUsage, + bool expectSuccess + ) internal { + // Create a request with callback + bytes32 userRandomNumber = bytes32(uint(42)); + uint fee = random.getFeeForGas(provider1, gasLimit); + + vm.deal(user1, fee); + vm.prank(user1); + EntropyConsumer consumer = new EntropyConsumer(address(random), false); + uint64 sequenceNumber = consumer.requestEntropyWithGasLimit{value: fee}( + userRandomNumber, + gasLimit + ); + + consumer.setTargetGasUsage(callbackGasUsage); + + EntropyStructs.Request memory req = random.getRequest( + provider1, + sequenceNumber + ); + + if (!expectSuccess) { + vm.expectEmit(false, false, false, true, address(random)); + emit CallbackFailed( + provider1, + address(consumer), + sequenceNumber, + userRandomNumber, + provider1Proofs[sequenceNumber], + random.combineRandomValues( + userRandomNumber, + provider1Proofs[sequenceNumber], + 0 + ), + // out-of-gas reverts have an empty bytes array as the return value. + "" + ); + random.revealWithCallback( + provider1, + sequenceNumber, + userRandomNumber, + provider1Proofs[sequenceNumber] + ); + + // Verify request is still active after failure + EntropyStructs.Request memory reqAfterFailure = random.getRequest( + provider1, + sequenceNumber + ); + assertEq(reqAfterFailure.sequenceNumber, sequenceNumber); + assertEq( + reqAfterFailure.callbackStatus, + EntropyStatusConstants.CALLBACK_FAILED + ); + } else { + vm.expectEmit(false, false, false, true, address(random)); + emit RevealedWithCallback( + req, + userRandomNumber, + provider1Proofs[sequenceNumber], + random.combineRandomValues( + userRandomNumber, + provider1Proofs[sequenceNumber], + 0 + ) + ); + random.revealWithCallback( + provider1, + sequenceNumber, + userRandomNumber, + provider1Proofs[sequenceNumber] + ); + + // Verify request is cleared after successful callback + EntropyStructs.Request memory reqAfterSuccess = random.getRequest( + provider1, + sequenceNumber + ); + assertEq(reqAfterSuccess.sequenceNumber, 0); + } + } } contract EntropyConsumer is IEntropyConsumer { uint64 public sequence; bytes32 public randomness; - address public entropy; address public provider; + address public entropy; bool public reverts; - uint256 public gasUsed; uint256 public targetGasUsage; constructor(address _entropy, bool _reverts) { @@ -1325,6 +1739,16 @@ contract EntropyConsumer is IEntropyConsumer { }(_provider, randomNumber); } + function requestEntropyWithGasLimit( + bytes32 randomNumber, + uint32 gasLimit + ) public payable returns (uint64 sequenceNumber) { + address _provider = IEntropy(entropy).getDefaultProvider(); + sequenceNumber = IEntropy(entropy).requestWithCallbackAndGasLimit{ + value: msg.value + }(_provider, randomNumber, gasLimit); + } + function getEntropy() internal view override returns (address) { return entropy; } @@ -1334,6 +1758,10 @@ contract EntropyConsumer is IEntropyConsumer { } function setTargetGasUsage(uint256 _targetGasUsage) public { + require( + _targetGasUsage > 60000, + "Target gas usage cannot be below 60k (~the cost of storing callback results)" + ); targetGasUsage = _targetGasUsage; } @@ -1343,22 +1771,21 @@ contract EntropyConsumer is IEntropyConsumer { bytes32 _randomness ) internal override { uint256 startGas = gasleft(); - uint256 currentGasUsed = 0; + // These seemingly innocuous instructions are actually quite expensive + // (~60k gas) because they're writes to contract storage. + sequence = _sequence; + provider = _provider; + randomness = _randomness; // Keep consuming gas until we reach our target + uint256 currentGasUsed = startGas - gasleft(); while (currentGasUsed < targetGasUsage) { // Consume gas with a hash operation keccak256(abi.encodePacked(currentGasUsed, _randomness)); currentGasUsed = startGas - gasleft(); } - gasUsed = currentGasUsed; - - if (!reverts) { - sequence = _sequence; - provider = _provider; - randomness = _randomness; - } else { + if (reverts) { revert("Callback failed"); } } diff --git a/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol b/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol index 0511daae9a..fb238f8382 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol @@ -43,4 +43,8 @@ library EntropyErrors { error LastRevealedTooOld(); // A more recent commitment is already revealed on-chain error UpdateTooOld(); + // Not enough gas was provided to the function to execute the callback with the desired amount of gas. + error InsufficientGas(); + // A gas limit value was provided that was greater than the maximum possible limit of 655,350,000 + error MaxGasLimitExceeded(); } diff --git a/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol b/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol index 6587cefe41..120cb0c858 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/EntropyEvents.sol @@ -41,6 +41,12 @@ interface EntropyEvents { event ProviderFeeUpdated(address provider, uint128 oldFee, uint128 newFee); + event ProviderDefaultGasLimitUpdated( + address indexed provider, + uint32 oldDefaultGasLimit, + uint32 newDefaultGasLimit + ); + event ProviderUriUpdated(address provider, bytes oldUri, bytes newUri); event ProviderFeeManagerUpdated( diff --git a/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol b/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol index c0092ff1c6..002ab7be20 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol @@ -37,6 +37,8 @@ contract EntropyStructs { // Maximum number of hashes to record in a request. This should be set according to the maximum gas limit // the provider supports for callbacks. uint32 maxNumHashes; + // Default gas limit to use for callbacks. + uint32 defaultGasLimit; } struct Request { @@ -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 - 655M, which should + // cover all real-world use cases. + uint16 gasLimit10k; } } diff --git a/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol b/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol index ce446ff44b..9d1f13a8c6 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol +++ b/target_chains/ethereum/entropy_sdk/solidity/IEntropy.sol @@ -48,14 +48,36 @@ interface IEntropy is EntropyEvents { // // The address calling this function should be a contract that inherits from the IEntropyConsumer interface. // The `entropyCallback` method on that interface will receive a callback with the generated random number. + // `entropyCallback` will be run with the provider's default gas limit (see `getProviderInfo(provider).defaultGasLimit`). + // If your callback needs additional gas, please use `requestWithCallbackAndGasLimit`. // - // This method will revert unless the caller provides a sufficient fee (at least getFee(provider)) as msg.value. + // This method will revert unless the caller provides a sufficient fee (at least `getFee(provider)`) as msg.value. // Note that excess value is *not* refunded to the caller. function requestWithCallback( address provider, bytes32 userRandomNumber ) external payable returns (uint64 assignedSequenceNumber); + // Request a random number from `provider`, getting a callback with the result. + // The caller must specify a provider to fulfill the request -- `getDefaultProvider()` is a sane default -- + // and a `userRandomNumber` to combine into the result. The method returns a sequence number which callers + // should save to correlate the request with the callback. + // + // The address calling this function should be a contract that inherits from the IEntropyConsumer interface. + // The `entropyCallback` method on that interface will receive a callback with the returned sequence number and + // the generated random number. `entropyCallback` will be run with the `gasLimit` provided to this function. + // The `gasLimit` will be rounded up to a multiple of 10k (e.g., 19000 -> 20000), and furthermore is lower bounded + // by the provider's configured default limit. + // + // This method will revert unless the caller provides a sufficient fee (at least `getFeeForGas(provider, gasLimit)`) as msg.value. + // Note that provider fees can change over time. Thus, callers of this method should explictly compute `getFeeForGas(provider, gasLimit)` + // prior to each invocation (as opposed to hardcoding a value). Further note that excess value is *not* refunded to the caller. + function requestWithCallbackAndGasLimit( + address provider, + bytes32 userRandomNumber, + uint32 gasLimit + ) external payable returns (uint64 assignedSequenceNumber); + // Fulfill a request for a random number. This method validates the provided userRandomness and provider's proof // against the corresponding commitments in the in-flight request. If both values are validated, this function returns // the corresponding random number. @@ -98,8 +120,16 @@ interface IEntropy is EntropyEvents { uint64 sequenceNumber ) external view returns (EntropyStructs.Request memory req); + // Get the fee charged by provider for a request with the default gasLimit (`request` or `requestWithCallback`). + // If you are calling `requestWithCallbackAndGasLimit`, please use `getFeeForGas`. function getFee(address provider) external view returns (uint128 feeAmount); + // Get the fee charged by `provider` for a request with a specific `gasLimit` (`requestWithCallbackAndGasLimit`). + function getFeeForGas( + address provider, + uint32 gasLimit + ) external view returns (uint128 feeAmount); + function getAccruedPythFees() external view @@ -124,6 +154,9 @@ interface IEntropy is EntropyEvents { // the provider supports for callbacks. function setMaxNumHashes(uint32 maxNumHashes) external; + // Set the default gas limit for a request. If 0, no + function setDefaultGasLimit(uint32 gasLimit) external; + // Advance the provider commitment and increase the sequence number. // This is used to reduce the `numHashes` required for future requests which leads to reduced gas usage. function advanceProviderCommitment( diff --git a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json index 24a6ef2548..f5a5faf248 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json +++ b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json @@ -19,6 +19,11 @@ "name": "InsufficientFee", "type": "error" }, + { + "inputs": [], + "name": "InsufficientGas", + "type": "error" + }, { "inputs": [], "name": "InvalidRevealCall", @@ -34,6 +39,11 @@ "name": "LastRevealedTooOld", "type": "error" }, + { + "inputs": [], + "name": "MaxGasLimitExceeded", + "type": "error" + }, { "inputs": [], "name": "NoSuchProvider", diff --git a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json index 164c3d4b19..f257fccf35 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json +++ b/target_chains/ethereum/entropy_sdk/solidity/abis/EntropyEvents.json @@ -48,6 +48,31 @@ "name": "CallbackFailed", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "oldDefaultGasLimit", + "type": "uint32" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "newDefaultGasLimit", + "type": "uint32" + } + ], + "name": "ProviderDefaultGasLimitUpdated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -212,6 +237,11 @@ "internalType": "uint32", "name": "maxNumHashes", "type": "uint32" + }, + { + "internalType": "uint32", + "name": "defaultGasLimit", + "type": "uint32" } ], "indexed": false, @@ -267,6 +297,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -346,6 +381,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -401,6 +441,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -480,6 +525,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, diff --git a/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json b/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json index 1c6de9d2d3..3012caf605 100644 --- a/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json +++ b/target_chains/ethereum/entropy_sdk/solidity/abis/IEntropy.json @@ -48,6 +48,31 @@ "name": "CallbackFailed", "type": "event" }, + { + "anonymous": false, + "inputs": [ + { + "indexed": true, + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "oldDefaultGasLimit", + "type": "uint32" + }, + { + "indexed": false, + "internalType": "uint32", + "name": "newDefaultGasLimit", + "type": "uint32" + } + ], + "name": "ProviderDefaultGasLimitUpdated", + "type": "event" + }, { "anonymous": false, "inputs": [ @@ -212,6 +237,11 @@ "internalType": "uint32", "name": "maxNumHashes", "type": "uint32" + }, + { + "internalType": "uint32", + "name": "defaultGasLimit", + "type": "uint32" } ], "indexed": false, @@ -267,6 +297,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -346,6 +381,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -401,6 +441,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -480,6 +525,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "indexed": false, @@ -650,6 +700,30 @@ "stateMutability": "view", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "internalType": "uint32", + "name": "gasLimit", + "type": "uint32" + } + ], + "name": "getFeeForGas", + "outputs": [ + { + "internalType": "uint128", + "name": "feeAmount", + "type": "uint128" + } + ], + "stateMutability": "view", + "type": "function" + }, { "inputs": [ { @@ -721,6 +795,11 @@ "internalType": "uint32", "name": "maxNumHashes", "type": "uint32" + }, + { + "internalType": "uint32", + "name": "defaultGasLimit", + "type": "uint32" } ], "internalType": "struct EntropyStructs.ProviderInfo", @@ -787,6 +866,11 @@ "internalType": "uint8", "name": "callbackStatus", "type": "uint8" + }, + { + "internalType": "uint16", + "name": "gasLimit10k", + "type": "uint16" } ], "internalType": "struct EntropyStructs.Request", @@ -883,6 +967,35 @@ "stateMutability": "payable", "type": "function" }, + { + "inputs": [ + { + "internalType": "address", + "name": "provider", + "type": "address" + }, + { + "internalType": "bytes32", + "name": "userRandomNumber", + "type": "bytes32" + }, + { + "internalType": "uint32", + "name": "gasLimit", + "type": "uint32" + } + ], + "name": "requestWithCallbackAndGasLimit", + "outputs": [ + { + "internalType": "uint64", + "name": "assignedSequenceNumber", + "type": "uint64" + } + ], + "stateMutability": "payable", + "type": "function" + }, { "inputs": [ { @@ -945,6 +1058,19 @@ "stateMutability": "nonpayable", "type": "function" }, + { + "inputs": [ + { + "internalType": "uint32", + "name": "gasLimit", + "type": "uint32" + } + ], + "name": "setDefaultGasLimit", + "outputs": [], + "stateMutability": "nonpayable", + "type": "function" + }, { "inputs": [ {