address PR comments

Author: jayantk

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,
@@ -267,7 +271,7 @@ abstract contract Entropy is IEntropy, EntropyState {
             //    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 = roundGas(
+            req.gasLimit10k = roundTo10kGas(
                 callbackGasLimit < providerInfo.defaultGasLimit
                     ? providerInfo.defaultGasLimit
                     : callbackGasLimit
@@ -546,7 +550,7 @@ abstract contract Entropy is IEntropy, EntropyState {
                 // 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) * 10000,
+                uint256(req.gasLimit10k) * TEN_THOUSAND,
                 256, // copy at most 256 bytes of the return value into ret.
                 abi.encodeWithSelector(
                     IEntropyConsumer._entropyCallback.selector,
@@ -568,7 +572,8 @@ abstract contract Entropy is IEntropy, EntropyState {
                 clearRequest(provider, sequenceNumber);
             } else if (
                 ret.length > 0 ||
-                (startingGas * 31) / 32 > uint256(req.gasLimit10k) * 10000
+                (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.
@@ -668,7 +673,7 @@ abstract contract Entropy is IEntropy, EntropyState {
         // 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(roundGas(gasLimit)) * 10000;
+        uint32 roundedGasLimit = uint32(roundTo10kGas(gasLimit)) * TEN_THOUSAND;
         if (
             provider.defaultGasLimit > 0 &&
             roundedGasLimit > provider.defaultGasLimit
@@ -787,6 +792,10 @@ abstract contract Entropy is IEntropy, EntropyState {
             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);
@@ -810,11 +819,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) {
-        uint32 gas10k = gas / 10000;
-        if (gas10k * 10000 < gas) {
+    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);
     }
 

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

@@ -1463,6 +1463,19 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
         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 {
@@ -1528,6 +1541,17 @@ contract EntropyTest is Test, EntropyTestUtils, EntropyEvents {
             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);

target_chains/ethereum/entropy_sdk/solidity/EntropyErrors.sol

@@ -45,4 +45,6 @@ library EntropyErrors {
     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();
 }

target_chains/ethereum/entropy_sdk/solidity/EntropyStructs.sol

@@ -64,7 +64,7 @@ contract EntropyStructs {
         // Status flag for requests with callbacks. See EntropyConstants for the possible values of this flag.
         uint8 callbackStatus;
         // 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
+        // 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;
     }

target_chains/ethereum/entropy_sdk/solidity/abis/EntropyErrors.json

@@ -39,6 +39,11 @@
     "name": "LastRevealedTooOld",
     "type": "error"
   },
+  {
+    "inputs": [],
+    "name": "MaxGasLimitExceeded",
+    "type": "error"
+  },
   {
     "inputs": [],
     "name": "NoSuchProvider",