Skip to content

Commit

Permalink
CCIP-4476 remove legacy curse check in rmnRemote (#15523)
Browse files Browse the repository at this point in the history
* add legacy curse check to offramps

* fill in coverage test gap

* remove legacy curse subject from RMNRemote

* snapshot, wrapper, and changeset fix

* [Bot] Update changeset file with jira issues

* snapshot fix

* Update test naming and comments

---------

Co-authored-by: app-token-issuer-infra-releng[bot] <120227048+app-token-issuer-infra-releng[bot]@users.noreply.github.com>
  • Loading branch information
1 parent dc51106 commit baa225e
Show file tree
Hide file tree
Showing 6 changed files with 18 additions and 21 deletions.
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));
}
}
2 changes: 1 addition & 1 deletion core/gethwrappers/ccip/generated/rmn_remote/rmn_remote.go

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

0 comments on commit baa225e

Please sign in to comment.