more stuff

Author: jayantk

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

@@ -521,6 +521,11 @@ abstract contract Entropy is IEntropy, EntropyState {
             bytes memory ret;
             uint256 startingGas = gasleft();
             (success, ret) = callAddress.excessivelySafeCall(
+                // Warning: the provided gas limit below is only an *upper bound* on the gas provided to the call.
+                // If the current context has less gas the desired value, 63/64ths of the current context's gas will
+                // be provided instead. (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) * 10000,
                 256, // copy at most 256 bytes of the return value into ret.
                 abi.encodeWithSelector(
@@ -543,7 +548,9 @@ abstract contract Entropy is IEntropy, EntropyState {
                 );
                 clearRequest(provider, sequenceNumber);
             } else if (ret.length > 0 || startingGas - endingGas >= uint256(req.gasLimit10k) * 10000) {
-                // Callback reverted for some reason. Note ret.length == 0 implies out of gas.
+                // The callback reverted for some reason.
+                // If ret.length == 0, then the callback ran out of gas. In this case, ensure that the
+                // callback was provided with sufficient gas.
                 emit CallbackFailed(
                     provider,
                     req.requester,
@@ -555,7 +562,13 @@ abstract contract Entropy is IEntropy, EntropyState {
                 );
                 req.callbackStatus = EntropyStatusConstants.CALLBACK_FAILED;
             } else {
-                require(false, "provider needs to send more gas");
+                // Callback reverted by 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

target_chains/ethereum/contracts/forge-test/Entropy.t.sol

@@ -1226,7 +1226,12 @@ 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);
@@ -1246,98 +1251,15 @@ 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();
+        vm.expectRevert(EntropyErrors.InsufficientGas.selector);
         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.status,
-            EntropyConstants.STATUS_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.status,
-            EntropyConstants.STATUS_CALLBACK_FAILED
-        );
-
-        // Calling without a gas limit should succeed
-        vm.expectEmit(false, false, false, true, address(random));
-        emit RevealedWithCallback(
-            reqAfterFailure,
-            userRandomNumber,
-            provider1Proofs[assignedSequenceNumber],
-            random.combineRandomValues(
-                userRandomNumber,
-                provider1Proofs[assignedSequenceNumber],
-                0
-            )
-        );
-        random.revealWithCallback(
-            provider1,
-            assignedSequenceNumber,
-            userRandomNumber,
-            provider1Proofs[assignedSequenceNumber]
-        );
-
-        // Verify request is cleared after successful reveal
-        EntropyStructs.Request memory reqAfterReveal = random.getRequest(
-            provider1,
-            assignedSequenceNumber
-        );
-        assertEq(reqAfterReveal.sequenceNumber, 0);
-        */
     }
 
     function testLastRevealedTooOld() public {

target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol

@@ -43,4 +43,6 @@ 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();
 }