Add scripts needed for taiko-client#152
Conversation
WalkthroughA new Makefile replaces the previous justfile, providing build and utility targets for project management. Additionally, a new Solidity deployment script, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DeployL1Contracts (Script)
participant Env
participant BlobRefRegistry
participant TaikoInbox
participant MockVerifier
participant SignalService
participant CheckpointTracker
participant Console
User->>DeployL1Contracts: run()
DeployL1Contracts->>Env: Read config parameters
Env-->>DeployL1Contracts: Return parameters
DeployL1Contracts->>BlobRefRegistry: Deploy
DeployL1Contracts->>TaikoInbox: Deploy (with params)
DeployL1Contracts->>MockVerifier: Deploy
DeployL1Contracts->>SignalService: Deploy
DeployL1Contracts->>CheckpointTracker: Deploy (with params)
DeployL1Contracts->>Console: Log configuration and addresses
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
scripts/DeployMinimalL1.s.sol (6)
18-20: Docstring mismatch: script deploys multiple contracts, not just TaikoInbox-/// @notice Script to deploy the TaikoInbox contract +/// @notice Script to deploy a minimal set of Taiko L1 contracts
30-35: Log the genesis hash for reproducibility/debuggingSeeing the exact
genesisused in a run is helpful when replaying deployments.console.log("Inclusion Delay:", inclusionDelay); + console.log("Genesis Hash:"); + console.logBytes32(genesis); console.log("------------------------");
37-53: Label deployed contract addresses in the VM for clearer tracesLabels greatly improve readability in script traces and test logs without altering behavior.
BlobRefRegistry blobRefRegistry = new BlobRefRegistry(); + vm.label(address(blobRefRegistry), "BlobRefRegistry"); TaikoInbox taikoInbox = new TaikoInbox( lookaheadAddr, address(blobRefRegistry), maxAnchorBlockIdOffset, inclusionDelay ); + vm.label(address(taikoInbox), "TaikoInbox"); MockVerifier verifier = new MockVerifier(); + vm.label(address(verifier), "MockVerifier"); SignalService signalService = new SignalService(); + vm.label(address(signalService), "SignalService"); CheckpointTracker tracker = new CheckpointTracker( genesis, address(taikoInbox), address(verifier), address(signalService) ); + vm.label(address(tracker), "CheckpointTracker");
55-61: Also log the MockVerifier address for completenessconsole.log("TaikoInbox deployed at:", address(taikoInbox)); console.log("CheckpointTracker deployed at:", address(tracker)); console.log("BlobRefRegistry address:", address(blobRefRegistry)); console.log("SignalService address:", address(signalService)); + console.log("MockVerifier address:", address(verifier)); console.log("------------------------");
25-29: Confirm whether zerolookaheadAddris acceptable in your target environmentsYou default
lookaheadAddrtoaddress(0). Fine for local/dev, but likely undesirable in staging/prod. Consider requiring it to be set or gating by network.If you want, I can add an environment gate (e.g., allow zero only when
CHAIN_IDis a known dev network) or arequire(lookaheadAddr != address(0), "...").
20-20: Optional: Align contract name with file name for consistencyIf you intend to invoke it as
forge script scripts/DeployMinimalL1.s.sol:DeployMinimalL1, rename the contract accordingly; otherwise consider renaming the file.-contract DeployL1Contracts is Script { +contract DeployMinimalL1 is Script {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Makefile(1 hunks)justfile(0 hunks)scripts/DeployMinimalL1.s.sol(1 hunks)
💤 Files with no reviewable changes (1)
- justfile
🧰 Additional context used
🪛 checkmake (0.2.2)
Makefile
[warning] 25-25: Missing required phony target "all"
(minphony)
[warning] 25-25: Missing required phony target "test"
(minphony)
| # Makefile converted from justfile | ||
|
|
||
| # Get signal slot for a signal and sender | ||
| # Usage: make get-signal-slot signal=<signal> sender=<sender> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add default all goal and test target; update PHONY to satisfy checkmake and typical expectations
Static analysis flagged missing “all” and “test” phony targets. Also set a sensible default goal.
# Makefile converted from justfile
+
+.DEFAULT_GOAL := all
+
+all: compile
@@
-# Default target
-.PHONY: get-signal-slot create-sample-signal-proof create-sample-deposit-proof compile clean
+# Default targets
+test:
+ forge test -vvv
+
+.PHONY: all test get-signal-slot create-sample-signal-proof create-sample-deposit-proof compile cleanIf you prefer not to define a test target here, we can suppress the checkmake rule instead—confirm your preference.
Also applies to: 20-25
🏁 Script executed:
#!/bin/bash
# Display the first 20 lines of the Makefile to verify existing goals and PHONY targets
if [ -f Makefile ]; then
sed -n '1,20p' Makefile
else
echo "Makefile not found in the repository root."
fiLength of output: 641
🏁 Script executed:
#!/bin/bash
# Check for existing .PHONY declarations
echo "Lines with .PHONY:"
grep -n '^\.PHONY' Makefile || echo "No .PHONY target defined"
echo
echo "Full Makefile content (first 200 lines):"
sed -n '1,200p' MakefileLength of output: 1049
Add a default all goal and a test target to Makefile
The Makefile currently defaults to the first target (get-signal-slot) and lacks a test target. Defining a sensible default and including test will satisfy checkmake and common expectations.
• File: Makefile
– Insert at top (after the header comment):
```makefile
.DEFAULT_GOAL := all
all: compile
test:
forge test -vvv
```
– Update the existing .PHONY: line (currently on line 25) to include all and test:
diff # Default target -.PHONY: get-signal-slot create-sample-signal-proof create-sample-deposit-proof compile clean +.PHONY: all test get-signal-slot create-sample-signal-proof create-sample-deposit-proof compile clean
🤖 Prompt for AI Agents
In Makefile lines 1 to 4, add a default goal by inserting '.DEFAULT_GOAL := all'
and define an 'all' target that depends on 'compile' at the top after the header
comment. Also, add a 'test' target that runs 'forge test -vvv'. Then, update the
existing .PHONY declaration around line 25 to include 'all' and 'test' as phony
targets.
| get-signal-slot: | ||
| cargo run --bin signal_slot $(signal) $(sender) | ||
|
|
There was a problem hiding this comment.
Fix Cargo arg parsing: pass positional args after --
Without --, Cargo may treat $(signal)/$(sender) as Cargo flags, breaking the command.
get-signal-slot:
- cargo run --bin signal_slot $(signal) $(sender)
+ cargo run --bin signal_slot -- $(signal) $(sender)🤖 Prompt for AI Agents
In the Makefile at lines 5 to 7, the cargo run command passes positional
arguments $(signal) and $(sender) without using `--`, causing Cargo to
misinterpret them as flags. Fix this by adding `--` before the positional
arguments in the command line to ensure Cargo treats them correctly as arguments
to the binary.
| # Create a sample signal proof | ||
| create-sample-signal-proof: | ||
| cargo run --bin sample_signal_proof > test/SignalService/SampleProof.t.sol | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make proof target idempotent: ensure output dir exists
Create the directory before redirecting, or the redirect will fail on a clean checkout.
create-sample-signal-proof:
- cargo run --bin sample_signal_proof > test/SignalService/SampleProof.t.sol
+ mkdir -p test/SignalService
+ cargo run --bin sample_signal_proof > test/SignalService/SampleProof.t.sol🤖 Prompt for AI Agents
In the Makefile at lines 8 to 11, the create-sample-signal-proof target
redirects output to a file in a directory that may not exist, causing failure on
a clean checkout. Modify the target to first create the output directory
(test/SignalService) if it does not exist before running the cargo command and
redirecting output. This ensures the target is idempotent and will not fail due
to missing directories.
| # Create a sample deposit proof | ||
| create-sample-deposit-proof: | ||
| cargo run --bin sample_deposit_proof > test/ETHBridge/SampleDepositProof.t.sol | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same here—ensure ETHBridge test dir exists before redirect
create-sample-deposit-proof:
- cargo run --bin sample_deposit_proof > test/ETHBridge/SampleDepositProof.t.sol
+ mkdir -p test/ETHBridge
+ cargo run --bin sample_deposit_proof > test/ETHBridge/SampleDepositProof.t.sol🤖 Prompt for AI Agents
In the Makefile around lines 12 to 15, the target create-sample-deposit-proof
redirects output to test/ETHBridge/SampleDepositProof.t.sol without ensuring the
ETHBridge directory exists. Modify the target to first create the test/ETHBridge
directory if it does not exist before running the cargo command and redirecting
output, to prevent errors due to missing directories.
| # Compile with forge | ||
| compile: | ||
| forge build --build-info --extra-output storage-layout |
There was a problem hiding this comment.
Incorrect forge extra-output key: use storageLayout (camelCase), not storage-layout
Solc’s JSON output selector is storageLayout; hyphenated form won’t be recognized.
compile:
- forge build --build-info --extra-output storage-layout
+ forge build --build-info --extra-output storageLayout📝 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.
| # Compile with forge | |
| compile: | |
| forge build --build-info --extra-output storage-layout | |
| # Compile with forge | |
| compile: | |
| forge build --build-info --extra-output storageLayout |
🤖 Prompt for AI Agents
In the Makefile at lines 16 to 18, the forge build command uses an incorrect
extra-output key `storage-layout`. Replace `storage-layout` with the correct
camelCase key `storageLayout` to match solc’s JSON output selector and ensure
proper recognition.
| import {IBlobRefRegistry} from "../src/blobs/IBlobRefRegistry.sol"; | ||
| import {CheckpointTracker} from "../src/protocol/CheckpointTracker.sol"; | ||
|
|
||
| import {ILookahead} from "../src/protocol/ILookahead.sol"; | ||
| import {IProposerFees} from "../src/protocol/IProposerFees.sol"; | ||
| import {TaikoInbox} from "../src/protocol/taiko_alethia/TaikoInbox.sol"; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unused imports to avoid compiler warnings
IBlobRefRegistry, ILookahead, and IProposerFees are unused.
import {BlobRefRegistry} from "../src/blobs/BlobRefRegistry.sol";
-import {IBlobRefRegistry} from "../src/blobs/IBlobRefRegistry.sol";
import {CheckpointTracker} from "../src/protocol/CheckpointTracker.sol";
-import {ILookahead} from "../src/protocol/ILookahead.sol";
-import {IProposerFees} from "../src/protocol/IProposerFees.sol";
import {TaikoInbox} from "../src/protocol/taiko_alethia/TaikoInbox.sol";Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In scripts/DeployMinimalL1.s.sol around lines 5 to 11, the imports
IBlobRefRegistry, ILookahead, and IProposerFees are not used anywhere in the
file. Remove these three unused import statements to prevent compiler warnings
and keep the code clean.
1 participant
Summary by CodeRabbit
New Features
Chores