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

Move curse logic to a shared location and add new helper. #323

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

winder
Copy link
Contributor

@winder winder commented Nov 22, 2024

Minor refactoring based on my PR feedback to your PR and to make the code easier to share across exec and commit plugins.

@winder winder requested a review from dimkouv November 22, 2024 15:09
Comment on lines 45 to 49
func FilterCursed(
ctx context.Context,
ccipReader CCIPReader,
destChain ccipocr3.ChainSelector,
inputChains []ccipocr3.ChainSelector,
) ([]ccipocr3.ChainSelector, *CurseInfo, error) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Common code across all the places using GetRmnCurseInfo, I'll use this in the execute plugin as well.

@@ -29,22 +32,22 @@ type ChainSupport interface {
KnownSourceChainsSlice() ([]cciptypes.ChainSelector, error)
}

type CCIPChainSupport struct {
type ccipChainSupport struct {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reduce visibility.

lggr logger.Logger,
homeChain reader.HomeChain,
oracleIDToP2PID map[commontypes.OracleID]libocrtypes.PeerID,
nodeID commontypes.OracleID,
destChain cciptypes.ChainSelector,
) CCIPChainSupport {
return CCIPChainSupport{
) ChainSupport {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Return interface.

Comment on lines -61 to -88
type CurseInfo struct {
// CursedSourceChains contains the cursed source chains.
CursedSourceChains map[cciptypes.ChainSelector]bool
// CursedDestination indicates that the destination chain is cursed.
CursedDestination bool
// GlobalCurse indicates that all chains are cursed.
GlobalCurse bool
}

// LegacyCurseSubject Defined as a const in RMNRemote.sol
// Docs of RMNRemote:
// 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().
var LegacyCurseSubject = [16]byte{
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
}

// GlobalCurseSubject Defined as a const in RMNRemote.sol
// Docs of RMNRemote:
// 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.
var GlobalCurseSubject = [16]byte{
0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01,
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to pkg/reader

@winder winder force-pushed the will/sharable-rmn-curse-code branch from 010d73b to 19586ef Compare November 22, 2024 15:14
commit/report.go Outdated Show resolved Hide resolved
commit/report.go Outdated Show resolved Hide resolved
commit/report.go Outdated Show resolved Hide resolved
pkg/reader/curses.go Outdated Show resolved Hide resolved
pkg/reader/curses.go Outdated Show resolved Hide resolved
@winder winder force-pushed the will/sharable-rmn-curse-code branch 2 times, most recently from e47e3b5 to acd94ca Compare November 25, 2024 14:24
Base automatically changed from dk/rmn-cursing-checks to main November 25, 2024 15:18
@winder winder force-pushed the will/sharable-rmn-curse-code branch 3 times, most recently from 83409e5 to 90d4476 Compare November 25, 2024 20:13
@winder winder marked this pull request as ready for review November 25, 2024 20:13
@winder winder requested a review from a team as a code owner November 25, 2024 20:13
@winder winder force-pushed the will/sharable-rmn-curse-code branch from 90d4476 to 22ed136 Compare November 25, 2024 21:02
Copy link

Metric will/sharable-rmn-curse-code main
Coverage 74.0% 74.3%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants