-
Notifications
You must be signed in to change notification settings - Fork 360
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
feat: Add succinct lightclient ISM #4334
base: main
Are you sure you want to change the base?
Conversation
…tClient unit tests
🦋 Changeset detectedLatest commit: 36a9d1f The changes in this PR will be included in the next version bump. This PR includes changesets to release 9 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4334 +/- ##
==========================================
+ Coverage 73.97% 74.29% +0.32%
==========================================
Files 101 105 +4
Lines 1460 1498 +38
Branches 191 195 +4
==========================================
+ Hits 1080 1113 +33
- Misses 359 364 +5
Partials 21 21
|
…server and ProofService to make a request to beacon node and RPC for proofs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just did a quick pass, probably would do another
} | ||
|
||
/// @inheritdoc AbstractPostDispatchHook | ||
function _quoteDispatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably be configurable to correctly meter for the relayer gas consumption of the ISM?
* @title SP1LightClientIsm | ||
* @notice Uses Succinct to verify that a message was delivered via a Hyperlane Mailbox and tracked by DispatchedHook | ||
*/ | ||
contract SP1LightClientIsm is AbstractCcipReadIsm, OwnableUpgradeable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit out my comfort zone, but IIUC there should be some pretty general logic in here that could be extracted to a "StorageProofIsm"? Specifically, the storage proof logic seems independent of SP1 right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move generalized logic to StorageProofIsm
import {StorageProof} from "../../libs/StateProofHelpers.sol"; | ||
import {ISuccinctProofsService} from "../../interfaces/ccip-gateways/ISuccinctProofsService.sol"; | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would love some readme/mermaid diagrams explain the general architecture
@@ -3,21 +3,15 @@ import dotenvFlow from 'dotenv-flow'; | |||
dotenvFlow.config(); | |||
|
|||
const RPC_ADDRESS = process.env.RPC_ADDRESS as string; | |||
const LIGHT_CLIENT_ADDR = process.env.LIGHT_CLIENT_ADDR as string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't realize that we have this already in the monorepo. I think we should have an opinion on whether they should be living in the monorepo. IMO at minimum it should be modular so that the default is not SP1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Olympix Integrated Security found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
Add SP1LightClientIsm that uses Succinct SP1 Helios to verify message delivery using ZK prove source chain header.
messageId
indispatched
on the source chain.executionStateRoot
Ism.getOffchainVerifyInfo()
, which responds with metadata to make a request to the CCIP server.eth_getProof
to get the state and storage proofs for the DispatchedHookdispatched
slot on the source chain.verify()
receives the proofs to get themessageId
on DispatchedHook.These are the deployments that uses the Ism to create a bridge between holesky and basesepolia:
SP1LightClientISM: https://sepolia.basescan.org/address/0xc93cF66649631303b70b7640Fbe4aaE667d93475
SP1LightClient: https://sepolia.basescan.org/address/0x2241db9AE432C0f7DE41f88f2f28dF1242bF0315
Native: https://holesky.etherscan.io/address/0x720127655fbb415C4c289C2462c8096f38ABD4cC
Synthetic: https://sepolia.basescan.org/address/0x828616Ac3CFaCB309862d19625b5d7AA02e620E1
Deployment
Known Issues
The consensus nodes are much slower the execution nodes (aka RPCs). This means that it takes a while (sometimes up to 20 mins) to receive the state proofs and update the LightClient.