chore: setup local scripts for crisp [skip-line-limit]#1580
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRefactors EVM extractors to accept full topic slices and updates all domain extractors/tests to match; adds local dev orchestration (tmux-based start, run_service/setup_nodes scripts), VS Code tasks and MCP config, and refreshes localhost deployment metadata and deployed contract block numbers. ChangesEVM Event Extraction and Local Development Infrastructure
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@deploy/local/run_service.sh`:
- Line 40: The polling loops (the SIGNAL_FILE file-wait using while [ ! -f
"${SIGNAL_FILE}" ] and the curl readiness loops for ports 8545, 13151, 4000 and
the combined client check) must be bounded with a timeout/retry counter; modify
each loop to track elapsed time or a retry count, break and exit non-zero with a
clear error message if the dependency doesn’t become ready within the timeout,
and return success if it becomes ready; ensure you update the SIGNAL_FILE loop
and each curl loop (and the combined client condition) to use the same
configurable TIMEOUT/RETRY variables and log which dependency failed before
exiting.
- Line 18: The script currently uses SIGNAL_FILE="/tmp/crisp-dev-daemon-ready"
and simple existence checks in the cn1|cn2|cn3|cn4|cn5, client, program and
server readiness loops which can be satisfied by a stale marker and have no
timeout; fix by proactively removing any existing SIGNAL_FILE at script start
(rm -f $SIGNAL_FILE), ensure the daemon creates the marker only when ready and
the script registers a trap to unlink SIGNAL_FILE on exit, update the readiness
checks that reference SIGNAL_FILE to also validate freshness (e.g., check
modification time or a short-lived token) and add a bounded timeout to each wait
loop (cn1..cn5, client, program, server) so they fail fast if prerequisites
never appear.
In `@deploy/local/setup_nodes.sh`:
- Around line 37-39: The wait loop for Anvil (the `until curl -s
http://localhost:8545 >/dev/null 2>&1; do sleep 1; done` loop in setup_nodes.sh)
can block forever; change it to respect a timeout or max-retries: add a
WAIT_TIMEOUT_SECONDS or MAX_RETRIES variable, track elapsed time or a counter in
the loop, and exit non-zero with an error log (e.g., echo "[deploy+setup] Anvil
did not start within $WAIT_TIMEOUT_SECONDS seconds") if the limit is reached;
update the same block (the echo, the `until curl ...` check and the final "Anvil
is ready." echo) so the script fails fast instead of hanging.
In `@deploy/local/start.sh`:
- Around line 240-247: The printed VS Code task names in start.sh are incorrect:
replace the incorrect labels "CRISP: Deploy Contracts" and "CRISP: Ciphernodes"
(seen in the echo lines) with the actual task labels defined in
.vscode/tasks.json (e.g. "CRISP: Deploy + Setup" and either "CRISP: Start All"
or the specific "CRISP: cn1"…"CRISP: cn5" entries you want to instruct users to
run), or alternatively add matching alias task names to tasks.json; update the
strings in start.sh so the echoed task names exactly match the unique task
labels ("CRISP: Deploy + Setup", "CRISP: cn1", …, "CRISP: cn5", "CRISP: Start
All") used by VS Code.
- Around line 193-205: Tmux currently starts
dev_program.sh/dev_server.sh/dev_client.sh directly which bypasses the readiness
waits; change the tmux new-window commands (the ones launching "program",
"server", "client") to either invoke deploy/local/run_service.sh with the same
service names (so they use deploy/local/setup_nodes.sh and port/ready-file
waiting) or, if you must keep the current scripts, have the tmux setup create
the same readiness sentinel (/tmp/crisp-dev-daemon-ready) that run_service.sh
expects (mirror setup_nodes.sh behavior). Also update
launch_vscode_instructions() to emit the actual task labels present in
.vscode/tasks.json (e.g., "CRISP: Deploy + Setup" and "CRISP: cn1..cn5") instead
of the incorrect names, and relax check_deps() so it does not require cargo
unconditionally—only require cargo when install_enclave() will perform an
install (or call check_deps() after install_enclave()), referencing
check_deps(), install_enclave(), run_service.sh, deploy/local/setup_nodes.sh,
dev_program.sh, dev_server.sh, dev_client.sh, and launch_vscode_instructions().
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6db717cd-a145-47a4-b03e-a4e91a6d1c06
⛔ Files ignored due to path filters (1)
examples/CRISP/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
.vscode/mcp.json.vscode/tasks.jsoncrates/evm/src/actors/evm_parser.rscrates/evm/src/contracts.rscrates/evm/src/domain/bonding_registry_events.rscrates/evm/src/domain/ciphernode_registry_events.rscrates/evm/src/domain/enclave_events.rscrates/evm/src/domain/slashing_events.rscrates/evm/tests/integration.rsdeploy/local/run_service.shdeploy/local/setup_nodes.shdeploy/local/start.shexamples/CRISP/enclave.config.yamlexamples/CRISP/packages/crisp-contracts/deployed_contracts.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/events/src/enclave_event/mod.rs`:
- Line 547: Update the stale test e3_request_complete_without_e3_id_is_ignored
to reflect that EnclaveEventData::E3RequestComplete now returns Some(e3_id) (see
EnclaveEventData::E3RequestComplete(ref data) => Some(data.e3_id.clone())).
Change the expectation from RoutingDecision::Ignore to RoutingDecision::Process
with the returned e3_id and PostForward::Teardown, and rename/comment the test
to indicate the event is processed (not ignored); ensure the assertion matches
RequestRouter::route behavior and uses RoutingDecision::Process { e3_id: id,
post_forward: PostForward::Teardown }.
In `@deploy/local/run_service.sh`:
- Around line 55-56: The cleanup code currently unconditionally removes the
SIGNAL_FILE which causes other per-service tasks to delete the deploy-ready
signal; update each cleanup/removal site (the rm -f "$SIGNAL_FILE" invocations)
to run only when this invocation is the deploy task (e.g. check an explicit task
indicator such as "$1" == "deploy" or an environment variable like TASK or MODE
== "deploy"); modify the blocks around SIGNAL_FILE, including the other
occurrences noted (the blocks around the later rm -f calls), to guard removal
with that conditional so only deploy runs remove /tmp/crisp-dev-daemon-ready.
- Line 26: In wait_for_port and wait_for_file split the single-line local
declarations so dependent defaults are assigned after the variables they depend
on (e.g., set local host/port/file first, then set local label="${3:-$port}" or
label="${3:-$file}"); change the loop test from string comparison to arithmetic
(e.g., use while (( attempt <= max )) or quote variables in [ ... ] like [
"$attempt" -le "$max" ] ) to satisfy SC2318 and SC2086 and ensure correct
defaulting and comparison behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: fe593058-5fe0-4264-bf98-c633740607b0
📒 Files selected for processing (3)
crates/events/src/enclave_event/mod.rsdeploy/local/run_service.shdeploy/local/start.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy/local/start.sh
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
VSCode Tasks to launch everything for Crisp.
Helps in diagnosing individual cipher nodes and testing
Summary by CodeRabbit
Refactor
Chores
Bug Fixes