This repository was archived by the owner on Mar 14, 2025. It is now read-only.
CCIP-1704 Checking for curse on Source Chain#580
Merged
mateusz-sekara merged 9 commits intoccip-developfrom Mar 6, 2024
Merged
CCIP-1704 Checking for curse on Source Chain#580mateusz-sekara merged 9 commits intoccip-developfrom
mateusz-sekara merged 9 commits intoccip-developfrom
Conversation
mateusz-sekara
changed the title
mateusz-sekara
force-pushed
the
checking-source-curse
branch
from
008bcfe to
027450c
Compare
mateusz-sekara
force-pushed
the
checking-source-curse
branch
from
276490c to
cefb174
Compare
mateusz-sekara
commented
Mar 5, 2024
| } | ||
| if down { | ||
| return nil, ccip.ErrCommitStoreIsDown | ||
| if err := ccipcommon.VerifyNotDown(ctx, r.lggr, r.commitStoreReader, r.onRampReader); err != nil { |
Contributor
Author
There was a problem hiding this comment.
Simplified. One check, instead of two (bool + error)
mateusz-sekara
force-pushed
the
checking-source-curse
branch
3 times, most recently
from
c96c31e to
6da16f4
Compare
mateusz-sekara
force-pushed
the
checking-source-curse
branch
from
6da16f4 to
ef56ac6
Compare
mateusz-sekara
changed the title
makramkd
reviewed
Mar 5, 2024
makramkd
reviewed
Mar 5, 2024
core/services/ocr2/plugins/ccip/internal/ccipdata/v1_0_0/onramp.go
Outdated
Show resolved
Hide resolved
makramkd
approved these changes
Mar 5, 2024
core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go
Outdated
Show resolved
Hide resolved
core/services/ocr2/plugins/ccip/internal/ccipcommon/shortcuts.go
Outdated
Show resolved
Hide resolved
|
|
||
| if isDown || isCursed { | ||
| lggr.Errorf("Source chain is cursed or CommitStore is down", "isDown", isDown, "isCursed", isCursed) | ||
| return ccip.ErrChainPausedOrCursed |
Contributor
There was a problem hiding this comment.
Do we care to be more specific here in this error or is the above log enough?
Contributor
Author
There was a problem hiding this comment.
Hmm, good question, maybe having distinction between source and dest curse could be useful. Do you think that separate errors per source/dest would be enough?
Contributor
Author
There was a problem hiding this comment.
On the other hand, to be very clear it would require three different errors
- CommitStoreDown
- SourceCursed
- Both
Maybe let's keep it simple and rely on an error log for now
Co-authored-by: Makram <makramkd@users.noreply.github.com>
asoliman92
pushed a commit
that referenced
this pull request
Jul 31, 2024
## Motivation CCIP doesn't verify if the source is cursed during OCR2 phases. We want to add these check to make the system more robust. ## Solution Additional check to verify if the source chain is cursed when checking if CommitStore is down. The first call to a specific OnRampReader requires an additional RPC call to get staticConfig from the chain. StaticConfig is cached forever. Next calls only get `ArmProxy` address from the cached config and ask ARM on the source chain whether it's cursed or not. This additional check is applied to both Commit and Exec, during Observation phase
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
CCIP doesn't verify if the source is cursed during OCR2 phases. We want to add these check to make the system more robust.
Solution
Additional check to verify if the source chain is cursed when checking if CommitStore is down.
The first call to a specific OnRampReader requires an additional RPC call to get staticConfig from the chain. StaticConfig is cached forever. Next calls only get
ArmProxyaddress from the cached config and ask ARM on the source chain whether it's cursed or not.This additional check is applied to both Commit and Exec, during Observation phase