Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CCIP-4477 : make gas for call exact check immutable #15575

Merged
merged 5 commits into from
Dec 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions contracts/.changeset/yellow-mugs-explode.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@chainlink/contracts': minor
---

#internal make gas for call exact check immutable

PR issue: CCIP-4477

Solidity Review issue: CCIP-3966
26 changes: 13 additions & 13 deletions contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ NonceManager_applyPreviousRampsUpdates:test_PreviousRampAlreadySet_overrideAllow
NonceManager_applyPreviousRampsUpdates:test_SingleRampUpdate_success() (gas: 66889)
NonceManager_applyPreviousRampsUpdates:test_ZeroInput_success() (gas: 12213)
NonceManager_typeAndVersion:test_typeAndVersion() (gas: 9705)
OffRamp_afterOC3ConfigSet:test_afterOCR3ConfigSet_SignatureVerificationDisabled_Revert() (gas: 5872422)
OffRamp_afterOC3ConfigSet:test_afterOCR3ConfigSet_SignatureVerificationDisabled_Revert() (gas: 5903354)
OffRamp_applySourceChainConfigUpdates:test_AddMultipleChains_Success() (gas: 626094)
OffRamp_applySourceChainConfigUpdates:test_AddNewChain_Success() (gas: 166505)
OffRamp_applySourceChainConfigUpdates:test_ApplyZeroUpdates_Success() (gas: 16719)
Expand All @@ -393,8 +393,8 @@ OffRamp_commit:test_FailedRMNVerification_Reverts() (gas: 63117)
OffRamp_commit:test_InvalidIntervalMinLargerThanMax_Revert() (gas: 69655)
OffRamp_commit:test_InvalidInterval_Revert() (gas: 65803)
OffRamp_commit:test_InvalidRootRevert() (gas: 64898)
OffRamp_commit:test_NoConfigWithOtherConfigPresent_Revert() (gas: 6633259)
OffRamp_commit:test_NoConfig_Revert() (gas: 6216677)
OffRamp_commit:test_NoConfigWithOtherConfigPresent_Revert() (gas: 6664144)
OffRamp_commit:test_NoConfig_Revert() (gas: 6247562)
OffRamp_commit:test_OnlyGasPriceUpdates_Success() (gas: 112728)
OffRamp_commit:test_OnlyPriceUpdateStaleReport_Revert() (gas: 120561)
OffRamp_commit:test_OnlyTokenPriceUpdates_Success() (gas: 112660)
Expand All @@ -409,23 +409,23 @@ OffRamp_commit:test_UnauthorizedTransmitter_Revert() (gas: 125027)
OffRamp_commit:test_Unhealthy_Revert() (gas: 60177)
OffRamp_commit:test_ValidPriceUpdateThenStaleReportWithRoot_Success() (gas: 206221)
OffRamp_commit:test_ZeroEpochAndRound_Revert() (gas: 53305)
OffRamp_constructor:test_Constructor_Success() (gas: 6179080)
OffRamp_constructor:test_SourceChainSelector_Revert() (gas: 136555)
OffRamp_constructor:test_ZeroChainSelector_Revert() (gas: 103592)
OffRamp_constructor:test_ZeroNonceManager_Revert() (gas: 101441)
OffRamp_constructor:test_ZeroOnRampAddress_Revert() (gas: 162036)
OffRamp_constructor:test_ZeroRMNRemote_Revert() (gas: 101358)
OffRamp_constructor:test_ZeroTokenAdminRegistry_Revert() (gas: 101362)
OffRamp_constructor:test_Constructor_Success() (gas: 6210339)
OffRamp_constructor:test_SourceChainSelector_Revert() (gas: 137118)
OffRamp_constructor:test_ZeroChainSelector_Revert() (gas: 103828)
OffRamp_constructor:test_ZeroNonceManager_Revert() (gas: 101677)
OffRamp_constructor:test_ZeroOnRampAddress_Revert() (gas: 162599)
OffRamp_constructor:test_ZeroRMNRemote_Revert() (gas: 101597)
OffRamp_constructor:test_ZeroTokenAdminRegistry_Revert() (gas: 101598)
OffRamp_execute:test_IncorrectArrayType_Revert() (gas: 17532)
OffRamp_execute:test_LargeBatch_Success() (gas: 3378447)
OffRamp_execute:test_MultipleReportsWithPartialValidationFailures_Success() (gas: 371209)
OffRamp_execute:test_MultipleReports_Success() (gas: 298806)
OffRamp_execute:test_NoConfigWithOtherConfigPresent_Revert() (gas: 7041684)
OffRamp_execute:test_NoConfig_Revert() (gas: 6266154)
OffRamp_execute:test_NoConfigWithOtherConfigPresent_Revert() (gas: 7072622)
OffRamp_execute:test_NoConfig_Revert() (gas: 6297092)
OffRamp_execute:test_NonArray_Revert() (gas: 27572)
OffRamp_execute:test_SingleReport_Success() (gas: 175631)
OffRamp_execute:test_UnauthorizedTransmitter_Revert() (gas: 147790)
OffRamp_execute:test_WrongConfigWithSigners_Revert() (gas: 6933352)
OffRamp_execute:test_WrongConfigWithSigners_Revert() (gas: 6964290)
OffRamp_execute:test_ZeroReports_Revert() (gas: 17248)
OffRamp_executeSingleMessage:test_executeSingleMessage_NoTokens() (gas: 56213)
OffRamp_executeSingleMessage:test_executeSingleMessage_NonContract() (gas: 20508)
Expand Down
4 changes: 0 additions & 4 deletions contracts/src/v0.8/ccip/libraries/Internal.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ import {MerkleMultiProof} from "../libraries/MerkleMultiProof.sol";
library Internal {
error InvalidEVMAddress(bytes encodedAddress);

/// @dev The minimum amount of gas to perform the call with exact gas.
/// We include this in the offramp so that we can redeploy to adjust it should a hardfork change the gas costs of
/// relevant opcodes in callWithExactGas.
uint16 internal constant GAS_FOR_CALL_EXACT_CHECK = 5_000;
/// @dev We limit return data to a selector plus 4 words. This is to avoid malicious contracts from returning
/// large amounts of data and causing repeated out-of-gas scenarios.
uint16 internal constant MAX_RET_BYTES = 4 + 4 * 32;
Expand Down
21 changes: 12 additions & 9 deletions contracts/src/v0.8/ccip/offRamp/OffRamp.sol
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,9 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
/// @dev RMN depends on this struct, if changing, please notify the RMN maintainers.
// solhint-disable-next-line gas-struct-packing
struct StaticConfig {
uint64 chainSelector; // ────╮ Destination chainSelector
IRMNRemote rmnRemote; // ────╯ RMN Verification Contract
uint64 chainSelector; // ───────╮ Destination chainSelector
uint16 gasForCallExactCheck; // | Gas for call exact check
IRMNRemote rmnRemote; // ───────╯ RMN Verification Contract
address tokenAdminRegistry; // Token admin registry address
address nonceManager; // Nonce manager address
}
Expand Down Expand Up @@ -151,6 +152,10 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
address internal immutable i_tokenAdminRegistry;
/// @dev The address of the nonce manager.
address internal immutable i_nonceManager;
/// @dev The minimum amount of gas to perform the call with exact gas.
/// We include this in the offramp so that we can redeploy to adjust it should a hardfork change the gas costs of
/// relevant opcodes in callWithExactGas.
uint16 internal immutable i_gasForCallExactCheck;

// DYNAMIC CONFIG
DynamicConfig internal s_dynamicConfig;
Expand Down Expand Up @@ -193,6 +198,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
i_rmnRemote = staticConfig.rmnRemote;
i_tokenAdminRegistry = staticConfig.tokenAdminRegistry;
i_nonceManager = staticConfig.nonceManager;
i_gasForCallExactCheck = staticConfig.gasForCallExactCheck;
emit StaticConfigSet(staticConfig);

_setDynamicConfig(dynamicConfig);
Expand Down Expand Up @@ -601,7 +607,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {

(bool success, bytes memory returnData,) = s_sourceChainConfigs[message.header.sourceChainSelector]
.router
.routeMessage(any2EvmMessage, Internal.GAS_FOR_CALL_EXACT_CHECK, message.gasLimit, message.receiver);
.routeMessage(any2EvmMessage, i_gasForCallExactCheck, message.gasLimit, message.receiver);
// If CCIP receiver execution is not successful, revert the call including token transfers.
if (!success) revert ReceiverError(returnData);
}
Expand Down Expand Up @@ -665,7 +671,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
),
localPoolAddress,
gasLeft,
Internal.GAS_FOR_CALL_EXACT_CHECK,
i_gasForCallExactCheck,
Internal.MAX_RET_BYTES
);

Expand Down Expand Up @@ -705,11 +711,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
uint256 gasLimit
) internal returns (uint256 balance, uint256 gasLeft) {
(bool success, bytes memory returnData, uint256 gasUsed) = CallWithExactGas._callWithExactGasSafeReturnData(
abi.encodeCall(IERC20.balanceOf, (receiver)),
token,
gasLimit,
Internal.GAS_FOR_CALL_EXACT_CHECK,
Internal.MAX_RET_BYTES
abi.encodeCall(IERC20.balanceOf, (receiver)), token, gasLimit, i_gasForCallExactCheck, Internal.MAX_RET_BYTES
);
if (!success) revert TokenHandlingError(token, returnData);

Expand Down Expand Up @@ -906,6 +908,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base {
function getStaticConfig() external view returns (StaticConfig memory) {
return StaticConfig({
chainSelector: i_chainSelector,
gasForCallExactCheck: i_gasForCallExactCheck,
rmnRemote: i_rmnRemote,
tokenAdminRegistry: i_tokenAdminRegistry,
nonceManager: i_nonceManager
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/v0.8/ccip/test/BaseTest.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ contract BaseTest is Test {
// OffRamp
uint32 internal constant MAX_DATA_SIZE = 30_000;
uint16 internal constant MAX_TOKENS_LENGTH = 5;
uint16 internal constant GAS_FOR_CALL_EXACT_CHECK = 5000;
uint16 internal constant GAS_FOR_CALL_EXACT_CHECK = 5_000;
uint32 internal constant MAX_GAS_LIMIT = 4_000_000;

MockRMN internal s_mockRMN;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ contract OffRamp_afterOC3ConfigSet is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ contract OffRamp_constructor is OffRampSetup {
function test_Constructor_Success() public {
OffRamp.StaticConfig memory staticConfig = OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand Down Expand Up @@ -142,6 +143,7 @@ contract OffRamp_constructor is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand All @@ -168,6 +170,7 @@ contract OffRamp_constructor is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand All @@ -188,6 +191,7 @@ contract OffRamp_constructor is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: IRMNRemote(address(0)),
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand All @@ -208,6 +212,7 @@ contract OffRamp_constructor is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: 0,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand All @@ -228,6 +233,7 @@ contract OffRamp_constructor is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(0),
nonceManager: address(s_inboundNonceManager)
Expand All @@ -248,6 +254,7 @@ contract OffRamp_constructor is OffRampSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ contract OffRamp_executeSingleMessage is OffRampSetup {
abi.encodeWithSelector(
IRouter.routeMessage.selector,
expectedAny2EvmMessage,
Internal.GAS_FOR_CALL_EXACT_CHECK,
GAS_FOR_CALL_EXACT_CHECK,
message.gasLimit,
message.receiver
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ contract OffRampSetup is FeeQuoterSetup, MultiOCR3BaseSetup {
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
rmnRemote: rmnRemote,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(nonceManager)
}),
Expand Down Expand Up @@ -348,6 +349,7 @@ contract OffRampSetup is FeeQuoterSetup, MultiOCR3BaseSetup {
s_offRamp = new OffRampHelper(
OffRamp.StaticConfig({
chainSelector: DEST_CHAIN_SELECTOR,
gasForCallExactCheck: GAS_FOR_CALL_EXACT_CHECK,
rmnRemote: s_mockRMNRemote,
tokenAdminRegistry: address(s_tokenAdminRegistry),
nonceManager: address(s_inboundNonceManager)
Expand Down
9 changes: 5 additions & 4 deletions core/gethwrappers/ccip/deployment_test/deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ func TestDeployAllV1_6(t *testing.T) {

// offramp
_, _, _, err = offramp.DeployOffRamp(owner, chain, offramp.OffRampStaticConfig{
ChainSelector: 1,
RmnRemote: common.HexToAddress("0x1"),
TokenAdminRegistry: common.HexToAddress("0x2"),
NonceManager: common.HexToAddress("0x3"),
ChainSelector: 1,
GasForCallExactCheck: 5_000,
RmnRemote: common.HexToAddress("0x1"),
TokenAdminRegistry: common.HexToAddress("0x2"),
NonceManager: common.HexToAddress("0x3"),
}, offramp.OffRampDynamicConfig{
FeeQuoter: common.HexToAddress("0x4"),
PermissionLessExecutionThresholdSeconds: uint32((8 * time.Hour).Seconds()),
Expand Down
15 changes: 8 additions & 7 deletions core/gethwrappers/ccip/generated/offramp/offramp.go

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ mock_v3_aggregator_contract: ../../../contracts/solc/v0.8.24/MockV3Aggregator/Mo
multi_aggregate_rate_limiter: ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.abi ../../../contracts/solc/v0.8.24/MultiAggregateRateLimiter/MultiAggregateRateLimiter.bin c3cac2010c2815b484055bf981363a2bd04e7fbe7bb502dc8fd29a16165d221c
multi_ocr3_helper: ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.abi ../../../contracts/solc/v0.8.24/MultiOCR3Helper/MultiOCR3Helper.bin a523e11ea4c069d7d61b309c156951cc6834aff0f352bd1ac37c3a838ff2588f
nonce_manager: ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.abi ../../../contracts/solc/v0.8.24/NonceManager/NonceManager.bin e6008490d916826cefd1903612db39621d51617300fc9bb42b68c6c117958198
offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin 7c65e586181c5099a6ecb5353f60043bb6add9ebad941ddf7ef9998c7ee008ea
offramp: ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.abi ../../../contracts/solc/v0.8.24/OffRamp/OffRamp.bin 067fdfbf7cae1557fc03ca16d9c38737ee4595655792a1b8bc4846c45caa0c74
onramp: ../../../contracts/solc/v0.8.24/OnRamp/OnRamp.abi ../../../contracts/solc/v0.8.24/OnRamp/OnRamp.bin 2bf74188a997218502031f177cb2df505b272d66b25fd341a741289e77380c59
ping_pong_demo: ../../../contracts/solc/v0.8.24/PingPongDemo/PingPongDemo.abi ../../../contracts/solc/v0.8.24/PingPongDemo/PingPongDemo.bin 24b4415a883a470d65c484be0fa20714a46b1c9262db205f1c958017820307b2
registry_module_owner_custom: ../../../contracts/solc/v0.8.24/RegistryModuleOwnerCustom/RegistryModuleOwnerCustom.abi ../../../contracts/solc/v0.8.24/RegistryModuleOwnerCustom/RegistryModuleOwnerCustom.bin 0fc277a0b512db4e20b5a32a775b94ed2c0d342d8237511de78c94f7dacad428
Expand Down
9 changes: 5 additions & 4 deletions deployment/ccip/changeset/cs_deploy_chain.go
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,11 @@ func deployChainContracts(
chain.DeployerKey,
chain.Client,
offramp.OffRampStaticConfig{
ChainSelector: chain.Selector,
RmnRemote: rmnProxyContract.Address(),
NonceManager: nmContract.Address(),
TokenAdminRegistry: tokenAdminReg.Address(),
ChainSelector: chain.Selector,
GasForCallExactCheck: 5_000,
RmnRemote: rmnProxyContract.Address(),
NonceManager: nmContract.Address(),
TokenAdminRegistry: tokenAdminReg.Address(),
},
offramp.OffRampDynamicConfig{
FeeQuoter: feeQuoterContract.Address(),
Expand Down
Loading