-
Notifications
You must be signed in to change notification settings - Fork 1
docs: L2 bridging doc #97
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
base: master
Are you sure you want to change the base?
Conversation
WalkthroughThis PR adds an ALCHEMY_API_KEY env entry, new relay tooling and tasks (including a new relay-op task), updates hardhat configs for Alchemy and Etherscan/customChains, migrates scripts to updated SDKs/libs (viem, ethers5, zksync-ethers), adds bridging documentation, and updates package scripts and dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HTask as Hardhat Task (relay-op)
participant L2 as L2 (OP Stack)
participant L1 as L1 (Ethereum)
User->>HTask: run relay-op --txhash <hash>
HTask->>L2: fetchTransactionReceipt(txhash)
L2-->>HTask: l2Receipt
rect rgb(230,240,250)
Note over HTask: Wait / Proof readiness
HTask->>HTask: waitToProve(l2Receipt)
HTask-->>HTask: status (waiting|ready)
end
alt ready-to-prove
rect rgb(245,230,250)
HTask->>HTask: buildProveWithdrawal()
HTask->>L1: proveWithdrawal(proof)
L1-->>HTask: proofSubmitted
end
end
rect rgb(230,250,240)
Note over HTask,L1: Finalization
HTask->>L1: checkFinalizeStatus()
alt ready-to-finalize
HTask->>L1: finalizeWithdrawal()
L1-->>HTask: finalized
else waiting-to-finalize
HTask-->>HTask: waitForFinalizationWindow()
end
end
HTask-->>User: log final status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
contracts/scripts/get_event_properties.js (1)
29-44: Restore ethers v5-compatible helpers.
ethers.idandethers.AbiCoder.defaultAbiCoder()only exist in ethers v6. The repo still runs on ethers v5 (e.g.,contracts/tasks/relay-arbitrum.jsrequires"ethers5"and the docs stress the v5 requirement), so these calls will throw at runtime, breaking selector derivation and encoding. Please keep using the v5ethers.utilshelpers until the toolchain is upgraded.Apply this diff to fix the regression:
const topicHash = "0x3a36e47291f4201faf137fab081d92295bce2d53be2c6ca68ba82c7faa9ce241"; // L1MessageSent const eventLogs = receipt.logs.filter((log) => log.topics[0] === topicHash); @@ function getFunctionSelector(functionSignature) { - const hash = ethers.id(functionSignature); + const hash = ethers.utils.id(functionSignature); const selector = hash.slice(0, 10); // 0x + first 4 bytes return selector; } @@ - return ethers.AbiCoder.defaultAbiCoder().encode([param.type], [param.value]).slice(2); + return ethers.utils.defaultAbiCoder.encode([param.type], [param.value]).slice(2); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
contracts/.env.example(1 hunks)contracts/docs/Bridging.md(1 hunks)contracts/hardhat.config.ts(6 hunks)contracts/hardhat.config.zksync.ts(0 hunks)contracts/package.json(3 hunks)contracts/scripts/execute_proof.js(5 hunks)contracts/scripts/get_event_properties.js(2 hunks)contracts/tasks/relay-arbitrum.js(3 hunks)contracts/tasks/relay-op.js(1 hunks)
💤 Files with no reviewable changes (1)
- contracts/hardhat.config.zksync.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-03-24T14:46:25.118Z
Learnt from: jaybuidl
Repo: kleros/cross-chain-realitio-proxy PR: 68
File: dynamic-script/package.json:2-4
Timestamp: 2025-03-24T14:46:25.118Z
Learning: The kleros/cross-chain-realitio-dynamic-script package is distributed via IPFS CID, not through NPM, so package name changes don't create breaking changes for consumers.
Applied to files:
contracts/package.json
🧬 Code graph analysis (4)
contracts/tasks/relay-op.js (3)
contracts/scripts/execute_proof.js (4)
require(3-3)require(4-4)hre(1-1)chainId(16-16)contracts/tasks/relay-arbitrum.js (5)
require(1-1)require(2-2)require(3-3)chainId(25-25)receipt(43-43)contracts/scripts/get_event_properties.js (2)
receipt(2-2)receipt(57-57)
contracts/scripts/get_event_properties.js (1)
contracts/scripts/execute_proof.js (1)
ethers(2-2)
contracts/tasks/relay-arbitrum.js (3)
contracts/scripts/execute_proof.js (5)
require(3-3)require(4-4)l2Receipt(45-45)l2Receipt(49-49)l2Provider(19-19)contracts/tasks/relay-op.js (7)
require(1-1)require(3-7)require(9-9)require(11-16)require(17-17)receipt(69-69)l1Wallet(52-55)contracts/scripts/get_event_properties.js (2)
receipt(2-2)receipt(57-57)
contracts/scripts/execute_proof.js (4)
contracts/tasks/relay-arbitrum.js (8)
require(1-1)require(2-2)require(3-3)foreignNetworks(21-24)chainId(25-25)l1Provider(28-28)l2Provider(29-29)l2Receipt(44-44)contracts/tasks/relay-op.js (6)
require(1-1)require(3-7)require(9-9)require(11-16)require(17-17)chainId(39-39)contracts/tasks/update-deployments.js (4)
require(1-1)require(3-3)homeProxyContract(84-84)foreignProxyContract(143-143)contracts/deploy/home/polygon.js (1)
homeProxy(31-31)
🪛 dotenv-linter (4.0.0)
contracts/.env.example
[warning] 2-2: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 2-2: [UnorderedKey] The ALCHEMY_API_KEY key should go before the PRIVATE_KEY key
(UnorderedKey)
🪛 markdownlint-cli2 (0.18.1)
contracts/docs/Bridging.md
9-9: Bare URL used
(MD034, no-bare-urls)
24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
33-33: Bare URL used
(MD034, no-bare-urls)
40-40: Bare URL used
(MD034, no-bare-urls)
53-53: Bare URL used
(MD034, no-bare-urls)
62-62: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
73-73: Bare URL used
(MD034, no-bare-urls)
74-74: Bare URL used
(MD034, no-bare-urls)
80-80: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
97-97: Bare URL used
(MD034, no-bare-urls)
| const blockNumber = l1MessageSentEvent.blockNumber; | ||
| const homeProxy = `0x${BigInt(l1MessageSentEvent.address).toString(16)}`; | ||
| const msgHash = l1MessageSentEvent.msgHash; | ||
| const eventData = await getCalldata(txHash, l1Provider); | ||
| const homeProxyContract = new ethers.Contract(homeProxy, RealitioHomeArbitrationProxy.abi, l1Provider); | ||
| const eventData = await getCalldata(txHash, l2Provider); | ||
| const homeProxyContract = new ethers.Contract(homeProxy, RealitioHomeArbitrationProxy.abi, l2Provider); | ||
| console.log(await homeProxyContract.foreignProxy()); | ||
| const foreignProxyContract = new ethers.Contract( |
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.
Preserve the proxy address formatting
Casting l1MessageSentEvent.address to BigInt and back removes any leading zeros, so ethers.Contract will reject addresses whose high nibble is 0x0. Use the address string directly (normalized with ethers.getAddress) to avoid intermittent failures.
Apply this diff:
- const homeProxy = `0x${BigInt(l1MessageSentEvent.address).toString(16)}`;
+ const homeProxy = ethers.getAddress(l1MessageSentEvent.address);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const blockNumber = l1MessageSentEvent.blockNumber; | |
| const homeProxy = `0x${BigInt(l1MessageSentEvent.address).toString(16)}`; | |
| const msgHash = l1MessageSentEvent.msgHash; | |
| const eventData = await getCalldata(txHash, l1Provider); | |
| const homeProxyContract = new ethers.Contract(homeProxy, RealitioHomeArbitrationProxy.abi, l1Provider); | |
| const eventData = await getCalldata(txHash, l2Provider); | |
| const homeProxyContract = new ethers.Contract(homeProxy, RealitioHomeArbitrationProxy.abi, l2Provider); | |
| console.log(await homeProxyContract.foreignProxy()); | |
| const foreignProxyContract = new ethers.Contract( | |
| const blockNumber = l1MessageSentEvent.blockNumber; | |
| const homeProxy = ethers.getAddress(l1MessageSentEvent.address); | |
| const msgHash = l1MessageSentEvent.msgHash; | |
| const eventData = await getCalldata(txHash, l2Provider); | |
| const homeProxyContract = new ethers.Contract(homeProxy, RealitioHomeArbitrationProxy.abi, l2Provider); | |
| console.log(await homeProxyContract.foreignProxy()); | |
| const foreignProxyContract = new ethers.Contract( |
🤖 Prompt for AI Agents
In contracts/scripts/execute_proof.js around lines 27 to 33, the code converts
l1MessageSentEvent.address to BigInt and back which strips leading zeros and can
create invalid addresses for ethers.Contract; replace that conversion with the
original address string normalized via ethers.getAddress (or ensure it already
starts with "0x" and pass ethers.getAddress(l1MessageSentEvent.address)) so the
resulting homeProxy preserves leading zeros and is a checksummed valid address
for the Contract constructor.
| const l1Wallet = createWalletClient({ | ||
| account, | ||
| transport: http(l1RpcUrl), | ||
| }).extend(walletActionsL1()); | ||
|
|
||
| // L2 clients | ||
| const l2Public = createPublicClient({ | ||
| chain: chain.l2, | ||
| transport: http(l2RpcUrl), | ||
| }).extend(publicActionsL2()); | ||
|
|
||
| const l2Wallet = createWalletClient({ | ||
| account, | ||
| transport: http(l2RpcUrl), | ||
| }).extend(walletActionsL2()); | ||
|
|
||
| // Retrieve L2 tx | ||
| const receipt = await l2Public.getTransactionReceipt({ hash: txhash }); | ||
| const status = await l1Public.getWithdrawalStatus({ | ||
| receipt, | ||
| targetChain: l2Public.chain, | ||
| }); | ||
|
|
||
| console.log(`Withdrawal status: ${status}`); | ||
|
|
||
| const { output, withdrawal } = await l1Public.waitToProve({ | ||
| receipt, | ||
| targetChain: l2Public.chain, | ||
| }); | ||
|
|
||
| // Only prove if necessary | ||
| // Note that proving the message resets the 1 week timeout each time, so this condition is mandatory. | ||
| if (status === "waiting-to-prove" || status === "ready-to-prove") { | ||
| console.log("Proving withdrawal..."); | ||
|
|
||
| const proveArgs = await l2Public.buildProveWithdrawal({ | ||
| account, | ||
| output, | ||
| withdrawal | ||
| }); | ||
|
|
||
| // Note that proof cant be obtained with Infura RPC | ||
| await l1Wallet.proveWithdrawal(proveArgs); | ||
| console.log("Proven ✅"); | ||
| } | ||
|
|
||
| if (status === "ready-to-finalize") { | ||
| // Not required by this script but keep in case bots need it | ||
| /* | ||
| console.log("Waiting until message becomes relayable..."); | ||
| await l1Public.waitToFinalize({ | ||
| targetChain: l2Public.chain, | ||
| withdrawalHash: receipt.transactionHash | ||
| });*/ | ||
|
|
||
| console.log("Finalizing withdrawal..."); | ||
| await l1Wallet.finalizeWithdrawal({ | ||
| targetChain: l2Wallet.chain, | ||
| withdrawal, | ||
| }); | ||
| console.log("Done ✅"); |
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.
Set wallet clients' chains to avoid undefined targetChain.
Both wallet clients are created without a chain, so l2Wallet.chain is undefined. When you reach the finalizeWithdrawal branch, targetChain: l2Wallet.chain ends up undefined and viem will reject the call at runtime. Provide the chain upfront (and reuse chain.l2 directly when finalizing) so the task can actually submit the finalization.
Apply this diff to fix the issue:
const l1Public = createPublicClient({
chain: chain.l1,
transport: http(l1RpcUrl),
}).extend(publicActionsL1());
const l1Wallet = createWalletClient({
account,
+ chain: chain.l1,
transport: http(l1RpcUrl),
}).extend(walletActionsL1());
// L2 clients
const l2Public = createPublicClient({
chain: chain.l2,
transport: http(l2RpcUrl),
}).extend(publicActionsL2());
const l2Wallet = createWalletClient({
account,
+ chain: chain.l2,
transport: http(l2RpcUrl),
}).extend(walletActionsL2());
@@
console.log("Finalizing withdrawal...");
await l1Wallet.finalizeWithdrawal({
- targetChain: l2Wallet.chain,
+ targetChain: chain.l2,
withdrawal,
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const l1Wallet = createWalletClient({ | |
| account, | |
| transport: http(l1RpcUrl), | |
| }).extend(walletActionsL1()); | |
| // L2 clients | |
| const l2Public = createPublicClient({ | |
| chain: chain.l2, | |
| transport: http(l2RpcUrl), | |
| }).extend(publicActionsL2()); | |
| const l2Wallet = createWalletClient({ | |
| account, | |
| transport: http(l2RpcUrl), | |
| }).extend(walletActionsL2()); | |
| // Retrieve L2 tx | |
| const receipt = await l2Public.getTransactionReceipt({ hash: txhash }); | |
| const status = await l1Public.getWithdrawalStatus({ | |
| receipt, | |
| targetChain: l2Public.chain, | |
| }); | |
| console.log(`Withdrawal status: ${status}`); | |
| const { output, withdrawal } = await l1Public.waitToProve({ | |
| receipt, | |
| targetChain: l2Public.chain, | |
| }); | |
| // Only prove if necessary | |
| // Note that proving the message resets the 1 week timeout each time, so this condition is mandatory. | |
| if (status === "waiting-to-prove" || status === "ready-to-prove") { | |
| console.log("Proving withdrawal..."); | |
| const proveArgs = await l2Public.buildProveWithdrawal({ | |
| account, | |
| output, | |
| withdrawal | |
| }); | |
| // Note that proof cant be obtained with Infura RPC | |
| await l1Wallet.proveWithdrawal(proveArgs); | |
| console.log("Proven ✅"); | |
| } | |
| if (status === "ready-to-finalize") { | |
| // Not required by this script but keep in case bots need it | |
| /* | |
| console.log("Waiting until message becomes relayable..."); | |
| await l1Public.waitToFinalize({ | |
| targetChain: l2Public.chain, | |
| withdrawalHash: receipt.transactionHash | |
| });*/ | |
| console.log("Finalizing withdrawal..."); | |
| await l1Wallet.finalizeWithdrawal({ | |
| targetChain: l2Wallet.chain, | |
| withdrawal, | |
| }); | |
| console.log("Done ✅"); | |
| const l1Wallet = createWalletClient({ | |
| account, | |
| chain: chain.l1, | |
| transport: http(l1RpcUrl), | |
| }).extend(walletActionsL1()); | |
| // L2 clients | |
| const l2Public = createPublicClient({ | |
| chain: chain.l2, | |
| transport: http(l2RpcUrl), | |
| }).extend(publicActionsL2()); | |
| const l2Wallet = createWalletClient({ | |
| account, | |
| chain: chain.l2, | |
| transport: http(l2RpcUrl), | |
| }).extend(walletActionsL2()); | |
| // Retrieve L2 tx | |
| const receipt = await l2Public.getTransactionReceipt({ hash: txhash }); | |
| const status = await l1Public.getWithdrawalStatus({ | |
| receipt, | |
| targetChain: l2Public.chain, | |
| }); | |
| console.log(`Withdrawal status: ${status}`); | |
| const { output, withdrawal } = await l1Public.waitToProve({ | |
| receipt, | |
| targetChain: l2Public.chain, | |
| }); | |
| // Only prove if necessary | |
| // Note that proving the message resets the 1 week timeout each time, so this condition is mandatory. | |
| if (status === "waiting-to-prove" || status === "ready-to-prove") { | |
| console.log("Proving withdrawal..."); | |
| const proveArgs = await l2Public.buildProveWithdrawal({ | |
| account, | |
| output, | |
| withdrawal | |
| }); | |
| // Note that proof cant be obtained with Infura RPC | |
| await l1Wallet.proveWithdrawal(proveArgs); | |
| console.log("Proven ✅"); | |
| } | |
| if (status === "ready-to-finalize") { | |
| // Not required by this script but keep in case bots need it | |
| /* | |
| console.log("Waiting until message becomes relayable..."); | |
| await l1Public.waitToFinalize({ | |
| targetChain: l2Public.chain, | |
| withdrawalHash: receipt.transactionHash | |
| });*/ | |
| console.log("Finalizing withdrawal..."); | |
| await l1Wallet.finalizeWithdrawal({ | |
| targetChain: chain.l2, | |
| withdrawal, | |
| }); | |
| console.log("Done ✅"); |
🤖 Prompt for AI Agents
In contracts/tasks/relay-op.js around lines 52 to 112, both wallet clients are
created without specifying the chain which leaves l2Wallet.chain undefined and
causes finalizeWithdrawal to be called with an undefined targetChain; set the
chain explicitly when creating the wallet clients (use chain.l1 for l1Wallet and
chain.l2 for l2Wallet) or, alternatively, pass chain.l2 directly to
finalizeWithdrawal as targetChain; update the createWalletClient calls to
include chain: chain.l1 and chain: chain.l2 respectively (or replace
targetChain: l2Wallet.chain with targetChain: chain.l2) so the finalization call
receives a valid chain object.
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
contracts/hardhat.config.ts(6 hunks)
🔇 Additional comments (4)
contracts/hardhat.config.ts (4)
7-7: LGTM!The relay-op task import follows the established pattern and enables the new relay functionality for OP Stack chains.
272-289: LGTM! Past issue resolved.The
customChainsconfiguration is correct. Both base and polygon entries now have properbrowserURLvalues pointing to their respective block explorers. The previous issue with Polygon'sbrowserURLincorrectly pointing to Base's Blockscout has been resolved.
88-88: ALCHEMY_API_KEY is already documented in setup instructions.Verification confirms that
ALCHEMY_API_KEYis documented incontracts/docs/Bridging.mdunder the Requirements section, satisfying the concern raised in the review comment.
153-153: Etherscan v2 API configuration is correct and compatible.The v2 API URLs with chainid parameters are properly configured:
- Mainnet:
chainid=1matches networkchainId: 1- Polygon:
chainid=137matches networkchainId: 137- Other networks follow the same pattern consistently
Etherscan V2 requires a chainid parameter for multichain calls and is supported by Hardhat's verification tooling. The implementation matches the official format and no changes are required.
| apiKey: { | ||
| // These are separate from Ethereum's etherscan API key | ||
| optimisticEthereum: process.env.OPTIMISM_API_KEY!, | ||
| mainnet: process.env.ETHERSCAN_API_KEY!, | ||
| polygon: process.env.ETHERSCAN_API_KEY!, | ||
| base: process.env.ETHERSCAN_API_KEY! | ||
| }, |
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.
🛠️ Refactor suggestion | 🟠 Major
Use optional chaining or default values instead of non-null assertions.
The non-null assertion operator (!) on lines 267-270 will throw runtime errors if the environment variables are undefined. This can cause Hardhat to fail even for tasks that don't require Etherscan verification.
Consider using optional chaining with fallback values:
etherscan: {
apiKey: {
// These are separate from Ethereum's etherscan API key
- optimisticEthereum: process.env.OPTIMISM_API_KEY!,
- mainnet: process.env.ETHERSCAN_API_KEY!,
- polygon: process.env.ETHERSCAN_API_KEY!,
- base: process.env.ETHERSCAN_API_KEY!
+ optimisticEthereum: process.env.OPTIMISM_API_KEY || "",
+ mainnet: process.env.ETHERSCAN_API_KEY || "",
+ polygon: process.env.ETHERSCAN_API_KEY || "",
+ base: process.env.ETHERSCAN_API_KEY || ""
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| apiKey: { | |
| // These are separate from Ethereum's etherscan API key | |
| optimisticEthereum: process.env.OPTIMISM_API_KEY!, | |
| mainnet: process.env.ETHERSCAN_API_KEY!, | |
| polygon: process.env.ETHERSCAN_API_KEY!, | |
| base: process.env.ETHERSCAN_API_KEY! | |
| }, | |
| apiKey: { | |
| // These are separate from Ethereum's etherscan API key | |
| optimisticEthereum: process.env.OPTIMISM_API_KEY || "", | |
| mainnet: process.env.ETHERSCAN_API_KEY || "", | |
| polygon: process.env.ETHERSCAN_API_KEY || "", | |
| base: process.env.ETHERSCAN_API_KEY || "" | |
| }, |
🤖 Prompt for AI Agents
In contracts/hardhat.config.ts around lines 265 to 271, the apiKey entries use
non-null assertions (process.env.VAR!) which will throw at runtime if those env
vars are missing; replace the non-null assertions with a safe fallback by using
optional chaining or default values (e.g., process.env.OPTIMISM_API_KEY ?? "" or
process.env.OPTIMISM_API_KEY || "") so the config loads even when keys are
undefined, ensuring tasks that don’t need Etherscan won’t fail.
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores