From e7d5569cc2ac460d50c7580e504442714298feb5 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Thu, 8 May 2025 11:45:49 -0700 Subject: [PATCH 1/2] log gas usage --- .../contracts/contracts/entropy/Entropy.sol | 36 ++++++++++--------- .../contracts/forge-test/Entropy.t.sol | 6 ++-- 2 files changed, 21 insertions(+), 21 deletions(-) diff --git a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol index b238f0a2b6..020b1dc45a 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol @@ -669,23 +669,6 @@ abstract contract Entropy is IEntropy, EntropyState { } } else { // This case uses the checks-effects-interactions pattern to avoid reentry attacks - emit RevealedWithCallback( - EntropyStructConverter.toV1Request(req), - userRandomNumber, - providerRevelation, - randomNumber - ); - emit EntropyEventsV2.Revealed( - provider, - req.requester, - req.sequenceNumber, - randomNumber, - false, - bytes(""), - 0, // gas usage not tracked in the old no-gas-limit flow. - bytes("") - ); - clearRequest(provider, sequenceNumber); // Check if the requester is a contract account. @@ -694,6 +677,7 @@ abstract contract Entropy is IEntropy, EntropyState { assembly { len := extcodesize(callAddress) } + uint256 startingGas = gasleft(); if (len != 0) { IEntropyConsumer(callAddress)._entropyCallback( sequenceNumber, @@ -701,6 +685,24 @@ abstract contract Entropy is IEntropy, EntropyState { randomNumber ); } + uint32 gasUsed = SafeCast.toUint32(startingGas - gasleft()); + + emit RevealedWithCallback( + EntropyStructConverter.toV1Request(req), + userRandomNumber, + providerRevelation, + randomNumber + ); + emit EntropyEventsV2.Revealed( + provider, + req.requester, + req.sequenceNumber, + randomNumber, + false, + bytes(""), + gasUsed, + bytes("") + ); } } diff --git a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol index db7026372c..9a5d88efd3 100644 --- a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol +++ b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol @@ -1866,12 +1866,10 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents, EntropyEventsV2 { // callback gas usage is approximate and only triggered when the provider has set a gas limit. // Note: this condition is somewhat janky, but we hit the stack limit so can't put in any more local variables :( - assertTrue( - random.getProviderInfoV2(provider1).defaultGasLimit == 0 || + assertTrue( ((callbackGasUsage * 90) / 100 < callbackGasUsed) ); - assertTrue( - random.getProviderInfoV2(provider1).defaultGasLimit == 0 || + assertTrue( (callbackGasUsed < (callbackGasUsage * 110) / 100) ); assertEq(extraArgs, bytes("")); From 31d3ba6c582156d637d07237d7693dbdd6e866f7 Mon Sep 17 00:00:00 2001 From: Jayant Krishnamurthy Date: Thu, 8 May 2025 12:29:07 -0700 Subject: [PATCH 2/2] track gas usage in the recovery flow --- .../contracts/contracts/entropy/Entropy.sol | 13 +++++++---- .../contracts/forge-test/Entropy.t.sol | 23 +++++++------------ 2 files changed, 16 insertions(+), 20 deletions(-) diff --git a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol index 020b1dc45a..222fef8648 100644 --- a/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol +++ b/target_chains/ethereum/contracts/contracts/entropy/Entropy.sol @@ -669,15 +669,18 @@ abstract contract Entropy is IEntropy, EntropyState { } } else { // This case uses the checks-effects-interactions pattern to avoid reentry attacks + address callAddress = req.requester; + EntropyStructs.Request memory reqV1 = EntropyStructConverter + .toV1Request(req); clearRequest(provider, sequenceNumber); + // WARNING: DO NOT USE req BELOW HERE AS ITS CONTENTS HAS BEEN CLEARED // Check if the requester is a contract account. uint len; - address callAddress = req.requester; assembly { len := extcodesize(callAddress) } - uint256 startingGas = gasleft(); + uint256 startingGas = gasleft(); if (len != 0) { IEntropyConsumer(callAddress)._entropyCallback( sequenceNumber, @@ -688,15 +691,15 @@ abstract contract Entropy is IEntropy, EntropyState { uint32 gasUsed = SafeCast.toUint32(startingGas - gasleft()); emit RevealedWithCallback( - EntropyStructConverter.toV1Request(req), + reqV1, userRandomNumber, providerRevelation, randomNumber ); emit EntropyEventsV2.Revealed( provider, - req.requester, - req.sequenceNumber, + callAddress, + sequenceNumber, randomNumber, false, bytes(""), diff --git a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol index 9a5d88efd3..b9732f5b2c 100644 --- a/target_chains/ethereum/contracts/forge-test/Entropy.t.sol +++ b/target_chains/ethereum/contracts/forge-test/Entropy.t.sol @@ -1778,7 +1778,6 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents, EntropyEventsV2 { // 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); @@ -1864,12 +1863,13 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents, EntropyEventsV2 { assertEq(callbackFailed, true); assertEq(callbackErrorCode, bytes("")); - // callback gas usage is approximate and only triggered when the provider has set a gas limit. - // Note: this condition is somewhat janky, but we hit the stack limit so can't put in any more local variables :( - assertTrue( + // callback gas usage is approximate + assertTrue( + random.getProviderInfoV2(provider1).defaultGasLimit == 0 || ((callbackGasUsage * 90) / 100 < callbackGasUsed) ); - assertTrue( + assertTrue( + random.getProviderInfoV2(provider1).defaultGasLimit == 0 || (callbackGasUsed < (callbackGasUsage * 110) / 100) ); assertEq(extraArgs, bytes("")); @@ -1939,16 +1939,9 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents, EntropyEventsV2 { ); assertEq(callbackFailed, false); assertEq(callbackErrorCode, bytes("")); - // callback gas usage is approximate and only triggered when the provider has set a gas limit - // Note: this condition is somewhat janky, but we hit the stack limit so can't put in any more local variables :( - assertTrue( - random.getProviderInfoV2(provider1).defaultGasLimit == 0 || - ((callbackGasUsage * 90) / 100 < callbackGasUsed) - ); - assertTrue( - random.getProviderInfoV2(provider1).defaultGasLimit == 0 || - (callbackGasUsed < (callbackGasUsage * 110) / 100) - ); + // callback gas usage is approximate + assertTrue((callbackGasUsage * 90) / 100 < callbackGasUsed); + assertTrue(callbackGasUsed < (callbackGasUsage * 110) / 100); assertEq(extraArgs, bytes("")); // Verify request is cleared after successful callback