Update Contracts due to ERC change #61
Conversation
📝 WalkthroughWalkthroughRename BlockHashProver → StateProver and BlockHashProverPointer → StateProverPointer across contracts, interfaces, scripts, tests, TS helpers; rename bhpInputs/storageProof → scpInputs/proof and getTargetBlockHash/verifyTargetBlockHash → getTargetStateCommitment/verifyTargetStateCommitment; add ERC‑7888 docs and a test payload. Changes
Sequence Diagram(s)sequenceDiagram
participant Broadcaster as Broadcaster
participant Receiver as Receiver
participant Pointer as StateProverPointer
participant Prover as StateProver
participant Verifier as StorageVerifier
Broadcaster->>Receiver: submit broadcast (route includes Pointer)
Receiver->>Pointer: read pointer via scpInputs
Pointer-->>Receiver: implementation address (Prover)
Receiver->>Prover: getTargetStateCommitment(scP_input)
Prover-->>Receiver: targetStateCommitment
Receiver->>Verifier: verifyStorageSlot(targetStateCommitment, proof)
Verifier-->>Receiver: verified slot value
Receiver-->>Broadcaster: accept/store result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
luiz-lvj
requested review from
frangio,
nahimterrazas and
pepebndc
and removed request for
pepebndc
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/contracts/StateProverPointer.sol (1)
24-34: Addoverridekeyword to interface method implementations.In Solidity 0.8.30, interface method implementations require the
overridekeyword. Without it, the contract will fail to compile.Proposed fix
- function implementationAddress() public view returns (address) { + function implementationAddress() public view override returns (address) { return _implementationAddress; } - function implementationCodeHash() public view returns (bytes32 codeHash) { + function implementationCodeHash() public view override returns (bytes32 codeHash) { codeHash = StorageSlot.getBytes32Slot(STATE_PROVER_POINTER_SLOT).value; }src/ts/ChildToParentProverHelper.ts (1)
119-133: Remove stray debug logging.
Theconsole.log("BBB")adds noise for library consumers and test output.🧹 Proposed fix
- - console.log("BBB"); - +src/contracts/provers/zksync/ParentToChildProver.sol (1)
112-131: Docs forverifyTargetStateCommitmentinput are incomplete.
The comment lists a 3‑tuple, but the function decodes 4 values includingaccountProof. Please align the docs with the actual ABI.📝 Proposed doc fix
- /// `@param` input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes storageProof). - /// - rlpBlockHeader: RLP-encoded block header of the home chain. - /// - batchNumber: The batch number for which to retrieve the L2 logs root hash. - /// - proof: Storage proof for the storage slot containing the L2 logs root hash. + /// `@param` input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes accountProof, bytes storageProof). + /// - rlpBlockHeader: RLP-encoded block header of the home chain. + /// - batchNumber: The batch number for which to retrieve the L2 logs root hash. + /// - accountProof: Account proof for the gateway ZkChain account. + /// - storageProof: Storage proof for the slot containing the L2 logs root hash.
🤖 Fix all issues with AI agents
In `@docs/PROVERS.md`:
- Around line 255-260: The fenced code block in PROVERS.md (the directory tree
under the "Create the directory structure" section) lacks a language tag which
triggers MD040; update the opening backtick fence to include a language
identifier (e.g., change "```" to "```text" or another appropriate tag) so the
directory listing block is properly tagged.
- Around line 97-99: Replace the bolded operational note line that starts with
"**Important operational note:**" and the following sentence "Pre-generated
proofs become stale when L1Block updates. Failed calls may need to be retried
with fresh proofs." with a proper Markdown heading (e.g., "### Important
operational note:") followed by the explanatory sentence to satisfy markdownlint
MD036; ensure the heading syntax is used for the phrase "Important operational
note" and keep the explanatory sentence as normal paragraph text below it.
🧹 Nitpick comments (3)
src/contracts/interfaces/IStateProver.sol (1)
8-13: Align “pure” wording withviewsignatures.The interface declares
view, but docs require “pure”; suggest “view/stateless” to match Solidity semantics and intent.✏️ Proposed doc tweak
- /// MUST be pure, with 1 exception: MAY read address(this).code. + /// MUST be view and SHOULD NOT read/write storage; MAY read address(this).code. ... - /// MUST be pure, with 1 exception: MAY read address(this).code. + /// MUST be view and SHOULD NOT read/write storage; MAY read address(this).code.Also applies to: 32-37
docs/ERC7888.md (1)
147-149: Clarify “pure” vsviewin prover requirements.Docs say
verifyTargetStateCommitment/verifyStorageSlotMUST be pure, but the interface defines them asview. Aligning avoids confusion for implementers.✏️ Proposed doc tweak
-- **Pure functions**: `verifyTargetStateCommitment()`, `verifyStorageSlot()`, and `version()` MUST be pure (with exception: MAY read `address(this).code`) +- **Stateless functions**: `verifyTargetStateCommitment()` and `verifyStorageSlot()` are `view` and SHOULD avoid storage reads/writes; `version()` MUST be pure (MAY read `address(this).code`)test/BlockHashProverPointer.t.sol (1)
1-55: Consider renaming the test file.The file is named
BlockHashProverPointer.t.solbut now containsStateProverPointerTest. For consistency with the terminology migration, consider renaming the file toStateProverPointer.t.sol.
docs/PROVERS.md
Outdated
| **Important operational note:** | ||
| Pre-generated proofs become stale when L1Block updates. Failed calls may need to be retried with fresh proofs. | ||
|
|
There was a problem hiding this comment.
Use a heading instead of bold text for the operational note.
Markdownlint MD036 flags emphasis-used-as-heading.
🔧 Proposed fix
-**Important operational note:**
+#### Important operational note
Pre-generated proofs become stale when L1Block updates. Failed calls may need to be retried with fresh proofs.📝 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.
| **Important operational note:** | |
| Pre-generated proofs become stale when L1Block updates. Failed calls may need to be retried with fresh proofs. | |
| #### Important operational note | |
| Pre-generated proofs become stale when L1Block updates. Failed calls may need to be retried with fresh proofs. | |
🤖 Prompt for AI Agents
In `@docs/PROVERS.md` around lines 97 - 99, Replace the bolded operational note
line that starts with "**Important operational note:**" and the following
sentence "Pre-generated proofs become stale when L1Block updates. Failed calls
may need to be retried with fresh proofs." with a proper Markdown heading (e.g.,
"### Important operational note:") followed by the explanatory sentence to
satisfy markdownlint MD036; ensure the heading syntax is used for the phrase
"Important operational note" and keep the explanatory sentence as normal
paragraph text below it.
docs/PROVERS.md
Outdated
| 1. **Create the directory structure:** | ||
| ``` | ||
| src/contracts/provers/{chain-name}/ | ||
| ├── ChildToParentProver.sol | ||
| └── ParentToChildProver.sol | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced block.
MD040 requires a language identifier on fenced code blocks.
🔧 Proposed fix
- ```
+ ```text
src/contracts/provers/{chain-name}/
├── ChildToParentProver.sol
└── ParentToChildProver.sol
```📝 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.
| 1. **Create the directory structure:** | |
| ``` | |
| src/contracts/provers/{chain-name}/ | |
| ├── ChildToParentProver.sol | |
| └── ParentToChildProver.sol | |
| ``` | |
| 1. **Create the directory structure:** |
🤖 Prompt for AI Agents
In `@docs/PROVERS.md` around lines 255 - 260, The fenced code block in PROVERS.md
(the directory tree under the "Create the directory structure" section) lacks a
language tag which triggers MD040; update the opening backtick fence to include
a language identifier (e.g., change "```" to "```text" or another appropriate
tag) so the directory listing block is properly tagged.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/contracts/provers/optimism/ChildToParentProver.sol (1)
73-92: Clarify param docs for “state commitment.”The parameter description still says “block hash,” which is narrower than the new interface terminology. This can confuse integrators on non-OP chains.
📝 Suggested doc tweak
- /// `@param` targetStateCommitment The block hash of the target chain. + /// `@param` targetStateCommitment The target chain's state commitment (OP uses the block hash).test/provers/scroll/ParentToChildProver.t.sol (1)
197-211: Add success test forverifyTargetStateCommitment.The test suite only includes
test_verifyTargetStateCommitment_onHomeChain, which validates the revert case. There is no test covering the success path when called from a non-home chain with valid proofs—a core function that deserves explicit validation, particularly given the complexity of the proof verification logic.Consider adding a test similar to the Arbitrum implementation's
test_verifyTargetStateCommitment, using appropriate proof fixtures to validate that the function correctly returns the target state commitment.src/ts/ParentToChildProverHelper.ts (1)
53-61: Remove debug logging and commented traces.The
console.logstatements and commented debug snippets add noise in library code and can leak runtime details.🧹 Cleanup diff
- console.log("blockNumber", blockNumber); - - // const targetBlock = await this.targetChainClient.getBlock({ blockNumber }); - // console.log("targetBlock", targetBlock); - // // `@ts-ignore` - // console.log("sendRoot", targetBlock.sendRoot); - - const { targetStateCommitment, sendRoot } = await this._findLatestAvailableTargetChainBlock(blockNumber); + const { targetStateCommitment, sendRoot } = + await this._findLatestAvailableTargetChainBlock(blockNumber); @@ - console.log("get proof BBB");Also applies to: 138-139
src/ts/ChildToParentProverHelper.ts (2)
17-28: Update the class docstring (currently misleading).It still states
buildInputForGetTargetBlockHashandbuildInputForVerifyTargetBlockHashare unimplemented, but both now return real data.✏️ Suggested doc update
- * buildInputForGetTargetBlockHash and buildInputForVerifyTargetBlockHash methods - * are currently not implemented and return a hardcoded block hash. + * buildInputForGetTargetBlockHashByBlockNumber is not implemented yet and + * currently returns a placeholder value.
124-133: Remove debug logging.The
console.log("BBB")should not ship in helper utilities.🧹 Cleanup diff
- console.log("BBB");test/provers/taiko/ChildToParentProver.t.sol (1)
8-22: Align MockSignalService storage layout with Taiko SignalServiceThe mock stores
blockNumberinside the mapping value. In Taiko’s SignalService, the mapping key is the block number and the stored struct only containsblockHashandstateRoot. This mismatch can produce incorrect slot offsets if storage-slot proofs are later added to these tests.🔧 Suggested fix
contract MockSignalService { struct Checkpoint { uint48 blockNumber; bytes32 blockHash; bytes32 stateRoot; } - mapping(uint48 => Checkpoint) private _checkpoints; + struct CheckpointRecord { + bytes32 blockHash; + bytes32 stateRoot; + } + + mapping(uint48 => CheckpointRecord) private _checkpoints; @@ function setCheckpoint(uint48 blockNumber, bytes32 blockHash, bytes32 stateRoot) external { - _checkpoints[blockNumber] = Checkpoint(blockNumber, blockHash, stateRoot); + _checkpoints[blockNumber] = CheckpointRecord(blockHash, stateRoot); } @@ function getCheckpoint(uint48 blockNumber) external view returns (Checkpoint memory) { - Checkpoint memory cp = _checkpoints[blockNumber]; - if (cp.blockHash == bytes32(0)) revert SS_CHECKPOINT_NOT_FOUND(); - return cp; + CheckpointRecord memory cp = _checkpoints[blockNumber]; + if (cp.blockHash == bytes32(0)) revert SS_CHECKPOINT_NOT_FOUND(); + return Checkpoint(blockNumber, cp.blockHash, cp.stateRoot); } }Based on learnings, Taiko’s on-chain storage record excludes
blockNumberand reconstructs it from the mapping key.
🤖 Fix all issues with AI agents
In `@docs/TUTORIAL.md`:
- Around line 478-502: The tutorial uses undefined constants PROVER_ADDRESS,
OP_TO_ETH_POINTER, and ETH_TO_ARB_POINTER; add clear definitions or explanatory
comments before their first use (e.g., provide example values or placeholders
and note they must be replaced with actual deployed addresses/pointers), or
reference where to obtain them from your deployment/config (so users can set
PROVER_ADDRESS to the prover contract address and
OP_TO_ETH_POINTER/ETH_TO_ARB_POINTER to the deployed pointer addresses); update
the code near the creation of ParentToChildProverHelper and the return route to
use those defined constants.
- Around line 512-514: Add a prerequisite note before the generated proof
example that the import from './wagmi/abi' (used by encodeFunctionData and
receiverAbi) relies on artifacts produced by the build step; instruct users to
run "yarn build" (which runs the wagmi CLI to generate wagmi/abi.ts) before
running the snippet to avoid a "module not found" error.
In `@src/contracts/provers/linea/ChildToParentProver.sol`:
- Around line 64-66: Update the NatSpec for the parameter on function
verifyStorageSlot to reflect the renamed parameter targetStateCommitment and
ERC-7888 terminology: replace the phrase "The block hash of the target chain."
with a description such as "The state commitment (state root) of the target
chain used for verification" so the docstring matches the identifier
targetStateCommitment and clarifies its meaning.
In `@src/contracts/provers/optimism/ParentToChildProver.sol`:
- Around line 7-8: Update the OpenZeppelin dependency from the release-candidate
to the stable 5.4.0 in your project dependency manifest (change any
"@openzeppelin/contracts" version 5.5.0-rc.0 to 5.4.0) and reinstall/update
lockfile (npm/yarn install) so imports like Bytes and IStateProver used in
ParentToChildProver.sol (import {Bytes} from
"@openzeppelin/contracts/utils/Bytes.sol";) resolve to the stable package; after
installing, compile and run tests and if any OpenZeppelin API differences
surface (e.g., the Bytes utility path or signatures), adjust the import or usage
in ParentToChildProver.sol accordingly.
In `@src/ts/ChildToParentProverHelper.ts`:
- Around line 53-62: The function buildInputForGetTargetBlockHashByBlockNumber
currently returns a placeholder targetStateCommitment ('0x') which can produce
invalid payloads; update it to throw an explicit error when the real
targetStateCommitment is not yet available instead of returning a placeholder.
Locate buildInputForGetTargetBlockHashByBlockNumber and replace the
placeholder-return path so it throws a clear, descriptive Error (e.g.,
"targetStateCommitment not implemented for blockNumber") before or instead of
returning the encodeAbiParameters result, ensuring callers fail fast; keep
encodeAbiParameters([{ type: 'uint256' }], [blockNumber]) as the input
generation if needed.
🧹 Nitpick comments (13)
src/contracts/provers/zksync/ChildToParentProver.sol (1)
88-91: Consider bumping the version number.The
@inheritdocreference is correctly updated toIStateProver. However, since the contract now implements a different interface (IStateProvervsIBlockHashProver) with renamed methods, consider whether the version should be incremented to reflect this breaking API change.src/contracts/provers/taiko/ParentToChildProver.sol (1)
37-37: Consider renaming error for terminology consistency.The error
TargetBlockHashNotFoundretains the oldBlockHashterminology while the rest of the contract has been updated to useStateCommitment. Consider renaming toTargetStateCommitmentNotFoundfor consistency with the ERC-7888 updates.♻️ Suggested rename
- error TargetBlockHashNotFound(); + error TargetStateCommitmentNotFound();And update the revert statements on lines 74 and 96:
- revert TargetBlockHashNotFound(); + revert TargetStateCommitmentNotFound();src/contracts/provers/taiko/ChildToParentProver.sol (1)
37-37: Consider renaming error for terminology consistency.Similar to
ParentToChildProver, the errorTargetBlockHashNotFoundretains the old terminology while the rest of the contract usesStateCommitment. Consider renaming toTargetStateCommitmentNotFoundfor consistency across both prover contracts.♻️ Suggested rename
- error TargetBlockHashNotFound(); + error TargetStateCommitmentNotFound();And update the revert statements on lines 74 and 96.
docs/TUTORIAL.md (4)
42-74: Optional: Add language specifier to fenced code block.The ASCII diagram lacks a language specifier. While this renders fine, adding
```textinstead of```improves markdown compliance.📝 Proposed fix
-``` +```text ┌─────────────────────────────────────────────────────────────────┐ │ Source Chain (L2-A) │
362-365: Clarify token burn implementation.The comment "Assume token has burn function, or send to dead address" is vague and may confuse implementers. Consider showing both approaches explicitly or recommending one.
💡 Suggested clarification
function burn(address recipient, uint256 amount) external { // Transfer and burn tokens token.transferFrom(msg.sender, address(this), amount); - // Assume token has burn function, or send to dead address + + // Option 1: If token implements burn() + // token.burn(amount); + + // Option 2: Transfer to dead address + // token.transfer(address(0xdead), amount);
174-177: Optional: Add language specifier to code block.The route explanation code block lacks a language specifier. Add
```textfor better markdown compliance.
179-179: Optional: Convert emphasis to heading.The bold text could be a proper subheading for better document structure and navigation.
📝 Proposed fix
-**Example: Arbitrum → Ethereum → Optimism** +#### Example: Arbitrum → Ethereum → Optimismsrc/contracts/provers/scroll/ParentToChildProver.sol (1)
89-107: Consider adding an explicit finalization check.The function reverts on
bytes32(0)state root, but theIScrollChaininterface includesisBatchFinalized(). Consider whether an explicit finalization check would provide clearer error semantics for callers, distinguishing between "batch not finalized yet" vs "batch doesn't exist".Current behavior implicitly relies on unfinalized batches having zero state roots, which may be valid but less explicit.
♻️ Optional: Add explicit finalization check
function getTargetStateCommitment(bytes calldata input) external view returns (bytes32 targetStateCommitment) { if (block.chainid != homeChainId) { revert CallNotOnHomeChain(); } // Decode the input uint256 batchIndex = abi.decode(input, (uint256)); + // Check if batch is finalized + if (!IScrollChain(scrollChain).isBatchFinalized(batchIndex)) { + revert BatchNotFinalized(); + } + // Get the state root from ScrollChain targetStateCommitment = IScrollChain(scrollChain).finalizedStateRoots(batchIndex); if (targetStateCommitment == bytes32(0)) { revert StateRootNotFound(); } }src/contracts/provers/scroll/ChildToParentProver.sol (2)
1-2: Pragma version inconsistency with sibling contract.This file uses
pragma solidity ^0.8.28whileParentToChildProver.solusespragma solidity 0.8.30. Consider aligning pragma versions across the Scroll prover contracts for consistency and to ensure identical compiler behavior.♻️ Align pragma version
// SPDX-License-Identifier: MIT -pragma solidity ^0.8.28; +pragma solidity 0.8.30;
67-86: Clarify thattargetStateCommitmentis a block hash in this prover.The comment on line 68 correctly states this is "The block hash of the target chain," but this creates a semantic mismatch with the parameter name
targetStateCommitment. Consider adding a note similar toParentToChildProver.sol's approach to clarify the distinction for maintainers.📝 Suggested documentation improvement
/// `@notice` Verify a storage slot given a target chain block hash and a proof. - /// `@param` targetStateCommitment The block hash of the target chain. + /// `@param` targetStateCommitment The block hash of the target chain (NOTE: for child-to-parent provers, + /// the state commitment is actually a block hash, unlike parent-to-child provers which may use state roots) /// `@param` input ABI encoded (bytes blockHeader, address account, uint256 slot, bytes accountProof, bytes storageProof)test/provers/scroll/ParentToChildProver.t.sol (2)
1-2: Pragma version inconsistency across test and source files.This test uses
pragma solidity ^0.8.27while the source contracts use0.8.30and^0.8.28. Consider aligning versions for consistent compiler behavior.
25-25: Fix indentation on contract declaration.The contract declaration has extra leading whitespace that breaks the file's formatting consistency.
♻️ Fix indentation
- contract ScrollParentToChildProverTest is Test { +contract ScrollParentToChildProverTest is Test {Note: This will require re-indenting the entire contract body accordingly.
src/contracts/provers/linea/ParentToChildProver.sol (1)
144-190: Add explicit non‑empty proof checks for clearer errors.Right now, empty
accountProoforstorageProofwill revert with a panic before your custom errors fire. A short guard makes failures deterministic and easier to debug.♻️ Suggested guard checks
( account, slot, accountLeafIndex, accountProof, accountValue, storageLeafIndex, storageProof, claimedStorageValue ) = abi.decode(input, (address, uint256, uint256, bytes[], bytes, uint256, bytes[], bytes32)); + + if (accountProof.length == 0) revert InvalidAccountProof(); + if (storageProof.length == 0) revert InvalidStorageProof();
docs/TUTORIAL.md
Outdated
| const ethToArbHelper = new ParentToChildProverHelper( | ||
| PROVER_ADDRESS, | ||
| mainnetClient, // home | ||
| arbitrumClient // target | ||
| ); | ||
|
|
||
| const { input: scpInput1, targetStateCommitment: arbBlockHash } = | ||
| await ethToArbHelper.buildInputForVerifyTargetBlockHash(ethBlockHash); | ||
|
|
||
| // Step 3: Generate storage proof for the broadcaster slot | ||
| const messageSlot = BigInt(keccak256(encodeAbiParameters( | ||
| [{ type: 'bytes32' }, { type: 'address' }], | ||
| [message, publisher] | ||
| ))); | ||
|
|
||
| const { input: storageProof, slotValue } = | ||
| await ethToArbHelper.buildInputForVerifyStorageSlot( | ||
| arbBlockHash, | ||
| broadcasterAddress, | ||
| messageSlot | ||
| ); | ||
|
|
||
| return { | ||
| route: [OP_TO_ETH_POINTER, ETH_TO_ARB_POINTER], | ||
| scpInputs: [scpInput0, scpInput1], |
There was a problem hiding this comment.
Define or explain undefined constants.
The code references PROVER_ADDRESS, OP_TO_ETH_POINTER, and ETH_TO_ARB_POINTER without defining them. Users following the tutorial won't know where these values come from.
🔧 Suggested fix
Add constant definitions before usage:
+ // Define prover and pointer addresses for your deployment
+ const PROVER_ADDRESS = '0x...' as const; // ParentToChild prover address
+ const OP_TO_ETH_POINTER = '0x...' as const; // Optimism→Ethereum pointer
+ const ETH_TO_ARB_POINTER = '0x...' as const; // Ethereum→Arbitrum pointer
+
// Route: Optimism → Ethereum → Arbitrum
// Step 1: Get Ethereum block hash from OptimismOr add a comment explaining:
// Note: Replace with actual deployed pointer addresses from your setup
const OP_TO_ETH_POINTER = '0x...';🤖 Prompt for AI Agents
In `@docs/TUTORIAL.md` around lines 478 - 502, The tutorial uses undefined
constants PROVER_ADDRESS, OP_TO_ETH_POINTER, and ETH_TO_ARB_POINTER; add clear
definitions or explanatory comments before their first use (e.g., provide
example values or placeholders and note they must be replaced with actual
deployed addresses/pointers), or reference where to obtain them from your
deployment/config (so users can set PROVER_ADDRESS to the prover contract
address and OP_TO_ETH_POINTER/ETH_TO_ARB_POINTER to the deployed pointer
addresses); update the code near the creation of ParentToChildProverHelper and
the return route to use those defined constants.
docs/TUTORIAL.md
Outdated
| import { encodeFunctionData } from 'viem'; | ||
| import { receiverAbi } from './wagmi/abi'; | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check repository structure for wagmi and abi files
echo "=== Looking for wagmi directory ==="
fd -t d "wagmi"
echo -e "\n=== Looking for abi files ==="
fd -t f "abi" | grep -E "\.(ts|js|json)$" | head -20
echo -e "\n=== Searching for receiverAbi references ==="
rg "receiverAbi" --type=ts --type=js --type=json -l
echo -e "\n=== Checking docs directory structure ==="
ls -la docs/ | head -20Repository: openintentsframework/broadcaster
Length of output: 595
🏁 Script executed:
#!/bin/bash
# Read the TUTORIAL.md file around lines 512-514 to understand context
sed -n '500,530p' docs/TUTORIAL.mdRepository: openintentsframework/broadcaster
Length of output: 772
🏁 Script executed:
#!/bin/bash
# Get broader view of repository structure
echo "=== Repository root structure ==="
ls -la | head -30
echo -e "\n=== Looking for all .json files that might contain ABIs ==="
find . -type f -name "*.json" | grep -v node_modules | head -20
echo -e "\n=== Looking for any out/ or artifacts/ directories ==="
fd -t d "out|artifacts|contracts" | head -20
echo -e "\n=== Checking if there's any setup instructions in docs ==="
grep -n "import.*abi" docs/TUTORIAL.md | head -10Repository: openintentsframework/broadcaster
Length of output: 3164
🏁 Script executed:
#!/bin/bash
# Check wagmi.config.js to understand how ABIs should be generated
cat wagmi.config.js
echo -e "\n=== Looking for src/contracts structure ==="
find src/contracts -type f | head -20
echo -e "\n=== Check if there's a src directory structure ==="
find src -type f -name "*.ts" -o -name "*.js" | head -20
echo -e "\n=== Check package.json for wagmi setup ==="
cat package.jsonRepository: openintentsframework/broadcaster
Length of output: 3041
🏁 Script executed:
#!/bin/bash
# Check if wagmi/abi.ts is generated or in gitignore
echo "=== Check .gitignore ==="
cat .gitignore
echo -e "\n=== Check if wagmi directory exists ==="
ls -la wagmi/ 2>/dev/null || echo "wagmi directory does not exist"
echo -e "\n=== Check entire TUTORIAL.md for context around the import ==="
sed -n '505,525p' docs/TUTORIAL.mdRepository: openintentsframework/broadcaster
Length of output: 934
🏁 Script executed:
#!/bin/bash
# Check if there's any setup/installation section in the tutorial
echo "=== First 100 lines of TUTORIAL.md ==="
head -100 docs/TUTORIAL.md
echo -e "\n=== Search for 'wagmi' or 'build' mentions ==="
grep -n "wagmi\|build\|generate\|npm\|yarn" docs/TUTORIAL.md | head -20Repository: openintentsframework/broadcaster
Length of output: 4673
🏁 Script executed:
#!/bin/bash
# Check the full section around line 513 to see if there's any setup instruction
sed -n '475,530p' docs/TUTORIAL.md
echo -e "\n=== Check if README mentions build step ==="
grep -n "yarn build\|wagmi generate" README.md docs/README.md 2>/dev/null || echo "Not found in README files"Repository: openintentsframework/broadcaster
Length of output: 1651
Add prerequisite note before the generated proof example.
The import from './wagmi/abi' references a file generated during the build process. Add a note before the code snippet at line 513 instructing users to run yarn build first (which generates wagmi/abi.ts via the wagmi CLI). Otherwise, users will encounter a "module not found" error.
Example addition:
Before using the generated proof, ensure the ABIs are generated by running:
\`\`\`bash
yarn build
\`\`\`
This generates the `wagmi/abi.ts` file from your Solidity contracts.
🤖 Prompt for AI Agents
In `@docs/TUTORIAL.md` around lines 512 - 514, Add a prerequisite note before the
generated proof example that the import from './wagmi/abi' (used by
encodeFunctionData and receiverAbi) relies on artifacts produced by the build
step; instruct users to run "yarn build" (which runs the wagmi CLI to generate
wagmi/abi.ts) before running the snippet to avoid a "module not found" error.
| /// @param targetStateCommitment The block hash of the target chain. | ||
| /// @param input ABI encoded (bytes blockHeader, address account, uint256 slot, bytes accountProof, bytes storageProof) | ||
| function verifyStorageSlot(bytes32 targetBlockHash, bytes calldata input) | ||
| function verifyStorageSlot(bytes32 targetStateCommitment, bytes calldata input) |
There was a problem hiding this comment.
NatSpec description doesn't match the renamed parameter.
The parameter was renamed to targetStateCommitment, but the description still says "The block hash of the target chain." Consider updating for consistency with the ERC-7888 terminology.
📝 Suggested fix
- /// `@param` targetStateCommitment The block hash of the target chain.
+ /// `@param` targetStateCommitment The state commitment of the target chain.📝 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.
| /// @param targetStateCommitment The block hash of the target chain. | |
| /// @param input ABI encoded (bytes blockHeader, address account, uint256 slot, bytes accountProof, bytes storageProof) | |
| function verifyStorageSlot(bytes32 targetBlockHash, bytes calldata input) | |
| function verifyStorageSlot(bytes32 targetStateCommitment, bytes calldata input) | |
| /// `@param` targetStateCommitment The state commitment of the target chain. | |
| /// `@param` input ABI encoded (bytes blockHeader, address account, uint256 slot, bytes accountProof, bytes storageProof) | |
| function verifyStorageSlot(bytes32 targetStateCommitment, bytes calldata input) |
🤖 Prompt for AI Agents
In `@src/contracts/provers/linea/ChildToParentProver.sol` around lines 64 - 66,
Update the NatSpec for the parameter on function verifyStorageSlot to reflect
the renamed parameter targetStateCommitment and ERC-7888 terminology: replace
the phrase "The block hash of the target chain." with a description such as "The
state commitment (state root) of the target chain used for verification" so the
docstring matches the identifier targetStateCommitment and clarifies its
meaning.
| import {IStateProver} from "../../interfaces/IStateProver.sol"; | ||
| import {Bytes} from "@openzeppelin/contracts/utils/Bytes.sol"; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
latest stable version @openzeppelin/contracts npm package 2025
💡 Result:
Latest stable @openzeppelin/contracts version released in 2025: 5.4.0 (released July 17, 2025). [1][2]
🏁 Script executed:
find . -name "package.json" -type f | head -5Repository: openintentsframework/broadcaster
Length of output: 137
🏁 Script executed:
cat package.json | grep -A 5 -B 5 "@openzeppelin/contracts"Repository: openintentsframework/broadcaster
Length of output: 552
Update to stable OpenZeppelin version.
The codebase uses @openzeppelin/contracts version 5.5.0-rc.0, a release candidate unsuitable for production. Stable version 5.4.0 is already available and should be used instead. Release candidate versions may contain bugs or breaking changes before final release.
🤖 Prompt for AI Agents
In `@src/contracts/provers/optimism/ParentToChildProver.sol` around lines 7 - 8,
Update the OpenZeppelin dependency from the release-candidate to the stable
5.4.0 in your project dependency manifest (change any "@openzeppelin/contracts"
version 5.5.0-rc.0 to 5.4.0) and reinstall/update lockfile (npm/yarn install) so
imports like Bytes and IStateProver used in ParentToChildProver.sol (import
{Bytes} from "@openzeppelin/contracts/utils/Bytes.sol";) resolve to the stable
package; after installing, compile and run tests and if any OpenZeppelin API
differences surface (e.g., the Bytes utility path or signatures), adjust the
import or usage in ParentToChildProver.sol accordingly.
| async buildInputForGetTargetBlockHashByBlockNumber(blockNumber: bigint): Promise<{ | ||
| input: Hex | ||
| targetBlockHash: Hash | ||
| targetStateCommitment: Hash | ||
| }> { | ||
| //// TODO | ||
|
|
||
| return { | ||
| input: encodeAbiParameters([{ type: 'uint256' }], [blockNumber]), | ||
| targetBlockHash: '0x' as `0x${string}`, | ||
| targetStateCommitment: '0x' as `0x${string}`, | ||
| } |
There was a problem hiding this comment.
Fail fast instead of returning a placeholder value.
Returning '0x' can silently produce invalid payloads. Until this is implemented, throw to make misuse obvious.
🛑 Safer temporary behavior
async buildInputForGetTargetBlockHashByBlockNumber(blockNumber: bigint): Promise<{
input: Hex
targetStateCommitment: Hash
}> {
- //// TODO
-
- return {
- input: encodeAbiParameters([{ type: 'uint256' }], [blockNumber]),
- targetStateCommitment: '0x' as `0x${string}`,
- }
+ throw new Error('buildInputForGetTargetBlockHashByBlockNumber is not implemented')
}If you want, I can draft an implementation once the expected source of targetStateCommitment for an explicit block number is confirmed.
📝 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.
| async buildInputForGetTargetBlockHashByBlockNumber(blockNumber: bigint): Promise<{ | |
| input: Hex | |
| targetBlockHash: Hash | |
| targetStateCommitment: Hash | |
| }> { | |
| //// TODO | |
| return { | |
| input: encodeAbiParameters([{ type: 'uint256' }], [blockNumber]), | |
| targetBlockHash: '0x' as `0x${string}`, | |
| targetStateCommitment: '0x' as `0x${string}`, | |
| } | |
| async buildInputForGetTargetBlockHashByBlockNumber(blockNumber: bigint): Promise<{ | |
| input: Hex | |
| targetStateCommitment: Hash | |
| }> { | |
| throw new Error('buildInputForGetTargetBlockHashByBlockNumber is not implemented') | |
| } |
🤖 Prompt for AI Agents
In `@src/ts/ChildToParentProverHelper.ts` around lines 53 - 62, The function
buildInputForGetTargetBlockHashByBlockNumber currently returns a placeholder
targetStateCommitment ('0x') which can produce invalid payloads; update it to
throw an explicit error when the real targetStateCommitment is not yet available
instead of returning a placeholder. Locate
buildInputForGetTargetBlockHashByBlockNumber and replace the placeholder-return
path so it throws a clear, descriptive Error (e.g., "targetStateCommitment not
implemented for blockNumber") before or instead of returning the
encodeAbiParameters result, ensuring callers fail fast; keep
encodeAbiParameters([{ type: 'uint256' }], [blockNumber]) as the input
generation if needed.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/contracts/provers/taiko/ParentToChildProver.sol (1)
100-121: NatSpec still describes a block hash instead of a state commitment.Align the
@notice/@param text withtargetStateCommitment.📝 Suggested doc fix
- /// `@notice` Verify a storage slot given a target chain block hash and a proof - /// `@param` targetStateCommitment The block hash of the target chain (L2) + /// `@notice` Verify a storage slot given a target chain state commitment and a proof + /// `@param` targetStateCommitment The state commitment of the target chain (L2)src/contracts/provers/arbitrum/ChildToParentProver.sol (1)
71-89: NatSpec still describes a block hash instead of a state commitment.Align the summary and parameter description with the new naming.
📝 Suggested doc fix
- /// `@notice` Verify a storage slot given a target chain block hash and a proof. - /// `@param` targetStateCommitment The block hash of the target chain. + /// `@notice` Verify a storage slot given a target chain state commitment and a proof. + /// `@param` targetStateCommitment The state commitment of the target chain.src/contracts/provers/taiko/ChildToParentProver.sol (1)
100-121: NatSpec still describes a block hash instead of a state commitment.Align the
@notice/@param text withtargetStateCommitment.📝 Suggested doc fix
- /// `@notice` Verify a storage slot given a target chain block hash and a proof - /// `@param` targetStateCommitment The block hash of the target chain (L1) + /// `@notice` Verify a storage slot given a target chain state commitment and a proof + /// `@param` targetStateCommitment The state commitment of the target chain (L1)src/contracts/provers/zksync/ParentToChildProver.sol (1)
112-139: Input tuple docs don’t match the actual decode signature.The function decodes
accountProofandstorageProof, but NatSpec only mentions a single proof.📝 Suggested doc fix
- /// `@param` input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes storageProof). + /// `@param` input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes accountProof, bytes storageProof). @@ - /// - proof: Storage proof for the storage slot containing the L2 logs root hash. + /// - accountProof: Account proof for the gateway ZkChain contract. + /// - storageProof: Storage proof for the storage slot containing the L2 logs root hash.
🤖 Fix all issues with AI agents
In `@src/contracts/provers/scroll/ChildToParentProver.sol`:
- Around line 29-32: Update the NatSpec on verifyTargetStateCommitment (and the
similar comment block around the second occurrence) to use consistent "state
commitment" terminology: either explicitly state "state commitment (block hash
on Scroll)" if the Scroll target is indeed a block hash, or replace all mentions
of "block hash" with "state commitment" to avoid ambiguity; ensure the parameter
descriptions (homeBlockHash and input) and the function notice clearly reference
"target state commitment" instead of "block hash" for both comment locations.
In `@src/contracts/ZkSyncBroadcaster.sol`:
- Around line 40-41: Update the contract-level documentation in
ZkSyncBroadcaster (the top-level comment for contract ZkSyncBroadcaster) to
match the NatSpec change: replace the phrase stating the L2→L1 message contains
"the original message and timestamp" with wording that it contains the "slot and
timestamp" (ABI-encoded as bytes32 slot, uint256 timestamp) so the contract doc
and the NatSpec for functions like the L1Messenger-related methods are
consistent.
| /// @notice Get a parent chain block hash from the buffer at `blockHashBuffer` using a storage proof | ||
| /// @param homeBlockHash The block hash of the home chain. | ||
| /// @param input ABI encoded (bytes blockHeader, uint256 targetBlockNumber, bytes accountProof, bytes storageProof) | ||
| function verifyTargetBlockHash(bytes32 homeBlockHash, bytes calldata input) | ||
| function verifyTargetStateCommitment(bytes32 homeBlockHash, bytes calldata input) |
There was a problem hiding this comment.
Align NatSpec with state-commitment terminology.
The comments still describe “block hash” for targetStateCommitment. If on Scroll the state commitment is specifically the block hash, please state that explicitly; otherwise update the wording to “state commitment” to avoid ambiguity.
Also applies to: 67-69
🤖 Prompt for AI Agents
In `@src/contracts/provers/scroll/ChildToParentProver.sol` around lines 29 - 32,
Update the NatSpec on verifyTargetStateCommitment (and the similar comment block
around the second occurrence) to use consistent "state commitment" terminology:
either explicitly state "state commitment (block hash on Scroll)" if the Scroll
target is indeed a block hash, or replace all mentions of "block hash" with
"state commitment" to avoid ambiguity; ensure the parameter descriptions
(homeBlockHash and input) and the function notice clearly reference "target
state commitment" instead of "block hash" for both comment locations.
| /// implementation sends an L2->L1 message via the L1Messenger containing the message slot and | ||
| /// timestamp ABI encoded together (bytes32 slot, uint256 timestamp). |
There was a problem hiding this comment.
Documentation fix is correct, but contract-level doc is now inconsistent.
The updated NatSpec accurately reflects the actual encoding (slot, timestamp). However, the contract-level documentation at line 23 still states the L2→L1 message contains "the original message and timestamp" rather than "the slot and timestamp."
Consider updating line 23 to maintain consistency:
📝 Proposed fix
-/// broadcast, it sends an L2->L1 message containing the original message and timestamp (ABI encoded together).
+/// broadcast, it sends an L2->L1 message containing the message slot and timestamp (ABI encoded together).🤖 Prompt for AI Agents
In `@src/contracts/ZkSyncBroadcaster.sol` around lines 40 - 41, Update the
contract-level documentation in ZkSyncBroadcaster (the top-level comment for
contract ZkSyncBroadcaster) to match the NatSpec change: replace the phrase
stating the L2→L1 message contains "the original message and timestamp" with
wording that it contains the "slot and timestamp" (ABI-encoded as bytes32 slot,
uint256 timestamp) so the contract doc and the NatSpec for functions like the
L1Messenger-related methods are consistent.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/contracts/provers/arbitrum/ChildToParentProver.sol`:
- Around line 69-70: Update the NatSpec to replace “block hash” wording with
state‑commitment terminology for the verifyStorageSlot comment: change the
summary to mention a target chain state commitment (not block hash) and update
the `@param` description for targetStateCommitment to something like “The state
commitment of the target chain” so it matches the renamed symbol; ensure this
change appears in the verifyStorageSlot function comment in ChildToParentProver
(and any other comments referencing targetStateCommitment).
In `@src/contracts/provers/zksync/ChildToParentProver.sol`:
- Around line 69-71: The NatSpec for the verify storage slot function uses
ambiguous wording for the targetStateCommitment parameter (currently described
as "block hash"); update the comment for targetStateCommitment in
ChildToParentProver.sol to clearly state exactly what value is expected (e.g.,
"state commitment / state root of the target chain block" or "state root of the
target chain block header"), so callers know whether to pass a block hash, block
header root, or state commitment; modify the `@param` line for
targetStateCommitment to use the precise term ("state root" or "state
commitment") and briefly indicate it must correspond to the blockHeader provided
in the input.
In `@src/contracts/provers/zksync/ParentToChildProver.sol`:
- Around line 115-119: Update the NatSpec for verifyTargetStateCommitment to
reflect the actual decoded calldata: change the `@param` for input to specify it
is an ABI-encoded tuple (bytes rlpBlockHeader, uint256 batchNumber, bytes
accountProof) and document that accountProof itself is an ABI-encoded tuple
which the function decodes (e.g., (bytes rlpAccountProof, address account,
bytes32 storageSlot, bytes storageProof) — or whatever exact fields
verifyTargetStateCommitment extracts from accountProof); mention that
rlpBlockHeader is the home-chain RLP block header and storageProof is the
storage proof for the specified storageSlot so integrators know how to assemble
calldata for verifyTargetStateCommitment and the accountProof decoding performed
inside that function.
| /// @notice Verify a storage slot given a target chain block hash and a proof. | ||
| /// @param targetBlockHash The block hash of the target chain. | ||
| /// @param targetStateCommitment The block hash of the target chain. |
There was a problem hiding this comment.
Align NatSpec with state‑commitment terminology.
The parameter description still says “block hash,” which can be confusing after the rename.
📝 Suggested fix
- /// `@param` targetStateCommitment The block hash of the target chain.
+ /// `@param` targetStateCommitment The state commitment (block hash) of the target chain.📝 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.
| /// @notice Verify a storage slot given a target chain block hash and a proof. | |
| /// @param targetBlockHash The block hash of the target chain. | |
| /// @param targetStateCommitment The block hash of the target chain. | |
| /// `@notice` Verify a storage slot given a target chain block hash and a proof. | |
| /// `@param` targetStateCommitment The state commitment (block hash) of the target chain. |
🤖 Prompt for AI Agents
In `@src/contracts/provers/arbitrum/ChildToParentProver.sol` around lines 69 - 70,
Update the NatSpec to replace “block hash” wording with state‑commitment
terminology for the verifyStorageSlot comment: change the summary to mention a
target chain state commitment (not block hash) and update the `@param` description
for targetStateCommitment to something like “The state commitment of the target
chain” so it matches the renamed symbol; ensure this change appears in the
verifyStorageSlot function comment in ChildToParentProver (and any other
comments referencing targetStateCommitment).
| /// @notice Verify a storage slot given a target chain block hash and a proof. | ||
| /// @param targetBlockHash The block hash of the target chain. | ||
| /// @param targetStateCommitment The block hash of the target chain. | ||
| /// @param input ABI encoded (bytes blockHeader, address account, uint256 slot, bytes accountProof, bytes storageProof) |
There was a problem hiding this comment.
Clarify NatSpec for targetStateCommitment.
The parameter description still says “block hash,” which can be ambiguous under the new terminology.
📝 Suggested fix
- /// `@param` targetStateCommitment The block hash of the target chain.
+ /// `@param` targetStateCommitment The state commitment (block hash) of the target chain.🤖 Prompt for AI Agents
In `@src/contracts/provers/zksync/ChildToParentProver.sol` around lines 69 - 71,
The NatSpec for the verify storage slot function uses ambiguous wording for the
targetStateCommitment parameter (currently described as "block hash"); update
the comment for targetStateCommitment in ChildToParentProver.sol to clearly
state exactly what value is expected (e.g., "state commitment / state root of
the target chain block" or "state root of the target chain block header"), so
callers know whether to pass a block hash, block header root, or state
commitment; modify the `@param` line for targetStateCommitment to use the precise
term ("state root" or "state commitment") and briefly indicate it must
correspond to the blockHeader provided in the input.
| /// @param homeStateCommitment The block hash of the home chain (L1) containing the gateway ZkChain state. | ||
| /// @param input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes storageProof). | ||
| /// - rlpBlockHeader: RLP-encoded block header of the home chain. | ||
| /// - batchNumber: The batch number for which to retrieve the L2 logs root hash. | ||
| /// - storageProof: Storage proof for the storage slot containing the L2 logs root hash. | ||
| /// @return targetL2LogsRootHash The L2 logs root hash for the specified batch number. | ||
| function verifyTargetBlockHash(bytes32 homeBlockHash, bytes calldata input) | ||
| /// - proof: Storage proof for the storage slot containing the L2 logs root hash. |
There was a problem hiding this comment.
Update NatSpec to match the decoded input tuple.
verifyTargetStateCommitment decodes accountProof, but the docstring only lists a 3‑tuple and mentions a generic “proof.” This can mislead integrators assembling calldata.
📝 Suggested fix
- /// `@param` input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes storageProof).
+ /// `@param` input ABI encoded tuple: (bytes rlpBlockHeader, uint256 batchNumber, bytes accountProof, bytes storageProof).
- /// - proof: Storage proof for the storage slot containing the L2 logs root hash.
+ /// - accountProof: Account proof for the gateway ZkChain contract.
+ /// - storageProof: Storage proof for the slot containing the L2 logs root hash.🤖 Prompt for AI Agents
In `@src/contracts/provers/zksync/ParentToChildProver.sol` around lines 115 - 119,
Update the NatSpec for verifyTargetStateCommitment to reflect the actual decoded
calldata: change the `@param` for input to specify it is an ABI-encoded tuple
(bytes rlpBlockHeader, uint256 batchNumber, bytes accountProof) and document
that accountProof itself is an ABI-encoded tuple which the function decodes
(e.g., (bytes rlpAccountProof, address account, bytes32 storageSlot, bytes
storageProof) — or whatever exact fields verifyTargetStateCommitment extracts
from accountProof); mention that rlpBlockHeader is the home-chain RLP block
header and storageProof is the storage proof for the specified storageSlot so
integrators know how to assemble calldata for verifyTargetStateCommitment and
the accountProof decoding performed inside that function.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/contracts/provers/linea/ParentToChildProver.sol (1)
99-147: Documentaccountandslotin the SMT proof input tuple.
verifyStorageSlotdecodesaddress accountanduint256 slot, but the input format list doesn’t mention them, which can mislead integrators assembling calldata.📝 Proposed doc fix
- /// Input format from linea_getProof: + /// Input format (ABI-encoded tuple): + /// - account: the L2 account address queried in linea_getProof + /// - slot: the L2 storage slot queried in linea_getProof /// - accountLeafIndex: from accountProof.leafIndex /// - accountProof: from accountProof.proof.proofRelatedNodes (42 elements) /// - accountValue: from accountProof.proof.value (192 bytes) /// - storageLeafIndex: from storageProofs[0].leafIndex /// - proof: from storageProofs[0].proof.proofRelatedNodes (42 elements) /// - storageValue: the claimed storage value (32 bytes, to verify)
🧹 Nitpick comments (1)
.gitmodules (1)
16-18: Use canonical OpenZeppelin repo URL and pin a specific version.The canonical GitHub URL uses uppercase
OpenZeppelin. Additionally, best practice is to pin a specific version/tag in the submodule configuration rather than pointing to the repository without a ref. The latest audited version for production is5.4.0.♻️ Proposed fix
[submodule "lib/openzeppelin-contracts-upgradeable"] path = lib/openzeppelin-contracts-upgradeable - url = https://github.com/openzeppelin/openzeppelin-contracts-upgradeable + url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeableConsider also pinning a version tag, e.g.,
branch = v5.4.0or via.gitmodulesversion pinning, to ensure reproducible builds.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/contracts/provers/scroll/ChildToParentProver.sol`:
- Around line 2-5: The project’s Solidity pragma (e.g., in
ChildToParentProver.sol: "pragma solidity 0.8.30") and Foundry config target
0.8.30 but the npm solc package is still pinned to 0.8.26; update package.json
to require solc version 0.8.30 (and run npm install to regenerate
package-lock.json) so Hardhat uses the same compiler as the pragma/Foundry
configuration.
| pragma solidity 0.8.30; | ||
|
|
||
| import {ProverUtils} from "../../libraries/ProverUtils.sol"; | ||
| import {IBlockHashProver} from "../../interfaces/IBlockHashProver.sol"; | ||
| import {IStateProver} from "../../interfaces/IStateProver.sol"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check for common Solidity config files
echo "=== Checking for Solidity config files ==="
mapfile -t files < <(fd -a -t f 'foundry\.toml|hardhat\.config\.(js|ts)|truffle-config\.js|brownie-config\.yaml|remappings\.txt|package\.json|.solcrc' 2>/dev/null || true)
if [ "${`#files`[@]}" -gt 0 ]; then
echo "Found config files:"
printf '%s\n' "${files[@]}"
echo ""
echo "=== Searching for Solidity/solc version specifications ==="
rg -n 'solidity|solc|0\.8\.|pragma' "${files[@]}" || true
else
echo "No common Solidity config files found."
fi
# Also check root-level for any version/compiler specifications
echo ""
echo "=== Checking root directory for compiler configs ==="
ls -la | grep -E '\.(toml|yaml|yml|json)$' || true
# Check if there's a foundry.toml in root
if [ -f "foundry.toml" ]; then
echo ""
echo "=== foundry.toml content ==="
cat foundry.toml
fiRepository: openintentsframework/broadcaster
Length of output: 192
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Root directory config files ==="
ls -la | grep -E '\.(toml|json|yaml|yml)$' || true
echo ""
echo "=== Checking foundry.toml ==="
if [ -f "foundry.toml" ]; then
cat foundry.toml
else
echo "foundry.toml not found"
fi
echo ""
echo "=== Checking package.json for solc version ==="
if [ -f "package.json" ]; then
cat package.json | head -50
else
echo "package.json not found"
fi
echo ""
echo "=== Search for solidity version constraints ==="
find . -maxdepth 2 -type f \( -name "*.toml" -o -name "*.json" -o -name "*.yaml" \) 2>/dev/null | head -20 | xargs -I {} sh -c 'echo "=== {} ===" && cat "{}" 2>/dev/null | grep -i "solidity\|solc" || true'Repository: openintentsframework/broadcaster
Length of output: 6872
Update npm solc package to 0.8.30 to match pragma and Foundry config.
Foundry is correctly configured for solc 0.8.30, but the npm solc package is pinned to 0.8.26 in package-lock.json. Hardhat will use the npm package and may fail to compile. Update package.json to require solc 0.8.30.
🤖 Prompt for AI Agents
In `@src/contracts/provers/scroll/ChildToParentProver.sol` around lines 2 - 5, The
project’s Solidity pragma (e.g., in ChildToParentProver.sol: "pragma solidity
0.8.30") and Foundry config target 0.8.30 but the npm solc package is still
pinned to 0.8.26; update package.json to require solc version 0.8.30 (and run
npm install to regenerate package-lock.json) so Hardhat uses the same compiler
as the pragma/Foundry configuration.
This PR updates all contracts due to changes in ERC 7888 and add docs to it.
Summary by CodeRabbit
New Features
Documentation
Refactor
User-facing change
✏️ Tip: You can customize this high-level summary in your review settings.