From 6db71d32d756eba147ec69f385252aa51589d517 Mon Sep 17 00:00:00 2001 From: Rens Rooimans Date: Mon, 28 Oct 2024 19:11:54 +0100 Subject: [PATCH] CCIP-4010 fix various nits (#14972) * fix various nits * rm redundant rpc call * [Bot] Update changeset file with jira issues * fix comments * add test for router update * undo RMNHome check * fix forge fmt --------- Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com> --- contracts/.changeset/metal-radios-push.md | 10 + contracts/gas-snapshots/ccip.gas-snapshot | 165 +++++----- contracts/src/v0.8/ccip/Router.sol | 4 +- .../ccip/applications/CCIPClientExample.sol | 4 +- .../src/v0.8/ccip/capability/CCIPHome.sol | 7 - .../src/v0.8/ccip/libraries/Internal.sol | 10 +- contracts/src/v0.8/ccip/offRamp/OffRamp.sol | 309 +++++++++--------- contracts/src/v0.8/ccip/onRamp/OnRamp.sol | 119 +++---- .../v0.8/ccip/pools/LockReleaseTokenPool.sol | 3 +- contracts/src/v0.8/ccip/rmn/RMNHome.sol | 11 +- contracts/src/v0.8/ccip/rmn/RMNRemote.sol | 26 +- .../src/v0.8/ccip/test/offRamp/OffRamp.t.sol | 40 +++ .../src/v0.8/ccip/test/onRamp/OnRamp.t.sol | 25 +- .../ccip/generated/ccip_home/ccip_home.go | 2 +- .../ccip/generated/offramp/offramp.go | 2 +- .../ccip/generated/onramp/onramp.go | 28 +- ...rapper-dependency-versions-do-not-edit.txt | 6 +- .../deployment/ccip/view/v1_6/onramp.go | 6 - 18 files changed, 394 insertions(+), 383 deletions(-) create mode 100644 contracts/.changeset/metal-radios-push.md diff --git a/contracts/.changeset/metal-radios-push.md b/contracts/.changeset/metal-radios-push.md new file mode 100644 index 00000000000..ebf95bcddc6 --- /dev/null +++ b/contracts/.changeset/metal-radios-push.md @@ -0,0 +1,10 @@ +--- +'@chainlink/contracts': patch +--- + +#internal minor nits, allow Router updates even when the offRamp has been used. Remove getRouter from onRamp + + +PR issue: CCIP-4010 + +Solidity Review issue: CCIP-3966 \ No newline at end of file diff --git a/contracts/gas-snapshots/ccip.gas-snapshot b/contracts/gas-snapshots/ccip.gas-snapshot index c0493393221..c73de5eb7a1 100644 --- a/contracts/gas-snapshots/ccip.gas-snapshot +++ b/contracts/gas-snapshots/ccip.gas-snapshot @@ -68,7 +68,7 @@ CCIPHome_setCandidate:test_setCandidate_ConfigDigestMismatch_reverts() (gas: 139 CCIPHome_setCandidate:test_setCandidate_success() (gas: 1365439) DefensiveExampleTest:test_HappyPath_Success() (gas: 200473) DefensiveExampleTest:test_Recovery() (gas: 424859) -E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1522070) +E2E:test_E2E_3MessagesMMultiOffRampSuccess_gas() (gas: 1521230) EtherSenderReceiverTest_ccipReceive:test_ccipReceive_fallbackToWethTransfer() (gas: 96962) EtherSenderReceiverTest_ccipReceive:test_ccipReceive_happyPath() (gas: 49812) EtherSenderReceiverTest_ccipReceive:test_ccipReceive_wrongToken() (gas: 17457) @@ -374,12 +374,12 @@ NonceManager_NonceIncrementation:test_getIncrementedOutboundNonce_Success() (gas NonceManager_NonceIncrementation:test_incrementInboundNonce_Skip() (gas: 23706) NonceManager_NonceIncrementation:test_incrementInboundNonce_Success() (gas: 38778) NonceManager_NonceIncrementation:test_incrementNoncesInboundAndOutbound_Success() (gas: 71901) -NonceManager_OffRampUpgrade:test_NoPrevOffRampForChain_Success() (gas: 186219) -NonceManager_OffRampUpgrade:test_UpgradedNonceNewSenderStartsAtZero_Success() (gas: 189666) -NonceManager_OffRampUpgrade:test_UpgradedNonceStartsAtV1Nonce_Success() (gas: 253079) -NonceManager_OffRampUpgrade:test_UpgradedOffRampNonceSkipsIfMsgInFlight_Success() (gas: 221316) +NonceManager_OffRampUpgrade:test_NoPrevOffRampForChain_Success() (gas: 185976) +NonceManager_OffRampUpgrade:test_UpgradedNonceNewSenderStartsAtZero_Success() (gas: 189423) +NonceManager_OffRampUpgrade:test_UpgradedNonceStartsAtV1Nonce_Success() (gas: 252593) +NonceManager_OffRampUpgrade:test_UpgradedOffRampNonceSkipsIfMsgInFlight_Success() (gas: 220830) NonceManager_OffRampUpgrade:test_UpgradedSenderNoncesReadsPreviousRamp_Success() (gas: 60591) -NonceManager_OffRampUpgrade:test_Upgraded_Success() (gas: 153253) +NonceManager_OffRampUpgrade:test_Upgraded_Success() (gas: 153010) NonceManager_OnRampUpgrade:test_UpgradeNonceNewSenderStartsAtZero_Success() (gas: 166101) NonceManager_OnRampUpgrade:test_UpgradeNonceStartsAtV1Nonce_Success() (gas: 195806) NonceManager_OnRampUpgrade:test_UpgradeSenderNoncesReadsPreviousRamp_Success() (gas: 139098) @@ -392,23 +392,24 @@ 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: 5926987) +OffRamp_afterOC3ConfigSet:test_afterOCR3ConfigSet_SignatureVerificationDisabled_Revert() (gas: 5910204) OffRamp_applySourceChainConfigUpdates:test_AddMultipleChains_Success() (gas: 626115) OffRamp_applySourceChainConfigUpdates:test_AddNewChain_Success() (gas: 166515) -OffRamp_applySourceChainConfigUpdates:test_ApplyZeroUpdates_Success() (gas: 16741) -OffRamp_applySourceChainConfigUpdates:test_InvalidOnRampUpdate_Revert() (gas: 274304) -OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChainOnRamp_Success() (gas: 168534) -OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChain_Success() (gas: 181033) +OffRamp_applySourceChainConfigUpdates:test_ApplyZeroUpdates_Success() (gas: 16763) +OffRamp_applySourceChainConfigUpdates:test_InvalidOnRampUpdate_Revert() (gas: 275075) +OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChainOnRamp_Success() (gas: 168560) +OffRamp_applySourceChainConfigUpdates:test_ReplaceExistingChain_Success() (gas: 181059) OffRamp_applySourceChainConfigUpdates:test_RouterAddress_Revert() (gas: 13463) OffRamp_applySourceChainConfigUpdates:test_ZeroOnRampAddress_Revert() (gas: 72746) -OffRamp_applySourceChainConfigUpdates:test_ZeroSourceChainSelector_Revert() (gas: 15541) -OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain_Success() (gas: 177807) -OffRamp_batchExecute:test_MultipleReportsDifferentChains_Success() (gas: 334644) -OffRamp_batchExecute:test_MultipleReportsSameChain_Success() (gas: 277910) -OffRamp_batchExecute:test_MultipleReportsSkipDuplicate_Success() (gas: 168974) -OffRamp_batchExecute:test_OutOfBoundsGasLimitsAccess_Revert() (gas: 188444) -OffRamp_batchExecute:test_SingleReport_Success() (gas: 156798) -OffRamp_batchExecute:test_Unhealthy_Success() (gas: 554586) +OffRamp_applySourceChainConfigUpdates:test_ZeroSourceChainSelector_Revert() (gas: 15476) +OffRamp_applySourceChainConfigUpdates:test_allowNonOnRampUpdateAfterLaneIsUsed_success() (gas: 285425) +OffRamp_batchExecute:test_MultipleReportsDifferentChainsSkipCursedChain_Success() (gas: 177564) +OffRamp_batchExecute:test_MultipleReportsDifferentChains_Success() (gas: 333809) +OffRamp_batchExecute:test_MultipleReportsSameChain_Success() (gas: 277075) +OffRamp_batchExecute:test_MultipleReportsSkipDuplicate_Success() (gas: 168494) +OffRamp_batchExecute:test_OutOfBoundsGasLimitsAccess_Revert() (gas: 187853) +OffRamp_batchExecute:test_SingleReport_Success() (gas: 156555) +OffRamp_batchExecute:test_Unhealthy_Success() (gas: 553993) OffRamp_batchExecute:test_ZeroReports_Revert() (gas: 10600) OffRamp_ccipReceive:test_Reverts() (gas: 15407) OffRamp_commit:test_CommitOnRampMismatch_Revert() (gas: 93097) @@ -416,8 +417,8 @@ OffRamp_commit:test_FailedRMNVerification_Reverts() (gas: 63763) OffRamp_commit:test_InvalidIntervalMinLargerThanMax_Revert() (gas: 70395) OffRamp_commit:test_InvalidInterval_Revert() (gas: 66458) OffRamp_commit:test_InvalidRootRevert() (gas: 65545) -OffRamp_commit:test_NoConfigWithOtherConfigPresent_Revert() (gas: 6688360) -OffRamp_commit:test_NoConfig_Revert() (gas: 6271690) +OffRamp_commit:test_NoConfigWithOtherConfigPresent_Revert() (gas: 6671577) +OffRamp_commit:test_NoConfig_Revert() (gas: 6254907) OffRamp_commit:test_OnlyGasPriceUpdates_Success() (gas: 113187) OffRamp_commit:test_OnlyPriceUpdateStaleReport_Revert() (gas: 121714) OffRamp_commit:test_OnlyTokenPriceUpdates_Success() (gas: 113186) @@ -432,23 +433,23 @@ OffRamp_commit:test_UnauthorizedTransmitter_Revert() (gas: 125579) OffRamp_commit:test_Unhealthy_Revert() (gas: 60822) OffRamp_commit:test_ValidPriceUpdateThenStaleReportWithRoot_Success() (gas: 207441) OffRamp_commit:test_ZeroEpochAndRound_Revert() (gas: 53889) -OffRamp_constructor:test_Constructor_Success() (gas: 6233679) -OffRamp_constructor:test_SourceChainSelector_Revert() (gas: 136597) -OffRamp_constructor:test_ZeroChainSelector_Revert() (gas: 103678) -OffRamp_constructor:test_ZeroNonceManager_Revert() (gas: 101505) -OffRamp_constructor:test_ZeroOnRampAddress_Revert() (gas: 162100) -OffRamp_constructor:test_ZeroRMNRemote_Revert() (gas: 101422) -OffRamp_constructor:test_ZeroTokenAdminRegistry_Revert() (gas: 101471) +OffRamp_constructor:test_Constructor_Success() (gas: 6216896) +OffRamp_constructor:test_SourceChainSelector_Revert() (gas: 136627) +OffRamp_constructor:test_ZeroChainSelector_Revert() (gas: 103707) +OffRamp_constructor:test_ZeroNonceManager_Revert() (gas: 101534) +OffRamp_constructor:test_ZeroOnRampAddress_Revert() (gas: 162129) +OffRamp_constructor:test_ZeroRMNRemote_Revert() (gas: 101451) +OffRamp_constructor:test_ZeroTokenAdminRegistry_Revert() (gas: 101500) OffRamp_execute:test_IncorrectArrayType_Revert() (gas: 17639) -OffRamp_execute:test_LargeBatch_Success() (gas: 3416637) -OffRamp_execute:test_MultipleReportsWithPartialValidationFailures_Success() (gas: 372340) -OffRamp_execute:test_MultipleReports_Success() (gas: 300029) -OffRamp_execute:test_NoConfigWithOtherConfigPresent_Revert() (gas: 7096447) -OffRamp_execute:test_NoConfig_Revert() (gas: 6320895) +OffRamp_execute:test_LargeBatch_Success() (gas: 3406667) +OffRamp_execute:test_MultipleReportsWithPartialValidationFailures_Success() (gas: 371505) +OffRamp_execute:test_MultipleReports_Success() (gas: 299194) +OffRamp_execute:test_NoConfigWithOtherConfigPresent_Revert() (gas: 7079421) +OffRamp_execute:test_NoConfig_Revert() (gas: 6303869) OffRamp_execute:test_NonArray_Revert() (gas: 27643) -OffRamp_execute:test_SingleReport_Success() (gas: 176052) -OffRamp_execute:test_UnauthorizedTransmitter_Revert() (gas: 148048) -OffRamp_execute:test_WrongConfigWithSigners_Revert() (gas: 6987861) +OffRamp_execute:test_SingleReport_Success() (gas: 175809) +OffRamp_execute:test_UnauthorizedTransmitter_Revert() (gas: 147805) +OffRamp_execute:test_WrongConfigWithSigners_Revert() (gas: 6971078) OffRamp_execute:test_ZeroReports_Revert() (gas: 17317) OffRamp_executeSingleMessage:test_MessageSender_Revert() (gas: 18537) OffRamp_executeSingleMessage:test_NonContractWithTokens_Success() (gas: 244193) @@ -460,51 +461,51 @@ OffRamp_executeSingleMessage:test_executeSingleMessage_WithFailingValidationNoRo OffRamp_executeSingleMessage:test_executeSingleMessage_WithFailingValidation_Revert() (gas: 85455) OffRamp_executeSingleMessage:test_executeSingleMessage_WithTokens_Success() (gas: 274305) OffRamp_executeSingleMessage:test_executeSingleMessage_WithVInterception_Success() (gas: 91944) -OffRamp_executeSingleReport:test_DisabledSourceChain_Revert() (gas: 28407) -OffRamp_executeSingleReport:test_EmptyReport_Revert() (gas: 22659) -OffRamp_executeSingleReport:test_InvalidSourcePoolAddress_Success() (gas: 482389) -OffRamp_executeSingleReport:test_ManualExecutionNotYetEnabled_Revert() (gas: 48508) -OffRamp_executeSingleReport:test_MismatchingDestChainSelector_Revert() (gas: 33996) -OffRamp_executeSingleReport:test_NonExistingSourceChain_Revert() (gas: 28572) -OffRamp_executeSingleReport:test_ReceiverError_Success() (gas: 187941) -OffRamp_executeSingleReport:test_RetryFailedMessageWithoutManualExecution_Revert() (gas: 198309) -OffRamp_executeSingleReport:test_RootNotCommitted_Revert() (gas: 40921) -OffRamp_executeSingleReport:test_RouterYULCall_Revert() (gas: 405240) -OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain_Success() (gas: 249184) -OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered_Success() (gas: 193050) -OffRamp_executeSingleReport:test_SingleMessageNoTokens_Success() (gas: 213073) -OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver_Success() (gas: 243948) -OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 141790) -OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes_Success() (gas: 409554) -OffRamp_executeSingleReport:test_SkippedIncorrectNonce_Success() (gas: 58484) -OffRamp_executeSingleReport:test_TokenDataMismatch_Revert() (gas: 74048) -OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE_Success() (gas: 583802) -OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 532387) -OffRamp_executeSingleReport:test_UnexpectedTokenData_Revert() (gas: 33853) -OffRamp_executeSingleReport:test_UnhealthySingleChainCurse_Revert() (gas: 550226) -OffRamp_executeSingleReport:test_Unhealthy_Success() (gas: 550173) -OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain_Success() (gas: 460842) -OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered_Success() (gas: 135741) -OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage_Success() (gas: 165390) +OffRamp_executeSingleReport:test_DisabledSourceChain_Revert() (gas: 28658) +OffRamp_executeSingleReport:test_EmptyReport_Revert() (gas: 15580) +OffRamp_executeSingleReport:test_InvalidSourcePoolAddress_Success() (gas: 481795) +OffRamp_executeSingleReport:test_ManualExecutionNotYetEnabled_Revert() (gas: 48273) +OffRamp_executeSingleReport:test_MismatchingDestChainSelector_Revert() (gas: 34100) +OffRamp_executeSingleReport:test_NonExistingSourceChain_Revert() (gas: 28823) +OffRamp_executeSingleReport:test_ReceiverError_Success() (gas: 187698) +OffRamp_executeSingleReport:test_RetryFailedMessageWithoutManualExecution_Revert() (gas: 197829) +OffRamp_executeSingleReport:test_RootNotCommitted_Revert() (gas: 40686) +OffRamp_executeSingleReport:test_RouterYULCall_Revert() (gas: 404997) +OffRamp_executeSingleReport:test_SingleMessageNoTokensOtherChain_Success() (gas: 248698) +OffRamp_executeSingleReport:test_SingleMessageNoTokensUnordered_Success() (gas: 192576) +OffRamp_executeSingleReport:test_SingleMessageNoTokens_Success() (gas: 212587) +OffRamp_executeSingleReport:test_SingleMessageToNonCCIPReceiver_Success() (gas: 243705) +OffRamp_executeSingleReport:test_SingleMessagesNoTokensSuccess_gas() (gas: 141547) +OffRamp_executeSingleReport:test_SkippedIncorrectNonceStillExecutes_Success() (gas: 408961) +OffRamp_executeSingleReport:test_SkippedIncorrectNonce_Success() (gas: 58241) +OffRamp_executeSingleReport:test_TokenDataMismatch_Revert() (gas: 73808) +OffRamp_executeSingleReport:test_TwoMessagesWithTokensAndGE_Success() (gas: 583208) +OffRamp_executeSingleReport:test_TwoMessagesWithTokensSuccess_gas() (gas: 531794) +OffRamp_executeSingleReport:test_UnexpectedTokenData_Revert() (gas: 26774) +OffRamp_executeSingleReport:test_UnhealthySingleChainCurse_Revert() (gas: 549633) +OffRamp_executeSingleReport:test_Unhealthy_Success() (gas: 549580) +OffRamp_executeSingleReport:test_WithCurseOnAnotherSourceChain_Success() (gas: 460249) +OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessageUnordered_Success() (gas: 135267) +OffRamp_executeSingleReport:test__execute_SkippedAlreadyExecutedMessage_Success() (gas: 164910) OffRamp_getExecutionState:test_FillExecutionState_Success() (gas: 3911118) OffRamp_getExecutionState:test_GetDifferentChainExecutionState_Success() (gas: 121222) OffRamp_getExecutionState:test_GetExecutionState_Success() (gas: 89706) OffRamp_manuallyExecute:test_ManualExecGasLimitMismatchSingleReport_Revert() (gas: 81178) OffRamp_manuallyExecute:test_manuallyExecute_DestinationGasAmountCountMismatch_Revert() (gas: 74108) -OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched_Success() (gas: 172877) -OffRamp_manuallyExecute:test_manuallyExecute_FailedTx_Revert() (gas: 213413) +OffRamp_manuallyExecute:test_manuallyExecute_DoesNotRevertIfUntouched_Success() (gas: 172634) +OffRamp_manuallyExecute:test_manuallyExecute_FailedTx_Revert() (gas: 212935) OffRamp_manuallyExecute:test_manuallyExecute_ForkedChain_Revert() (gas: 27166) OffRamp_manuallyExecute:test_manuallyExecute_GasLimitMismatchMultipleReports_Revert() (gas: 164939) OffRamp_manuallyExecute:test_manuallyExecute_InvalidReceiverExecutionGasLimit_Revert() (gas: 27703) OffRamp_manuallyExecute:test_manuallyExecute_InvalidTokenGasOverride_Revert() (gas: 55274) -OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit_Success() (gas: 490056) -OffRamp_manuallyExecute:test_manuallyExecute_MultipleReportsWithSingleCursedLane_Revert() (gas: 315336) -OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails_Success() (gas: 2228409) -OffRamp_manuallyExecute:test_manuallyExecute_SourceChainSelectorMismatch_Revert() (gas: 165282) -OffRamp_manuallyExecute:test_manuallyExecute_Success() (gas: 226452) -OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride_Success() (gas: 227014) -OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride_Success() (gas: 777756) -OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages_Success() (gas: 346008) +OffRamp_manuallyExecute:test_manuallyExecute_LowGasLimit_Success() (gas: 489576) +OffRamp_manuallyExecute:test_manuallyExecute_MultipleReportsWithSingleCursedLane_Revert() (gas: 314392) +OffRamp_manuallyExecute:test_manuallyExecute_ReentrancyFails_Success() (gas: 2227930) +OffRamp_manuallyExecute:test_manuallyExecute_SourceChainSelectorMismatch_Revert() (gas: 165133) +OffRamp_manuallyExecute:test_manuallyExecute_Success() (gas: 225972) +OffRamp_manuallyExecute:test_manuallyExecute_WithGasOverride_Success() (gas: 226534) +OffRamp_manuallyExecute:test_manuallyExecute_WithMultiReportGasOverride_Success() (gas: 774706) +OffRamp_manuallyExecute:test_manuallyExecute_WithPartialMessages_Success() (gas: 344831) OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken_NotACompatiblePool_Revert() (gas: 37632) OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken_Success() (gas: 104648) OffRamp_releaseOrMintSingleToken:test__releaseOrMintSingleToken_TokenHandlingError_transfer_Revert() (gas: 83092) @@ -528,17 +529,17 @@ OffRamp_trialExecute:test_TokenHandlingErrorIsCaught_Success() (gas: 228644) OffRamp_trialExecute:test_TokenPoolIsNotAContract_Success() (gas: 295794) OffRamp_trialExecute:test_trialExecute_Success() (gas: 278096) OnRampTokenPoolReentrancy:test_OnRampTokenPoolReentrancy_Success() (gas: 251617) -OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_InvalidAllowListRequestDisabledAllowListWithAdds() (gas: 17265) -OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_Revert() (gas: 67177) -OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_Success() (gas: 326074) -OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_Success() (gas: 65220) +OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_InvalidAllowListRequestDisabledAllowListWithAdds() (gas: 17176) +OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_Revert() (gas: 66999) +OnRamp_applyAllowlistUpdates:test_applyAllowlistUpdates_Success() (gas: 325860) +OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_Success() (gas: 65892) OnRamp_applyDestChainConfigUpdates:test_ApplyDestChainConfigUpdates_WithInvalidChainSelector_Revert() (gas: 12902) -OnRamp_constructor:test_Constructor_EnableAllowList_ForwardFromRouter_Reverts() (gas: 2654417) -OnRamp_constructor:test_Constructor_InvalidConfigChainSelectorEqZero_Revert() (gas: 95283) -OnRamp_constructor:test_Constructor_InvalidConfigNonceManagerEqAddressZero_Revert() (gas: 93225) -OnRamp_constructor:test_Constructor_InvalidConfigRMNProxyEqAddressZero_Revert() (gas: 98201) -OnRamp_constructor:test_Constructor_InvalidConfigTokenAdminRegistryEqAddressZero_Revert() (gas: 93259) -OnRamp_constructor:test_Constructor_Success() (gas: 2734279) +OnRamp_constructor:test_Constructor_EnableAllowList_ForwardFromRouter_Reverts() (gas: 2640786) +OnRamp_constructor:test_Constructor_InvalidConfigChainSelectorEqZero_Revert() (gas: 95267) +OnRamp_constructor:test_Constructor_InvalidConfigNonceManagerEqAddressZero_Revert() (gas: 93209) +OnRamp_constructor:test_Constructor_InvalidConfigRMNProxyEqAddressZero_Revert() (gas: 98185) +OnRamp_constructor:test_Constructor_InvalidConfigTokenAdminRegistryEqAddressZero_Revert() (gas: 93243) +OnRamp_constructor:test_Constructor_Success() (gas: 2718936) OnRamp_forwardFromRouter:test_ForwardFromRouterExtraArgsV2AllowOutOfOrderTrue_Success() (gas: 115376) OnRamp_forwardFromRouter:test_ForwardFromRouterExtraArgsV2_Success() (gas: 146244) OnRamp_forwardFromRouter:test_ForwardFromRouterSuccessCustomExtraArgs() (gas: 145819) diff --git a/contracts/src/v0.8/ccip/Router.sol b/contracts/src/v0.8/ccip/Router.sol index d83216fb6f6..150ba00f39d 100644 --- a/contracts/src/v0.8/ccip/Router.sol +++ b/contracts/src/v0.8/ccip/Router.sol @@ -175,8 +175,8 @@ contract Router is IRouter, IRouterClient, ITypeAndVersion, OwnerIsCreator { return (success, retData, gasUsed); } - // @notice Merges a chain selector and offRamp address into a single uint256 by shifting the - // chain selector 160 bits to the left. + /// @notice Merges a chain selector and offRamp address into a single uint256 by shifting the + /// chain selector 160 bits to the left. function _mergeChainSelectorAndOffRamp( uint64 sourceChainSelector, address offRampAddress diff --git a/contracts/src/v0.8/ccip/applications/CCIPClientExample.sol b/contracts/src/v0.8/ccip/applications/CCIPClientExample.sol index 56f3cd1fe8d..67826cb2e0a 100644 --- a/contracts/src/v0.8/ccip/applications/CCIPClientExample.sol +++ b/contracts/src/v0.8/ccip/applications/CCIPClientExample.sol @@ -140,8 +140,8 @@ contract CCIPClientExample is CCIPReceiver, OwnerIsCreator { emit MessageSent(messageId); } - // @notice user sends tokens to a receiver - // Approvals can be optimized with a whitelist of tokens and inf approvals if desired. + /// @notice user sends tokens to a receiver + /// Approvals can be optimized with a whitelist of tokens and inf approvals if desired. function sendTokens( uint64 destChainSelector, bytes memory receiver, diff --git a/contracts/src/v0.8/ccip/capability/CCIPHome.sol b/contracts/src/v0.8/ccip/capability/CCIPHome.sol index c343e0200ec..26650e925ac 100644 --- a/contracts/src/v0.8/ccip/capability/CCIPHome.sol +++ b/contracts/src/v0.8/ccip/capability/CCIPHome.sol @@ -94,13 +94,6 @@ contract CCIPHome is OwnerIsCreator, ITypeAndVersion, ICapabilityConfiguration, error InvalidSelector(bytes4 selector); error DONIdMismatch(uint32 callDonId, uint32 capabilityRegistryDonId); - error InvalidStateTransition( - bytes32 currentActiveDigest, - bytes32 currentCandidateDigest, - bytes32 proposedActiveDigest, - bytes32 proposedCandidateDigest - ); - /// @notice Represents an oracle node in OCR3 configs part of the role DON. /// Every configured node should be a signer, but does not have to be a transmitter. struct OCR3Node { diff --git a/contracts/src/v0.8/ccip/libraries/Internal.sol b/contracts/src/v0.8/ccip/libraries/Internal.sol index f029fbcbb31..5c5f2c61bbc 100644 --- a/contracts/src/v0.8/ccip/libraries/Internal.sol +++ b/contracts/src/v0.8/ccip/libraries/Internal.sol @@ -11,9 +11,9 @@ library Internal { /// 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. + /// @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; /// @dev The expected number of bytes returned by the balanceOf function. uint256 internal constant MAX_BALANCE_OF_RET_BYTES = 32; @@ -227,8 +227,8 @@ library Internal { // The source pool EVM address encoded to bytes. This value is trusted as it is obtained through the onRamp. It can be // relied upon by the destination pool to validate the source pool. bytes sourcePoolAddress; - address destTokenAddress; // ───╮ Address of destination token - uint32 destGasAmount; //────────╯ The amount of gas available for the releaseOrMint and transfer calls on the offRamp. + address destTokenAddress; // ─╮ Address of destination token + uint32 destGasAmount; //──────╯ The amount of gas available for the releaseOrMint and transfer calls on the offRamp. // Optional pool data to be transferred to the destination chain. Be default this is capped at // CCIP_LOCK_OR_BURN_V1_RET_BYTES bytes. If more data is required, the TokenTransferFeeConfig.destBytesOverhead // has to be set for the specific token. diff --git a/contracts/src/v0.8/ccip/offRamp/OffRamp.sol b/contracts/src/v0.8/ccip/offRamp/OffRamp.sol index c34e4995cad..da9eac80282 100644 --- a/contracts/src/v0.8/ccip/offRamp/OffRamp.sol +++ b/contracts/src/v0.8/ccip/offRamp/OffRamp.sol @@ -22,13 +22,11 @@ import {IERC20} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/token/ import {ERC165Checker} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/introspection/ERC165Checker.sol"; import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol"; -/// @notice OffRamp enables OCR networks to execute multiple messages -/// in an OffRamp in a single transaction. -/// @dev The OnRamp and OffRamp form an xchain upgradeable unit. Any change to one of them -/// results an onchain upgrade of both contracts. -/// @dev MultiOCR3Base is used to store multiple OCR configs for the OffRamp. -/// The execution plugin type has to be configured without signature verification, and the commit -/// plugin type with verification. +/// @notice OffRamp enables OCR networks to execute multiple messages in an OffRamp in a single transaction. +/// @dev The OnRamp and OffRamp form a cross chain upgradeable unit. Any change to one of them results an +/// onchain upgrade of both contracts. +/// @dev MultiOCR3Base is used to store multiple OCR configs for the OffRamp. The execution plugin type has to be +/// configured without signature verification, and the commit plugin type with verification. contract OffRamp is ITypeAndVersion, MultiOCR3Base { using ERC165Checker for address; using EnumerableSet for EnumerableSet.UintSet; @@ -90,80 +88,78 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { event RootRemoved(bytes32 root); event SkippedReportExecution(uint64 sourceChainSelector); - /// @dev Struct that contains the static configuration + /// @dev Struct that contains the static configuration. The individual components are stored as immutable variables. /// @dev RMN depends on this struct, if changing, please notify the RMN maintainers. - /// @dev not sure why solhint complains about this, seems like a buggy detector - /// https://github.com/protofire/solhint/issues/597 // solhint-disable-next-line gas-struct-packing struct StaticConfig { - uint64 chainSelector; // ───╮ Destination chainSelector - IRMNRemote rmnRemote; // ───╯ RMN Verification Contract + uint64 chainSelector; // ────╮ Destination chainSelector + IRMNRemote rmnRemote; // ────╯ RMN Verification Contract address tokenAdminRegistry; // Token admin registry address address nonceManager; // Nonce manager address } - /// @dev Per-chain source config (defining a lane from a Source Chain -> Dest OffRamp) + /// @dev Per-chain source config (defining a lane from a Source Chain -> Dest OffRamp). struct SourceChainConfig { - IRouter router; // ──────────╮ Local router to use for messages coming from this source chain - bool isEnabled; // | Flag whether the source chain is enabled or not - uint64 minSeqNr; // ─────────╯ The min sequence number expected for future messages - bytes onRamp; // OnRamp address on the source chain + IRouter router; // ───╮ Local router to use for messages coming from this source chain. + bool isEnabled; // | Flag whether the source chain is enabled or not. + uint64 minSeqNr; // ──╯ The min sequence number expected for future messages. + bytes onRamp; // OnRamp address on the source chain. } /// @dev Same as SourceChainConfig but with source chain selector so that an array of these /// can be passed in the constructor and the applySourceChainConfigUpdates function. struct SourceChainConfigArgs { - IRouter router; // ────────────────╮ Local router to use for messages coming from this source chain - uint64 sourceChainSelector; // | Source chain selector of the config to update - bool isEnabled; // ────────────────╯ Flag whether the source chain is enabled or not - bytes onRamp; // OnRamp address on the source chain + IRouter router; // ────────────╮ Local router to use for messages coming from this source chain. + uint64 sourceChainSelector; // | Source chain selector of the config to update. + bool isEnabled; // ────────────╯ Flag whether the source chain is enabled or not. + bytes onRamp; // OnRamp address on the source chain. } - /// @dev Dynamic offRamp config - /// @dev Since DynamicConfig is part of DynamicConfigSet event, if changing it, we should update the ABI on Atlas + /// @dev Dynamic offRamp config. + /// @dev Since DynamicConfig is part of DynamicConfigSet event, if changing it, we should update the ABI on Atlas. struct DynamicConfig { - address feeQuoter; // ──────────────────────────────╮ FeeQuoter address on the local chain - uint32 permissionLessExecutionThresholdSeconds; // | Waiting time before manual execution is enabled - bool isRMNVerificationDisabled; // ─────────────────╯ Flag whether the RMN verification is disabled or not - address messageInterceptor; // Optional message interceptor to validate incoming messages (zero address = no interceptor) + address feeQuoter; // ─────────────────────────────╮ FeeQuoter address on the local chain. + uint32 permissionLessExecutionThresholdSeconds; // | Waiting time before manual execution is enabled. + bool isRMNVerificationDisabled; // ────────────────╯ Flag whether the RMN verification is disabled or not. + address messageInterceptor; // Optional, validates incoming messages (zero address = no interceptor). } - /// @dev Report that is committed by the observing DON at the committing phase + /// @dev Report that is committed by the observing DON at the committing phase. /// @dev RMN depends on this struct, if changing, please notify the RMN maintainers. struct CommitReport { - Internal.PriceUpdates priceUpdates; // Collection of gas and price updates to commit - Internal.MerkleRoot[] merkleRoots; // Collection of merkle roots per source chain to commit - IRMNRemote.Signature[] rmnSignatures; // RMN signatures on the merkle roots - uint256 rmnRawVs; // Raw v values of the RMN signatures + Internal.PriceUpdates priceUpdates; // Collection of gas and price updates to commit. + Internal.MerkleRoot[] merkleRoots; // Collection of merkle roots per source chain to commit. + IRMNRemote.Signature[] rmnSignatures; // RMN signatures on the merkle roots. + uint256 rmnRawVs; // Raw v values of the RMN signatures. } + /// @dev Both receiverExecutionGasLimit and tokenGasOverrides are optional. To indicate no override, set the value + /// to 0. The length of tokenGasOverrides must match the length of tokenAmounts, even if it only contains zeros. struct GasLimitOverride { - // A value of zero in both fields signifies no override and allows the corresponding field to be overridden as valid uint256 receiverExecutionGasLimit; // Overrides EVM2EVMMessage.gasLimit. uint32[] tokenGasOverrides; // Overrides EVM2EVMMessage.sourceTokenData.destGasAmount, length must be same as tokenAmounts. } // STATIC CONFIG string public constant override typeAndVersion = "OffRamp 1.6.0-dev"; - /// @dev Hash of encoded address(0) used for empty address checks + /// @dev Hash of encoded address(0) used for empty address checks. bytes32 internal constant EMPTY_ENCODED_ADDRESS_HASH = keccak256(abi.encode(address(0))); - /// @dev ChainSelector of this chain + /// @dev ChainSelector of this chain. uint64 internal immutable i_chainSelector; - /// @dev The RMN verification contract + /// @dev The RMN verification contract. IRMNRemote internal immutable i_rmnRemote; - /// @dev The address of the token admin registry + /// @dev The address of the token admin registry. address internal immutable i_tokenAdminRegistry; - /// @dev The address of the nonce manager + /// @dev The address of the nonce manager. address internal immutable i_nonceManager; // DYNAMIC CONFIG DynamicConfig internal s_dynamicConfig; - /// @notice Set of source chain selectors + /// @notice Set of source chain selectors. EnumerableSet.UintSet internal s_sourceChainSelectors; - /// @notice SourceChainConfig per chain - /// (forms lane configurations from sourceChainSelector => StaticConfig.chainSelector) + /// @notice SourceChainConfig per source chain selector. mapping(uint64 sourceChainSelector => SourceChainConfig sourceChainConfig) private s_sourceChainConfigs; // STATE @@ -173,9 +169,9 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { mapping(uint64 sourceChainSelector => mapping(uint64 seqNum => uint256 executionStateBitmap)) internal s_executionStates; - /// @notice Commit timestamp of merkle roots per source chain + /// @notice Commit timestamp of merkle roots per source chain. mapping(uint64 sourceChainSelector => mapping(bytes32 merkleRoot => uint256 timestamp)) internal s_roots; - /// @dev The sequence number of the last price update + /// @dev The sequence number of the last price update. uint64 private s_latestPriceSequenceNumber; constructor( @@ -208,13 +204,13 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { // │ Execution │ // ================================================================ - // The size of the execution state in bits + // The size of the execution state in bits. uint256 private constant MESSAGE_EXECUTION_STATE_BIT_WIDTH = 2; - // The mask for the execution state bits + // The mask for the execution state bits. uint256 private constant MESSAGE_EXECUTION_STATE_MASK = (1 << MESSAGE_EXECUTION_STATE_BIT_WIDTH) - 1; /// @notice Returns the current execution state of a message based on its sequenceNumber. - /// @param sourceChainSelector The source chain to get the execution state for + /// @param sourceChainSelector The source chain to get the execution state for. /// @param sequenceNumber The sequence number of the message to get the execution state for. /// @return executionState The current execution state of the message. /// @dev We use the literal number 128 because using a constant increased gas usage. @@ -231,7 +227,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { } /// @notice Sets a new execution state for a given sequence number. It will overwrite any existing state. - /// @param sourceChainSelector The source chain to set the execution state for + /// @param sourceChainSelector The source chain to set the execution state for. /// @param sequenceNumber The sequence number for which the state will be saved. /// @param newState The new value the state will be in after this function is called. /// @dev We use the literal number 128 because using a constant increased gas usage. @@ -245,15 +241,16 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { // To unset any potential existing state we zero the bits of the section the state occupies, // then we do an AND operation to blank out any existing state for the section. bitmap &= ~(MESSAGE_EXECUTION_STATE_MASK << offset); - // Set the new state + // Set the new state. bitmap |= uint256(newState) << offset; s_executionStates[sourceChainSelector][sequenceNumber / 128] = bitmap; } - /// @param sourceChainSelector remote source chain selector to get sequence number bitmap for - /// @param sequenceNumber sequence number to get bitmap for - /// @return bitmap Bitmap of the given sequence number for the provided source chain selector. One bitmap represents 128 sequence numbers + /// @param sourceChainSelector remote source chain selector to get sequence number bitmap for. + /// @param sequenceNumber sequence number to get bitmap for. + /// @return bitmap Bitmap of the given sequence number for the provided source chain selector. One bitmap represents + /// 128 sequence numbers. function _getSequenceNumberBitmap( uint64 sourceChainSelector, uint64 sequenceNumber @@ -262,13 +259,13 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { } /// @notice Manually executes a set of reports. - /// @param reports Internal.ExecutionReportSingleChain[] - list of reports to execute - /// @param gasLimitOverrides New gasLimit for each message per report - // The outer array represents each report, inner array represents each message in the report. - // i.e. gasLimitOverrides[report1][report1Message1] -> access message1 from report1 - /// @dev We permit gas limit overrides so that users may manually execute messages which failed due to - /// insufficient gas provided. - /// The reports do not have to contain all the messages (they can be omitted). Multiple reports can be passed in simultaneously. + /// @param reports Internal.ExecutionReportSingleChain[] - list of reports to execute. + /// @param gasLimitOverrides New gasLimit for each message per report. The outer array represents each report, the + // inner array represents each message in the report. + // i.e. gasLimitOverrides[report1][report1Message1] -> access message1 from report1 + /// @dev We permit gas limit overrides so that users may manually execute messages which failed due to insufficient + /// gas provided. The reports do not have to contain all the messages (they can be omitted). Multiple reports can be + /// passed in simultaneously. function manuallyExecute( Internal.ExecutionReport[] memory reports, GasLimitOverride[][] memory gasLimitOverrides @@ -284,13 +281,16 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { uint256 numMsgs = report.messages.length; GasLimitOverride[] memory msgGasLimitOverrides = gasLimitOverrides[reportIndex]; + + // Gas override values need to be provided, even when no override is desired. We expect an array of the correct + // size with all `0` values if no override is desired. if (numMsgs != msgGasLimitOverrides.length) revert ManualExecutionGasLimitMismatch(); for (uint256 msgIndex = 0; msgIndex < numMsgs; ++msgIndex) { uint256 newLimit = msgGasLimitOverrides[msgIndex].receiverExecutionGasLimit; - // Checks to ensure message cannot be executed with less gas than specified. Internal.Any2EVMRampMessage memory message = report.messages[msgIndex]; if (newLimit != 0) { + // Checks to ensure messages will not be executed with less gas than specified. if (newLimit < message.gasLimit) { revert InvalidManualExecutionGasLimit(report.sourceChainSelector, message.header.messageId, newLimit); } @@ -320,9 +320,9 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { _batchExecute(reports, gasLimitOverrides); } - /// @notice Transmit function for execution reports. The function takes no signatures, - /// and expects the exec plugin type to be configured with no signatures. - /// @param report serialized execution report + /// @notice Transmit function for execution reports. The function takes no signatures, and expects the exec plugin + /// type to be configured with no signatures. + /// @param report serialized execution report. function execute(bytes32[3] calldata reportContext, bytes calldata report) external { _batchExecute(abi.decode(report, (Internal.ExecutionReport[])), new GasLimitOverride[][](0)); @@ -330,12 +330,12 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { _transmit(uint8(Internal.OCRPluginType.Execution), reportContext, report, emptySigs, emptySigs, bytes32("")); } - /// @notice Batch executes a set of reports, each report matching one single source chain - /// @param reports Set of execution reports (one per chain) containing the messages and proofs - /// @param manualExecGasLimits An array of gas limits to use for manual execution - // The outer array represents each report, inner array represents each message in the report. - // i.e. gasLimitOverrides[report1][report1Message1] -> access message1 from report1 - /// @dev The manualExecGasLimits array should either be empty, or match the length of the reports array + /// @notice Batch executes a set of reports, each report matching one single source chain. + /// @param reports Set of execution reports (one per chain) containing the messages and proofs. + /// @param manualExecGasLimits An array of gas limits to use for manual execution The outer array represents each + // report, the inner array represents each message in the report. + // i.e. gasLimitOverrides[report1][report1Message1] -> access message1 from report1. + /// @dev The manualExecGasLimits array should either be empty, or match the length of the reports array. /// @dev If called from manual execution, each inner array's length has to match the number of messages. function _batchExecute( Internal.ExecutionReport[] memory reports, @@ -344,7 +344,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { if (reports.length == 0) revert EmptyBatch(); bool areManualGasLimitsEmpty = manualExecGasOverrides.length == 0; - // Cache array for gas savings in the loop's condition + // Cache array for gas savings in the loop's condition. GasLimitOverride[] memory emptyGasLimits = new GasLimitOverride[](0); for (uint256 i = 0; i < reports.length; ++i) { @@ -365,76 +365,75 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { bool manualExecution = manualExecGasExecOverrides.length != 0; if (i_rmnRemote.isCursed(bytes16(uint128(sourceChainSelector)))) { if (manualExecution) { - // For manual execution we don't want to silently fail so we revert + // For manual execution we don't want to silently fail so we revert. revert CursedByRMN(sourceChainSelector); } - // For DON execution we do not revert as a single lane curse can revert the entire batch + // For DON execution we do not revert as a single lane curse can revert the entire batch. emit SkippedReportExecution(sourceChainSelector); return; } - bytes memory onRamp = _getEnabledSourceChainConfig(sourceChainSelector).onRamp; - uint256 numMsgs = report.messages.length; if (numMsgs == 0) revert EmptyReport(report.sourceChainSelector); if (numMsgs != report.offchainTokenData.length) revert UnexpectedTokenData(); bytes32[] memory hashedLeaves = new bytes32[](numMsgs); - for (uint256 i = 0; i < numMsgs; ++i) { - Internal.Any2EVMRampMessage memory message = report.messages[i]; - - // Commits do not verify the destChainSelector in the message, since only the root is committed, - // so we have to check it explicitly - if (message.header.destChainSelector != i_chainSelector) { - revert InvalidMessageDestChainSelector(message.header.destChainSelector); - } - // If the message source chain selector does not match the report's source chain selector and - // the root has not been committed for the report source chain selector, this will be caught by the root verification. - // This acts as an extra check. - if (message.header.sourceChainSelector != sourceChainSelector) { - revert SourceChainSelectorMismatch(sourceChainSelector, message.header.sourceChainSelector); - } - - // We do this hash here instead of in _verify to avoid two separate loops - // over the same data, which increases gas cost. - // Hashing all of the message fields ensures that the message being executed is correct and not tampered with. - // Including the known OnRamp ensures that the message originates from the correct on ramp version - hashedLeaves[i] = Internal._hash( - message, - keccak256( - abi.encode( - Internal.ANY_2_EVM_MESSAGE_HASH, - message.header.sourceChainSelector, - message.header.destChainSelector, - keccak256(onRamp) - ) + { + // We do this hash here instead of in _verify to avoid two separate loops over the same data. Hashing all of the + // message fields ensures that the message being executed is correct and not tampered with. Including the known + // OnRamp ensures that the message originates from the correct on ramp version. We know the sourceChainSelector + // and i_destChainSelector are correct because we revert below when they are not. + bytes32 metaDataHash = keccak256( + abi.encode( + Internal.ANY_2_EVM_MESSAGE_HASH, + sourceChainSelector, + i_chainSelector, + keccak256(_getEnabledSourceChainConfig(sourceChainSelector).onRamp) ) ); + + for (uint256 i = 0; i < numMsgs; ++i) { + Internal.Any2EVMRampMessage memory message = report.messages[i]; + + // Commits do not verify the destChainSelector in the message since only the root is committed, so we + // have to check it explicitly. This check is also important as we have assumed the metaDataHash above uses + // the i_chainSelector as the destChainSelector. + if (message.header.destChainSelector != i_chainSelector) { + revert InvalidMessageDestChainSelector(message.header.destChainSelector); + } + // If the message source chain selector does not match the report's source chain selector and the root has not + // been committed for the report source chain selector this will be caught by the root verification. + // This acts as an extra check to ensure the message source chain selector matches the report's source chain. + if (message.header.sourceChainSelector != sourceChainSelector) { + revert SourceChainSelectorMismatch(sourceChainSelector, message.header.sourceChainSelector); + } + + hashedLeaves[i] = Internal._hash(message, metaDataHash); + } } - // SECURITY CRITICAL CHECK - // NOTE: This check also verifies that all messages match the report's sourceChainSelector + // SECURITY CRITICAL CHECK. uint256 timestampCommitted = _verify(sourceChainSelector, hashedLeaves, report.proofs, report.proofFlagBits); if (timestampCommitted == 0) revert RootNotCommitted(sourceChainSelector); - // Execute messages + // Execute messages. for (uint256 i = 0; i < numMsgs; ++i) { uint256 gasStart = gasleft(); Internal.Any2EVMRampMessage memory message = report.messages[i]; Internal.MessageExecutionState originalState = getExecutionState(sourceChainSelector, message.header.sequenceNumber); - // Two valid cases here, we either have never touched this message before, or we tried to execute - // and failed. This check protects against reentry and re-execution because the other state is - // IN_PROGRESS which should not be allowed to execute. + // Two valid cases here, we either have never touched this message before, or we tried to execute and failed. This + // check protects against reentry and re-execution because the other state is IN_PROGRESS which should not be + // allowed to execute. if ( !( originalState == Internal.MessageExecutionState.UNTOUCHED || originalState == Internal.MessageExecutionState.FAILURE ) ) { - // If the message has already been executed, we skip it. We want to not revert on race conditions between + // If the message has already been executed, we skip it. We want to not revert on race conditions between // executing parties. This will allow us to open up manual exec while also attempting with the DON, without // reverting an entire DON batch when a user manually executes while the tx is inflight. emit SkippedAlreadyExecutedMessage(sourceChainSelector, message.header.sequenceNumber); @@ -445,8 +444,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { tokenGasOverrides = manualExecGasExecOverrides[i].tokenGasOverrides; bool isOldCommitReport = (block.timestamp - timestampCommitted) > s_dynamicConfig.permissionLessExecutionThresholdSeconds; - // Manually execution is fine if we previously failed or if the commit report is just too old - // Acceptable state transitions: UNTOUCHED->SUCCESS, UNTOUCHED->FAILURE, FAILURE->SUCCESS + // Manually execution is fine if we previously failed or if the commit report is just too old. + // Acceptable state transitions: UNTOUCHED->SUCCESS, UNTOUCHED->FAILURE, FAILURE->SUCCESS. if (!(isOldCommitReport || originalState == Internal.MessageExecutionState.FAILURE)) { revert ManualExecutionNotYetEnabled(sourceChainSelector); } @@ -456,8 +455,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { message.gasLimit = manualExecGasExecOverrides[i].receiverExecutionGasLimit; } } else { - // DON can only execute a message once - // Acceptable state transitions: UNTOUCHED->SUCCESS, UNTOUCHED->FAILURE + // DON can only execute a message once. + // Acceptable state transitions: UNTOUCHED->SUCCESS, UNTOUCHED->FAILURE. if (originalState != Internal.MessageExecutionState.UNTOUCHED) { emit AlreadyAttempted(sourceChainSelector, message.header.sequenceNumber); continue; @@ -465,14 +464,14 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { } // Nonce changes per state transition (these only apply for ordered messages): - // UNTOUCHED -> FAILURE nonce bump - // UNTOUCHED -> SUCCESS nonce bump - // FAILURE -> SUCCESS no nonce bump - // UNTOUCHED messages MUST be executed in order always - // If nonce == 0 then out of order execution is allowed + // UNTOUCHED -> FAILURE nonce bump. + // UNTOUCHED -> SUCCESS nonce bump. + // FAILURE -> SUCCESS no nonce bump. + // UNTOUCHED messages MUST be executed in order always. + // If nonce == 0 then out of order execution is allowed. if (message.header.nonce != 0) { if (originalState == Internal.MessageExecutionState.UNTOUCHED) { - // If a nonce is not incremented, that means it was skipped, and we can ignore the message + // If a nonce is not incremented, that means it was skipped, and we can ignore the message. if ( !INonceManager(i_nonceManager).incrementInboundNonce( sourceChainSelector, message.header.nonce, message.sender @@ -481,8 +480,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { } } - // Although we expect only valid messages will be committed, we check again - // when executing as a defense in depth measure. + // We expect only valid messages will be committed but we check when executing as a defense in depth measure. bytes[] memory offchainTokenData = report.offchainTokenData[i]; if (message.tokenAmounts.length != offchainTokenData.length) { revert TokenDataMismatch(sourceChainSelector, message.header.sequenceNumber); @@ -493,8 +491,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { _trialExecute(message, offchainTokenData, tokenGasOverrides); _setExecutionState(sourceChainSelector, message.header.sequenceNumber, newState); - // Since it's hard to estimate whether manual execution will succeed, we revert the entire transaction - // if it fails. This will show the user if their manual exec will fail before they submit it. + // Since it's hard to estimate whether manual execution will succeed, we revert the entire transaction if it + // fails. This will show the user if their manual exec will fail before they submit it. if (manualExecution) { if (newState == Internal.MessageExecutionState.FAILURE) { if (originalState != Internal.MessageExecutionState.UNTOUCHED) { @@ -505,8 +503,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { } } - // The only valid prior states are UNTOUCHED and FAILURE (checked above) - // The only valid post states are FAILURE and SUCCESS (checked below) + // The only valid prior states are UNTOUCHED and FAILURE (checked above). + // The only valid post states are FAILURE and SUCCESS (checked below). if (newState != Internal.MessageExecutionState.SUCCESS) { if (newState != Internal.MessageExecutionState.FAILURE) { revert InvalidNewState(sourceChainSelector, message.header.sequenceNumber, newState); @@ -539,8 +537,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { ) internal returns (Internal.MessageExecutionState executionState, bytes memory) { try this.executeSingleMessage(message, offchainTokenData, tokenGasOverrides) {} catch (bytes memory err) { - // return the message execution state as FAILURE and the revert data - // Max length of revert data is Router.MAX_RET_BYTES, max length of err is 4 + Router.MAX_RET_BYTES + // return the message execution state as FAILURE and the revert data. + // Max length of revert data is Router.MAX_RET_BYTES, max length of err is 4 + Router.MAX_RET_BYTES. return (Internal.MessageExecutionState.FAILURE, err); } // If message execution succeeded, no CCIP receiver return data is expected, return with empty bytes. @@ -552,8 +550,8 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { /// @param offchainTokenData Token transfer data to be passed to TokenPool. /// @dev We make this external and callable by the contract itself, in order to try/catch /// its execution and enforce atomicity among successful message processing and token transfer. - /// @dev We use ERC-165 to check for the ccipReceive interface to permit sending tokens to contracts - /// (for example smart contract wallets) without an associated message. + /// @dev We use ERC-165 to check for the ccipReceive interface to permit sending tokens to contracts, for example + /// smart contract wallets, without an associated message. function executeSingleMessage( Internal.Any2EVMRampMessage memory message, bytes[] calldata offchainTokenData, @@ -605,7 +603,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); - // If CCIP receiver execution is not successful, revert the call including token transfers + // If CCIP receiver execution is not successful, revert the call including token transfers. if (!success) revert ReceiverError(returnData); } @@ -616,14 +614,14 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { /// @notice Uses a pool to release or mint a token to a receiver address, with balance checks before and after the /// transfer. This is done to ensure the exact number of tokens the pool claims to release are actually transferred. /// @dev The local token address is validated through the TokenAdminRegistry. If, due to some misconfiguration, the - /// token is unknown to the registry, the offRamp will revert. The tx, and the tokens, can be retrieved by - /// registering the token on this chain, and re-trying the msg. + /// token is unknown to the registry, the offRamp will revert. The tx, and the tokens, can be retrieved by registering + /// the token on this chain, and re-trying the msg. /// @param sourceTokenAmount Amount and source data of the token to be released/minted. /// @param originalSender The message sender on the source chain. /// @param receiver The address that will receive the tokens. /// @param sourceChainSelector The remote source chain selector /// @param offchainTokenData Data fetched offchain by the DON. - /// @return destTokenAmount local token address with amount + /// @return destTokenAmount local token address with amount. function _releaseOrMintSingleToken( Internal.Any2EVMTokenTransfer memory sourceTokenAmount, bytes memory originalSender, @@ -631,15 +629,14 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { uint64 sourceChainSelector, bytes memory offchainTokenData ) internal returns (Client.EVMTokenAmount memory destTokenAmount) { - // We need to safely decode the token address from the sourceTokenData, as it could be wrong, - // in which case it doesn't have to be a valid EVM address. - // We assume this destTokenAddress has already been fully validated by a (trusted) OnRamp. + // We need to safely decode the token address from the sourceTokenData as it could be wrong, in which case it + // doesn't have to be a valid EVM address. address localToken = sourceTokenAmount.destTokenAddress; // We check with the token admin registry if the token has a pool on this chain. address localPoolAddress = ITokenAdminRegistry(i_tokenAdminRegistry).getPool(localToken); // This will call the supportsInterface through the ERC165Checker, and not directly on the pool address. // This is done to prevent a pool from reverting the entire transaction if it doesn't support the interface. - // The call gets a max or 30k gas per instance, of which there are three. This means gas estimations should + // The call gets a max or 30k gas per instance, of which there are three. This means offchain gas estimations should // account for 90k gas overhead due to the interface check. if (localPoolAddress == address(0) || !localPoolAddress.supportsInterface(Pool.CCIP_POOL_V1)) { revert NotACompatiblePool(localPoolAddress); @@ -648,11 +645,10 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { // We retrieve the local token balance of the receiver before the pool call. (uint256 balancePre, uint256 gasLeft) = _getBalanceOfReceiver(receiver, localToken, sourceTokenAmount.destGasAmount); - // We determined that the pool address is a valid EVM address, but that does not mean the code at this - // address is a (compatible) pool contract. _callWithExactGasSafeReturnData will check if the location - // contains a contract. If it doesn't it reverts with a known error, which we catch gracefully. - // We call the pool with exact gas to increase resistance against malicious tokens or token pools. - // We protect against return data bombs by capping the return data size at MAX_RET_BYTES. + // We determined that the pool address is a valid EVM address, but that does not mean the code at this address is a + // (compatible) pool contract. _callWithExactGasSafeReturnData will check if the location contains a contract. If it + // doesn't it reverts with a known error. We call the pool with exact gas to increase resistance against malicious + // tokens or token pools. We protect against return data bombs by capping the return data size at MAX_RET_BYTES. (bool success, bytes memory returnData, uint256 gasUsedReleaseOrMint) = CallWithExactGas ._callWithExactGasSafeReturnData( abi.encodeCall( @@ -674,21 +670,22 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { Internal.MAX_RET_BYTES ); - // Wrap and rethrow the error so we can catch it lower in the stack + // Wrap and rethrow the error so we can catch it lower in the stack. if (!success) revert TokenHandlingError(returnData); - // If the call was successful, the returnData should be the amount released or minted denominated in the local token's decimals. + // If the call was successful, the returnData should be the amount released or minted denominated in the local + // token's decimals. if (returnData.length != Pool.CCIP_POOL_V1_RET_BYTES) { revert InvalidDataLength(Pool.CCIP_POOL_V1_RET_BYTES, returnData.length); } - uint256 localAmount = abi.decode(returnData, (uint256)); + // We don't need to do balance checks if the pool is the receiver, as they would always fail in the case // of a lockRelease pool. if (receiver != localPoolAddress) { (uint256 balancePost,) = _getBalanceOfReceiver(receiver, localToken, gasLeft - gasUsedReleaseOrMint); - // First we check if the subtraction would result in an underflow to ensure we revert with a clear error + // First we check if the subtraction would result in an underflow to ensure we revert with a clear error. if (balancePost < balancePre || balancePost - balancePre != localAmount) { revert ReleaseOrMintBalanceMismatch(localAmount, balancePre, balancePost); } @@ -735,10 +732,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { /// @param offchainTokenData Array of token data fetched offchain by the DON. /// @param tokenGasOverrides Array of override gas limits to use for token transfers. If empty, the normal gas limit /// as defined on the source chain is used. - /// @return destTokenAmounts local token addresses with amounts - /// @dev This function wraps the token pool call in a try catch block to gracefully handle - /// any non-rate limiting errors that may occur. If we encounter a rate limiting related error - /// we bubble it up. If we encounter a non-rate limiting error we wrap it in a TokenHandlingError. + /// @return destTokenAmounts local token addresses with amounts. function _releaseOrMintTokens( Internal.Any2EVMTokenTransfer[] memory sourceTokenAmounts, bytes memory originalSender, @@ -769,17 +763,16 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { /// @notice Transmit function for commit reports. The function requires signatures, /// and expects the commit plugin type to be configured with signatures. - /// @param report serialized commit report + /// @param report serialized commit report. /// @dev A commitReport can have two distinct parts (batched together to amortize the cost of checking sigs): /// 1. Price updates /// 2. A batch of merkle root and sequence number intervals (per-source) - /// Both have their own, separate, staleness checks, with price updates using the epoch and round - /// number of the latest price update. The merkle root checks for staleness are based on the seqNums. - /// They need to be separate because a price report for round t+2 might be included before a report - /// containing a merkle root for round t+1. This merkle root report for round t+1 is still valid - /// and should not be rejected. When a report with a stale root but valid price updates is submitted, - /// we are OK to revert to preserve the invariant that we always revert on invalid sequence number ranges. - /// If that happens, prices will be updated in later rounds. + /// Both have their own, separate, staleness checks, with price updates using the epoch and round number of the latest + /// price update. The merkle root checks for staleness are based on the seqNums. They need to be separate because + /// a price report for round t+2 might be included before a report containing a merkle root for round t+1. This merkle + /// root report for round t+1 is still valid and should not be rejected. When a report with a stale root but valid + /// price updates is submitted, we are OK to revert to preserve the invariant that we always revert on invalid + /// sequence number ranges. If that happens, prices will be updated in later rounds. function commit( bytes32[3] calldata reportContext, bytes calldata report, @@ -983,7 +976,7 @@ contract OffRamp is ITypeAndVersion, MultiOCR3Base { currentConfig.minSeqNr = 1; emit SourceChainSelectorAdded(sourceChainSelector); } else { - if (currentConfig.minSeqNr != 1) { + if (currentConfig.minSeqNr != 1 && keccak256(currentConfig.onRamp) != keccak256(newOnRamp)) { // OnRamp updates should only happens due to a misconfiguration // If an OnRamp is misconfigured, no reports should have been committed and no messages should have been executed // This is enforced by the onRamp address check in the commit function diff --git a/contracts/src/v0.8/ccip/onRamp/OnRamp.sol b/contracts/src/v0.8/ccip/onRamp/OnRamp.sol index 51997129036..967d1df842b 100644 --- a/contracts/src/v0.8/ccip/onRamp/OnRamp.sol +++ b/contracts/src/v0.8/ccip/onRamp/OnRamp.sol @@ -22,8 +22,8 @@ import {SafeERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/tok import {EnumerableSet} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/structs/EnumerableSet.sol"; /// @notice The OnRamp is a contract that handles lane-specific fee logic. -/// @dev The OnRamp and OffRamp form an xchain upgradeable unit. Any change to one of them -/// results in an onchain upgrade of all 3. +/// @dev The OnRamp and OffRamp form a cross chain upgradeable unit. Any change to one of them results in an onchain +/// upgrade of both contracts. contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { using SafeERC20 for IERC20; using EnumerableSet for EnumerableSet.AddressSet; @@ -55,7 +55,7 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { event AllowListSendersAdded(uint64 indexed destChainSelector, address[] senders); event AllowListSendersRemoved(uint64 indexed destChainSelector, address[] senders); - /// @dev Struct that contains the static configuration + /// @dev Struct that contains the static configuration. /// RMN depends on this struct, if changing, please notify the RMN maintainers. // solhint-disable-next-line gas-struct-packing struct StaticConfig { @@ -68,20 +68,19 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { /// @dev Struct that contains the dynamic configuration // solhint-disable-next-line gas-struct-packing struct DynamicConfig { - address feeQuoter; // FeeQuoter address - bool reentrancyGuardEntered; // Reentrancy protection - address messageInterceptor; // Optional message interceptor to validate outbound messages. Zero address = no interceptor - address feeAggregator; // Fee aggregator address - address allowlistAdmin; // authorized admin to add or remove allowed senders + address feeQuoter; // FeeQuoter address. + bool reentrancyGuardEntered; // Reentrancy protection. + address messageInterceptor; // Optional message interceptor to validate messages. Zero address = no interceptor. + address feeAggregator; // Fee aggregator address. + address allowlistAdmin; // authorized admin to add or remove allowed senders. } - /// @dev Struct to hold the configs for a destination chain - /// @dev sequenceNumber, allowlistEnabled, router will all be packed in 1 slot + /// @dev Struct to hold the configs for a single destination chain struct DestChainConfig { // The last used sequence number. This is zero in the case where no messages have yet been sent. - // 0 is not a valid sequence number for any real transaction. + // 0 is not a valid sequence number for any real transaction as this value will be incremented before use. uint64 sequenceNumber; // ──╮ The last used sequence number. - bool allowlistEnabled; // │ boolean indicator to specify if allowlist check is enabled. + bool allowlistEnabled; // │ True if the allowlist is enabled. IRouter router; // ─────────╯ Local router address that is allowed to send messages to the destination chain. EnumerableSet.AddressSet allowedSendersList; // The list of addresses allowed to send messages. } @@ -90,35 +89,35 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { /// can be passed in the constructor and the applyDestChainConfigUpdates function // solhint-disable gas-struct-packing struct DestChainConfigArgs { - uint64 destChainSelector; // ─╮ Destination chain selector - IRouter router; // │ Source router address - bool allowlistEnabled; // ────╯ Boolean indicator to specify if allowlist check is enabled + uint64 destChainSelector; // ─╮ Destination chain selector. + IRouter router; // │ Source router address. + bool allowlistEnabled; // ────╯ True if the allowlist is enabled. } /// @dev Struct to hold the allowlist configuration args per dest chain. struct AllowlistConfigArgs { uint64 destChainSelector; // ──╮ Destination chain selector. bool allowlistEnabled; // ─────╯ True if the allowlist is enabled. - address[] addedAllowlistedSenders; // list of senders to be added to the allowedSendersList - address[] removedAllowlistedSenders; // list of senders to be removed from the allowedSendersList + address[] addedAllowlistedSenders; // list of senders to be added to the allowedSendersList. + address[] removedAllowlistedSenders; // list of senders to be removed from the allowedSendersList. } // STATIC CONFIG string public constant override typeAndVersion = "OnRamp 1.6.0-dev"; - /// @dev The chain ID of the source chain that this contract is deployed to + /// @dev The chain ID of the source chain that this contract is deployed to. uint64 private immutable i_chainSelector; - /// @dev The rmn contract + /// @dev The rmn contract. IRMNRemote private immutable i_rmnRemote; - /// @dev The address of the nonce manager + /// @dev The address of the nonce manager. address private immutable i_nonceManager; - /// @dev The address of the token admin registry + /// @dev The address of the token admin registry. address private immutable i_tokenAdminRegistry; // DYNAMIC CONFIG - /// @dev The dynamic config for the onRamp + /// @dev The dynamic config for the onRamp. DynamicConfig private s_dynamicConfig; - /// @dev The destination chain specific configs + /// @dev The destination chain specific configs. mapping(uint64 destChainSelector => DestChainConfig destChainConfig) private s_destChainConfigs; constructor( @@ -146,9 +145,9 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { // │ Messaging │ // ================================================================ - /// @notice Gets the next sequence number to be used in the onRamp - /// @param destChainSelector The destination chain selector - /// @return nextSequenceNumber The next sequence number to be used + /// @notice Gets the next sequence number to be used in the onRamp. + /// @param destChainSelector The destination chain selector. + /// @return nextSequenceNumber The next sequence number to be used. function getExpectedNextSequenceNumber( uint64 destChainSelector ) external view returns (uint64) { @@ -162,16 +161,15 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { uint256 feeTokenAmount, address originalSender ) external returns (bytes32) { - // We rely on a reentrancy guard here due to the untrusted calls performed to the pools - // This enables some optimizations by not following the CEI pattern + // We rely on a reentrancy guard here due to the untrusted calls performed to the pools. This enables some + // optimizations by not following the CEI pattern if (s_dynamicConfig.reentrancyGuardEntered) revert ReentrancyGuardReentrantCall(); - s_dynamicConfig.reentrancyGuardEntered = true; DestChainConfig storage destChainConfig = s_destChainConfigs[destChainSelector]; - // NOTE: assumes the message has already been validated through the getFee call - // Validate message sender is set and allowed. Not validated in `getFee` since it is not user-driven. + // NOTE: assumes the message has already been validated through the getFee call. + // Validate originalSender is set and allowed. Not validated in `getFee` since it is not user-driven. if (originalSender == address(0)) revert RouterMustSetOriginalSender(); if (destChainConfig.allowlistEnabled) { @@ -180,7 +178,7 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { } } - // Router address may be zero intentionally to pause. + // Router address may be zero intentionally to pause, which should stop all messages. if (msg.sender != address(destChainConfig.router)) revert MustBeCalledByRouter(); { @@ -193,14 +191,14 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { Internal.EVM2AnyRampMessage memory newMessage = Internal.EVM2AnyRampMessage({ header: Internal.RampMessageHeader({ - // Should be generated after the message is complete + // Should be generated after the message is complete. messageId: "", sourceChainSelector: i_chainSelector, destChainSelector: destChainSelector, - // We need the next available sequence number so we increment before we use the value + // We need the next available sequence number so we increment before we use the value. sequenceNumber: ++destChainConfig.sequenceNumber, - // Only bump nonce for messages that specify allowOutOfOrderExecution == false. Otherwise, we - // may block ordered message nonces, which is not what we want. + // Only bump nonce for messages that specify allowOutOfOrderExecution == false. Otherwise, we may block ordered + // message nonces, which is not what we want. nonce: 0 }), sender: originalSender, @@ -210,7 +208,7 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { feeToken: message.feeToken, feeTokenAmount: feeTokenAmount, feeValueJuels: 0, // calculated later - // Should be populated via lock / burn pool calls + // Should be populated via lock / burn pool calls. tokenAmounts: new Internal.EVM2AnyTokenTransfer[](message.tokenAmounts.length) }); @@ -221,8 +219,8 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { _lockOrBurnSingleToken(tokenAmounts[i], destChainSelector, message.receiver, originalSender); } - // Convert message fee to juels and retrieve converted args - // Validate pool return data after it is populated (view function - no state changes) + // Convert message fee to juels and retrieve converted args. + // Validate pool return data after it is populated (view function - no state changes). bool isOutOfOrderExecution; bytes memory convertedExtraArgs; bytes[] memory destExecDataPerToken; @@ -241,15 +239,15 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { newMessage.tokenAmounts[i].destExecData = destExecDataPerToken[i]; } - // Hash only after all fields have been set + // Hash only after all fields have been set. newMessage.header.messageId = Internal._hash( newMessage, - // Metadata hash preimage to ensure global uniqueness, ensuring 2 identical messages sent to 2 different - // lanes will have a distinct hash. + // Metadata hash preimage to ensure global uniqueness, ensuring 2 identical messages sent to 2 different lanes + // will have a distinct hash. keccak256(abi.encode(Internal.EVM_2_ANY_MESSAGE_HASH, i_chainSelector, destChainSelector, address(this))) ); - // Emit message request + // Emit message request. // This must happen after any pool events as some tokens (e.g. USDC) emit events that we expect to precede this // event in the offchain code. emit CCIPMessageSent(destChainSelector, newMessage.header.sequenceNumber, newMessage); @@ -259,12 +257,12 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { return newMessage.header.messageId; } - /// @notice Uses a pool to lock or burn a token - /// @param tokenAndAmount Token address and amount to lock or burn - /// @param destChainSelector Target destination chain selector of the message - /// @param receiver Message receiver - /// @param originalSender Message sender - /// @return evm2AnyTokenTransfer EVM2Any token and amount data + /// @notice Uses a pool to lock or burn a token. + /// @param tokenAndAmount Token address and amount to lock or burn. + /// @param destChainSelector Target destination chain selector of the message. + /// @param receiver Message receiver. + /// @param originalSender Message sender. + /// @return evm2AnyTokenTransfer EVM2Any token and amount data. function _lockOrBurnSingleToken( Client.EVMTokenAmount memory tokenAndAmount, uint64 destChainSelector, @@ -291,13 +289,13 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { }) ); - // NOTE: pool data validations are outsourced to the FeeQuoter to handle family-specific logic handling + // NOTE: pool data validations are outsourced to the FeeQuoter to handle family-specific logic handling. return Internal.EVM2AnyTokenTransfer({ sourcePoolAddress: address(sourcePool), destTokenAddress: poolReturnData.destTokenAddress, extraData: poolReturnData.destPoolData, amount: tokenAndAmount.amount, - destExecData: "" // This is set in the processPoolReturnData function + destExecData: "" // This is set in the processPoolReturnData function. }); } @@ -331,15 +329,6 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { _setDynamicConfig(dynamicConfig); } - /// @notice Gets the source router for a destination chain - /// @param destChainSelector The destination chain selector - /// @return router the router for the provided destination chain - function getRouter( - uint64 destChainSelector - ) external view returns (IRouter) { - return s_destChainConfigs[destChainSelector].router; - } - /// @notice Internal version of setDynamicConfig to allow for reuse in the constructor. function _setDynamicConfig( DynamicConfig memory dynamicConfig @@ -393,11 +382,11 @@ contract OnRamp is IEVM2AnyOnRampClient, ITypeAndVersion, OwnerIsCreator { } } - /// @notice get ChainConfig configured for the DestinationChainSelector - /// @param destChainSelector The destination chain selector - /// @return sequenceNumber The last used sequence number - /// @return allowlistEnabled boolean indicator to specify if allowlist check is enabled - /// @return router address of the router + /// @notice get ChainConfig configured for the DestinationChainSelector. + /// @param destChainSelector The destination chain selector. + /// @return sequenceNumber The last used sequence number. + /// @return allowlistEnabled boolean indicator to specify if allowlist check is enabled. + /// @return router address of the router. function getDestChainConfig( uint64 destChainSelector ) external view returns (uint64 sequenceNumber, bool allowlistEnabled, address router) { diff --git a/contracts/src/v0.8/ccip/pools/LockReleaseTokenPool.sol b/contracts/src/v0.8/ccip/pools/LockReleaseTokenPool.sol index 0f77304b186..98c20df30c1 100644 --- a/contracts/src/v0.8/ccip/pools/LockReleaseTokenPool.sol +++ b/contracts/src/v0.8/ccip/pools/LockReleaseTokenPool.sol @@ -9,6 +9,7 @@ import {TokenPool} from "./TokenPool.sol"; import {IERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/IERC20.sol"; import {SafeERC20} from "../../vendor/openzeppelin-solidity/v4.8.3/contracts/token/ERC20/utils/SafeERC20.sol"; +import {IERC165} from "../../vendor/openzeppelin-solidity/v5.0.2/contracts/utils/introspection/IERC165.sol"; /// @notice Token pool used for tokens on their native chain. This uses a lock and release mechanism. /// Because of lock/unlock requiring liquidity, this pool contract also has function to add and remove @@ -69,7 +70,7 @@ contract LockReleaseTokenPool is TokenPool, ILiquidityContainer, ITypeAndVersion return Pool.ReleaseOrMintOutV1({destinationAmount: releaseOrMintIn.amount}); } - // @inheritdoc IERC165 + /// @inheritdoc IERC165 function supportsInterface( bytes4 interfaceId ) public pure virtual override returns (bool) { diff --git a/contracts/src/v0.8/ccip/rmn/RMNHome.sol b/contracts/src/v0.8/ccip/rmn/RMNHome.sol index 61dc6119f28..f9a9f938809 100644 --- a/contracts/src/v0.8/ccip/rmn/RMNHome.sol +++ b/contracts/src/v0.8/ccip/rmn/RMNHome.sol @@ -80,14 +80,13 @@ contract RMNHome is OwnerIsCreator, ITypeAndVersion { } struct SourceChain { - uint64 chainSelector; // ─────╮ The Source chain selector. - uint64 f; // ─────────────────╯ Maximum number of faulty observers; f+1 observers required to agree on an observation for this source chain. - // ObserverNodesBitmap & (1<