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-4476 remove legacy curse check in rmnRemote #15523

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
10 changes: 10 additions & 0 deletions contracts/.changeset/eighty-cycles-film.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
'@chainlink/contracts': patch
---

remove legacy curse check from RMNRemote isCursed() method #bugfix


PR issue: CCIP-4476

Solidity Review issue: CCIP-3966
2 changes: 1 addition & 1 deletion contracts/gas-snapshots/ccip.gas-snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -572,7 +572,7 @@ RMNRemote_constructor:test_constructor() (gas: 8398)
RMNRemote_curse:test_curse_AlreadyCursed_duplicateSubject_reverts() (gas: 154501)
RMNRemote_curse:test_curse_calledByNonOwner_reverts() (gas: 18734)
RMNRemote_curse:test_curse_success() (gas: 149475)
RMNRemote_global_and_legacy_curses:test_global_and_legacy_curses_success() (gas: 133441)
RMNRemote_global_curses:test_isCursed_globalCurseSubject() (gas: 71715)
RMNRemote_isBlessed:test_isBlessed() (gas: 17588)
RMNRemote_setConfig:test_setConfig_ZeroValueNotAllowed_revert() (gas: 37971)
RMNRemote_setConfig:test_setConfig_addSigner_removeSigner_success() (gas: 993448)
Expand Down
9 changes: 2 additions & 7 deletions contracts/src/v0.8/ccip/rmn/RMNRemote.sol
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@ import {Ownable2StepMsgSender} from "../../shared/access/Ownable2StepMsgSender.s
import {EnumerableSet} from "../../shared/enumerable/EnumerableSetWithBytes16.sol";
import {Internal} from "../libraries/Internal.sol";

/// @dev An active curse on this subject will cause isCursed() to return true. Use this subject if there is an issue
/// with a remote chain, for which there exists a legacy lane contract deployed on the same chain as this RMN contract
/// is deployed, relying on isCursed().
bytes16 constant LEGACY_CURSE_SUBJECT = 0x01000000000000000000000000000000;

/// @dev An active curse on this subject will cause isCursed() and isCursed(bytes16) to return true. Use this subject
/// for issues affecting all of CCIP chains, or pertaining to the chain that this contract is deployed on, instead of
/// using the local chain selector as a subject.
Expand Down Expand Up @@ -256,11 +251,11 @@ contract RMNRemote is Ownable2StepMsgSender, ITypeAndVersion, IRMNRemote, IRMN {
/// @inheritdoc IRMNRemote
function isCursed() external view override(IRMN, IRMNRemote) returns (bool) {
// There are zero curses under normal circumstances, which means it's cheaper to check for the absence of curses.
// than to check the subject list twice, as we have to check for both the legacy and global curse subjects.
// than to check the subject list for the global curse subject.
if (s_cursedSubjects.length() == 0) {
return false;
}
return s_cursedSubjects.contains(LEGACY_CURSE_SUBJECT) || s_cursedSubjects.contains(GLOBAL_CURSE_SUBJECT);
return s_cursedSubjects.contains(GLOBAL_CURSE_SUBJECT);
}

/// @inheritdoc IRMNRemote
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
// SPDX-License-Identifier: BUSL-1.1
pragma solidity 0.8.24;

import {GLOBAL_CURSE_SUBJECT, LEGACY_CURSE_SUBJECT} from "../../../rmn/RMNRemote.sol";
import {GLOBAL_CURSE_SUBJECT} from "../../../rmn/RMNRemote.sol";
import {RMNRemoteSetup} from "./RMNRemoteSetup.t.sol";

contract RMNRemote_global_and_legacy_curses is RMNRemoteSetup {
function test_global_and_legacy_curses_success() public {
contract RMNRemote_global_curses is RMNRemoteSetup {
function test_isCursed_globalCurseSubject() public {
bytes16 randSubject = bytes16(keccak256("random subject"));
assertFalse(s_rmnRemote.isCursed());
assertFalse(s_rmnRemote.isCursed(randSubject));
Expand All @@ -17,13 +17,5 @@ contract RMNRemote_global_and_legacy_curses is RMNRemoteSetup {
s_rmnRemote.uncurse(GLOBAL_CURSE_SUBJECT);
assertFalse(s_rmnRemote.isCursed());
assertFalse(s_rmnRemote.isCursed(randSubject));

s_rmnRemote.curse(LEGACY_CURSE_SUBJECT);
assertTrue(s_rmnRemote.isCursed());
assertFalse(s_rmnRemote.isCursed(randSubject)); // legacy curse doesn't affect specific subjects

s_rmnRemote.uncurse(LEGACY_CURSE_SUBJECT);
assertFalse(s_rmnRemote.isCursed());
assertFalse(s_rmnRemote.isCursed(randSubject));
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ registry_module_owner_custom: ../../../contracts/solc/v0.8.24/RegistryModuleOwne
report_codec: ../../../contracts/solc/v0.8.24/ReportCodec/ReportCodec.abi ../../../contracts/solc/v0.8.24/ReportCodec/ReportCodec.bin 6c943b39f003aa67c3cefa19a8ff99e846236a058e1ceae77569c3a065ffd5c7
rmn_home: ../../../contracts/solc/v0.8.24/RMNHome/RMNHome.abi ../../../contracts/solc/v0.8.24/RMNHome/RMNHome.bin 84ca84b3d0c00949905a3d10a91255f877cf32b2a0d7f7f7ce3121ced34a8cb7
rmn_proxy_contract: ../../../contracts/solc/v0.8.24/ARMProxy/ARMProxy.abi ../../../contracts/solc/v0.8.24/ARMProxy/ARMProxy.bin b048d8e752e3c41113ebb305c1efa06737ad36b4907b93e627fb0a3113023454
rmn_remote: ../../../contracts/solc/v0.8.24/RMNRemote/RMNRemote.abi ../../../contracts/solc/v0.8.24/RMNRemote/RMNRemote.bin 941118dfdc6bb042c339cfe8d8e0c7a0b486afb731a785d58a64994e7a13c459
rmn_remote: ../../../contracts/solc/v0.8.24/RMNRemote/RMNRemote.abi ../../../contracts/solc/v0.8.24/RMNRemote/RMNRemote.bin 2bf58225d1ceec21f3dd9e65b8088945c643ec527ae54b9983ec46316f5bca1f
router: ../../../contracts/solc/v0.8.24/Router/Router.abi ../../../contracts/solc/v0.8.24/Router/Router.bin 2e4f0a7826c8abb49d882bb49fc5ff20a186dbd3137624b9097ffed903ae4888
token_admin_registry: ../../../contracts/solc/v0.8.24/TokenAdminRegistry/TokenAdminRegistry.abi ../../../contracts/solc/v0.8.24/TokenAdminRegistry/TokenAdminRegistry.bin 397bc7be08c2848c0f4715f90b16206d6367f78ffb7cd48e2b1dfc0ccc5aea26
token_pool: ../../../contracts/solc/v0.8.24/TokenPool/TokenPool.abi ../../../contracts/solc/v0.8.24/TokenPool/TokenPool.bin 793d65f336929becdcf8bc3f2208a5b6de93774215fe2e863bef64df419cfdb0
Expand Down
Loading