From a494da6b51d87b481a7180c5f5f14d6eee01b207 Mon Sep 17 00:00:00 2001 From: Josh Date: Thu, 19 Dec 2024 12:32:13 -0500 Subject: [PATCH] Optimize library with better boolean logic. --- contracts/gas-snapshots/ccip.gas-snapshot | 102 +++++++++--------- .../ccip/libraries/ERC165CheckerReverting.sol | 35 +++--- ...Reverting.supportsInterfaceReverting.t.sol | 8 +- 3 files changed, 68 insertions(+), 77 deletions(-) diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot index 7e2085f4da2..aa3d5ca5284 100644 --- a/contracts/gas-snapshots/ccip.gas-snapshot +++ b/contracts/gas-snapshots/ccip.gas-snapshot @@ -28,8 +28,8 @@ CCIPHome_setCandidate:test_setCandidate() (gas: 1365438) CCIPHome_supportsInterface:test_supportsInterface() (gas: 9885) DefensiveExampleTest:test_HappyPath() (gas: 200521) DefensiveExampleTest:test_Recovery() (gas: 425013) -E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1488604) -ERC165CheckerReverting_supportsInterfaceReverting:test__supportsInterfaceReverting() (gas: 10485) +E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1488253) +ERC165CheckerReverting_supportsInterfaceReverting:test__supportsInterfaceReverting() (gas: 10445) EtherSenderReceiverTest_ccipReceive:test_ccipReceive_fallbackToWethTransfer() (gas: 96980) EtherSenderReceiverTest_ccipReceive:test_ccipReceive_happyPath() (gas: 49812) EtherSenderReceiverTest_ccipReceive:test_ccipReceive_wrongToken() (gas: 17479) @@ -196,11 +196,11 @@ NonceManager_applyPreviousRampsUpdates:test_MultipleRampsUpdates() (gas: 123593) NonceManager_applyPreviousRampsUpdates:test_PreviousRampAlreadySet_overrideAllowed() (gas: 45963) NonceManager_applyPreviousRampsUpdates:test_SingleRampUpdate() (gas: 66943) NonceManager_applyPreviousRampsUpdates:test_ZeroInput() (gas: 12191) -NonceManager_getInboundNonce:test_getInboundNonce_NoPrevOffRampForChain() (gas: 178809) -NonceManager_getInboundNonce:test_getInboundNonce_Upgraded() (gas: 145964) -NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceNewSenderStartsAtZero() (gas: 182284) -NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceStartsAtV1Nonce() (gas: 244860) -NonceManager_getInboundNonce:test_getInboundNonce_UpgradedOffRampNonceSkipsIfMsgInFlight() (gas: 213660) +NonceManager_getInboundNonce:test_getInboundNonce_NoPrevOffRampForChain() (gas: 178770) +NonceManager_getInboundNonce:test_getInboundNonce_Upgraded() (gas: 145925) +NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceNewSenderStartsAtZero() (gas: 182245) +NonceManager_getInboundNonce:test_getInboundNonce_UpgradedNonceStartsAtV1Nonce() (gas: 244782) +NonceManager_getInboundNonce:test_getInboundNonce_UpgradedOffRampNonceSkipsIfMsgInFlight() (gas: 213621) NonceManager_getInboundNonce:test_getInboundNonce_UpgradedSenderNoncesReadsPreviousRamp() (gas: 60520) NonceManager_getIncrementedOutboundNonce:test_getIncrementedOutboundNonce() (gas: 37979) NonceManager_getIncrementedOutboundNonce:test_incrementInboundNonce() (gas: 38756) @@ -216,12 +216,12 @@ OffRamp_applySourceChainConfigUpdates:test_ApplyZeroUpdates() (gas: 16720) OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChain() (gas: 181035) OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChainOnRamp() (gas: 168558) OffRamp_applySourceChainConfigUpdates:test_allowNonOnRampUpdateAfterLaneIsUsed() (gas: 284716) -OffRamp_batchExecute:test_MultipleReportsDifferentChains() (gas: 325290) -OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain() (gas: 170469) -OffRamp_batchExecute:test_MultipleReportsSameChain() (gas: 268589) -OffRamp_batchExecute:test_MultipleReportsSkipDuplicate() (gas: 161486) -OffRamp_batchExecute:test_SingleReport() (gas: 149417) -OffRamp_batchExecute:test_Unhealthy() (gas: 532472) +OffRamp_batchExecute:test_MultipleReportsDifferentChains() (gas: 325173) +OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain() (gas: 170430) +OffRamp_batchExecute:test_MultipleReportsSameChain() (gas: 268472) +OffRamp_batchExecute:test_MultipleReportsSkipDuplicate() (gas: 161447) +OffRamp_batchExecute:test_SingleReport() (gas: 149378) +OffRamp_batchExecute:test_Unhealthy() (gas: 532238) OffRamp_commit:test_OnlyGasPriceUpdates() (gas: 112719) OffRamp_commit:test_OnlyTokenPriceUpdates() (gas: 112673) OffRamp_commit:test_PriceSequenceNumberCleared() (gas: 354787) @@ -230,51 +230,51 @@ OffRamp_commit:test_ReportOnlyRootSuccess_gas() (gas: 140878) OffRamp_commit:test_RootWithRMNDisabled() (gas: 153674) OffRamp_commit:test_StaleReportWithRoot() (gas: 231732) OffRamp_commit:test_ValidPriceUpdateThenStaleReportWithRoot() (gas: 206199) -OffRamp_constructor:test_Constructor() (gas: 6214884) -OffRamp_execute:test_LargeBatch() (gas: 3356668) -OffRamp_execute:test_MultipleReports() (gas: 290810) -OffRamp_execute:test_MultipleReportsWithPartialValidationFailures() (gas: 364216) -OffRamp_execute:test_SingleReport() (gas: 168660) -OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 51668) +OffRamp_constructor:test_Constructor() (gas: 6211675) +OffRamp_execute:test_LargeBatch() (gas: 3355498) +OffRamp_execute:test_MultipleReports() (gas: 290693) +OffRamp_execute:test_MultipleReportsWithPartialValidationFailures() (gas: 364177) +OffRamp_execute:test_SingleReport() (gas: 168621) +OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 51629) OffRamp_executeSingleMessage:test_executeSingleMessage_NonContract() (gas: 20508) -OffRamp_executeSingleMessage:test_executeSingleMessage_NonContractWithTokens() (gas: 230500) -OffRamp_executeSingleMessage:test_executeSingleMessage_WithMessageInterceptor() (gas: 87507) -OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens() (gas: 260081) -OffRamp_executeSingleReport:test_InvalidSourcePoolAddress() (gas: 454936) -OffRamp_executeSingleReport:test_ReceiverError() (gas: 180740) -OffRamp_executeSingleReport:test_SingleMessageNoTokens() (gas: 204974) -OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain() (gas: 241285) -OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered() (gas: 184983) -OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver() (gas: 243852) -OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 134510) +OffRamp_executeSingleMessage:test_executeSingleMessage_NonContractWithTokens() (gas: 230422) +OffRamp_executeSingleMessage:test_executeSingleMessage_WithMessageInterceptor() (gas: 87468) +OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens() (gas: 259964) +OffRamp_executeSingleReport:test_InvalidSourcePoolAddress() (gas: 454780) +OffRamp_executeSingleReport:test_ReceiverError() (gas: 180701) +OffRamp_executeSingleReport:test_SingleMessageNoTokens() (gas: 204896) +OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain() (gas: 241207) +OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered() (gas: 184905) +OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver() (gas: 243806) +OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 134471) OffRamp_executeSingleReport:test_SkippedIncorrectNonce() (gas: 58298) -OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes() (gas: 392022) -OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE() (gas: 561726) -OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 510090) -OffRamp_executeSingleReport:test_Unhealthy() (gas: 528255) -OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain() (gas: 439300) -OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage() (gas: 157898) -OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered() (gas: 128289) +OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes() (gas: 391905) +OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE() (gas: 561492) +OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 509856) +OffRamp_executeSingleReport:test_Unhealthy() (gas: 528021) +OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain() (gas: 439066) +OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage() (gas: 157859) +OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered() (gas: 128250) OffRamp_getExecutionState:test_FillExecutionState() (gas: 3905742) OffRamp_getExecutionState:test_GetDifferentChainExecutionState() (gas: 121049) OffRamp_getExecutionState:test_GetExecutionState() (gas: 89738) -OffRamp_manuallyExecute:test_manuallyExecute() (gas: 212136) -OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched() (gas: 165627) -OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit() (gas: 475645) -OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 2214967) -OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride() (gas: 212686) -OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride() (gas: 730621) -OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages() (gas: 336254) -OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken() (gas: 94672) -OffRamp_releaseOrMintTokens:test_releaseOrMintTokens() (gas: 161242) -OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_WithGasOverride() (gas: 163112) -OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals() (gas: 174351) +OffRamp_manuallyExecute:test_manuallyExecute() (gas: 212058) +OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched() (gas: 165588) +OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit() (gas: 475567) +OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails() (gas: 2214889) +OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride() (gas: 212608) +OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride() (gas: 730231) +OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages() (gas: 336098) +OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken() (gas: 94633) +OffRamp_releaseOrMintTokens:test_releaseOrMintTokens() (gas: 161164) +OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_WithGasOverride() (gas: 163034) +OffRamp_releaseOrMintTokens:test_releaseOrMintTokens_destDenominatedDecimals() (gas: 174273) OffRamp_setDynamicConfig:test_SetDynamicConfig() (gas: 25397) OffRamp_setDynamicConfig:test_SetDynamicConfigWithInterceptor() (gas: 47448) -OffRamp_trialExecute:test_trialExecute() (gas: 263717) -OffRamp_trialExecute:test_trialExecute_RateLimitError() (gas: 120730) -OffRamp_trialExecute:test_trialExecute_TokenHandlingErrorIsCaught() (gas: 132040) -OffRamp_trialExecute:test_trialExecute_TokenPoolIsNotAContract() (gas: 281358) +OffRamp_trialExecute:test_trialExecute() (gas: 263600) +OffRamp_trialExecute:test_trialExecute_RateLimitError() (gas: 120691) +OffRamp_trialExecute:test_trialExecute_TokenHandlingErrorIsCaught() (gas: 132001) +OffRamp_trialExecute:test_trialExecute_TokenPoolIsNotAContract() (gas: 281241) OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy() (gas: 244196) OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates() (gas: 325984) OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_InvalidAllowListRequestDisabledAllowListWithAdds() (gas: 17227) diff --git a/contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol b/contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol index 1ff6f08c772..8844882f53a 100644 --- a/contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol +++ b/contracts/src/v0.8/ccip/libraries/ERC165CheckerReverting.sol @@ -6,9 +6,9 @@ import {IERC165} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/utils /// @notice Library used to query support of an interface declared via {IERC165}. /// @dev These functions return the actual result of the query: they do not `revert` if an interface is not supported. library ERC165CheckerReverting { - error InsufficientGasForStaticcall(); + error InsufficientGasForStaticCall(); - // As per the EIP-165 spec, no interface should ever match 0xffffffff + // As per the EIP-165 spec, no interface should ever match 0xffffffff. bytes4 private constant INTERFACE_ID_INVALID = 0xffffffff; /// @dev 30k gas is required to make the staticcall. Under the 63/64 rule this means that 30,477 gas must be available @@ -17,26 +17,17 @@ library ERC165CheckerReverting { /// 30,000 = ((30,477 * 63) / 64) uint256 private constant MINIMUM_GAS_REQUIREMENT = 31_000; - /// @notice Returns true if `account` supports the {IERC165} interface. - /// @dev Any contract that implements ERC165 must explicitly indicate support of InterfaceId_ERC165 and explicitly - /// indicate non-support of InterfaceId_Invalid as per the standard. - /// @param account the address to be queried for support for ERC165 - /// @return true if the contract at account indicates support for ERC165, false otherwise - function _supportsERC165Reverting( - address account - ) internal view returns (bool) { - return _supportsERC165InterfaceUncheckedReverting(account, type(IERC165).interfaceId) - && !_supportsERC165InterfaceUncheckedReverting(account, INTERFACE_ID_INVALID); - } - - /// @notice Returns true if `account` supports a defined interface - /// @dev The function must support both the interfaceId and interfaces specified by ERC165 generally as per the standard - /// @param account the contract to be queried for support - /// @param interfaceId the interface being checked for support + /// @notice Returns true if `account` supports a defined interface. + /// @dev The function must support both the interfaceId and interfaces specified by ERC165 generally as per the standard. + /// @param account the contract to be queried for support. + /// @param interfaceId the interface being checked for support. /// @return true if the contract at account indicates support of the interface with, false otherwise. function _supportsInterfaceReverting(address account, bytes4 interfaceId) internal view returns (bool) { - // query support of both ERC165 as per the spec and support of _interfaceId - return _supportsERC165Reverting(account) && _supportsERC165InterfaceUncheckedReverting(account, interfaceId); + // As a gas optimization, short circuit return false if interfaceId is not supported, as it is most likely interfaceId + // to be unsupported by the target. + return _supportsERC165InterfaceUncheckedReverting(account, interfaceId) + && !_supportsERC165InterfaceUncheckedReverting(account, INTERFACE_ID_INVALID) + && _supportsERC165InterfaceUncheckedReverting(account, type(IERC165).interfaceId); } /// @notice Query if a contract implements an interface, does not check ERC165 support @@ -45,7 +36,7 @@ library ERC165CheckerReverting { /// @return true if the contract at account indicates support of the interface with /// identifier interfaceId, false otherwise /// @dev Assumes that account contains a contract that supports ERC165, otherwise - /// the behavior of this method is undefined. This precondition can be checked + /// the behavior of this method is undefined. This precondition can be checked. /// @dev Function will only revert if the minimum gas requirement is not met before the staticcall is performed. function _supportsERC165InterfaceUncheckedReverting(address account, bytes4 interfaceId) internal view returns (bool) { bytes memory encodedParams = abi.encodeWithSelector(IERC165.supportsInterface.selector, interfaceId); @@ -54,7 +45,7 @@ library ERC165CheckerReverting { uint256 returnSize; uint256 returnValue; - bytes4 notEnoughGasSelector = InsufficientGasForStaticcall.selector; + bytes4 notEnoughGasSelector = InsufficientGasForStaticCall.selector; assembly { // The EVM does not return a specific error code if a revert is due to OOG. This check ensures that diff --git a/contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting.supportsInterfaceReverting.t.sol b/contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting.supportsInterfaceReverting.t.sol index d8fa3bd14c8..89e61ba2063 100644 --- a/contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting.supportsInterfaceReverting.t.sol +++ b/contracts/src/v0.8/ccip/test/libraries/ERC165CheckerReverting.supportsInterfaceReverting.t.sol @@ -15,7 +15,7 @@ contract ERC165CheckerReverting_supportsInterfaceReverting is Test { bytes4 internal constant EXAMPLE_INTERFACE_ID = 0xdeadbeef; - error InsufficientGasForStaticcall(); + error InsufficientGasForStaticCall(); constructor() { s_receiver = address(new MaybeRevertMessageReceiver(false)); @@ -28,13 +28,13 @@ contract ERC165CheckerReverting_supportsInterfaceReverting is Test { // Reverts function test__supportsInterfaceReverting_RevertWhen_NotEnoughGasForSupportsInterface() public { - vm.expectRevert(InsufficientGasForStaticcall.selector); + vm.expectRevert(InsufficientGasForStaticCall.selector); // Library calls cannot be called with gas limit overrides, so a public function must be exposed // instead which can proxy the call to the library. - // The gas limit was chosen so that after overhead, <30k would remain to trigger the error. - this.invokeERC165Checker{gas: 35_000}(); + // The gas limit was chosen so that after overhead, <31k would remain to trigger the error. + this.invokeERC165Checker{gas: 33_000}(); } // Meant to test the call with a manual gas limit override