feat(scripts): include ribasushi-rpc-checks - #7477
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThe PR adds a Docker Compose workflow for RPC checks. It prepares a chain snapshot, starts Forest with backfill settings and health checks, then runs Ruby RPC checks over the calculated height range. ChangesRPC checks workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant SnapshotService
participant Forest
participant RPCChecks
SnapshotService->>SnapshotService: Select and prepare snapshot
SnapshotService->>Forest: Provide snapshot and backfill settings
Forest-->>RPCChecks: Report healthy status
RPCChecks->>Forest: Run RPC checks for START to END
Possibly related issues
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
scripts/tests/ribasushi-rpc-checks/docker-compose.yaml (1)
39-39: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winPin the Forest and RPC-checks images.
The
:latesttags can change between CI runs. Different binaries can then run without a source change, including an incompatible Forest and checker pair. Pin both images to tested release references or immutable digests. Docker documents digests as immutable while tags can be changed. (docs.docker.com)Also applies to: 61-61
🤖 Prompt for 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. In `@scripts/tests/ribasushi-rpc-checks/docker-compose.yaml` at line 39, Replace the mutable :latest tags for both the Forest image and the RPC-checks image in the Docker Compose configuration with tested release references or immutable digests, keeping the Forest/checker versions compatible.
🤖 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 `@scripts/tests/ribasushi-rpc-checks/docker-compose.yaml`:
- Around line 1-73: Format the docker-compose YAML with Prettier using its write
mode, then rerun the YAML lint job to confirm no formatting differences remain.
- Around line 21-27: Validate DAYS_AGO, EPOCHS, and the extracted head as
non-negative integers before performing arithmetic; reject malformed values and
EPOCHS=0. In the range-building logic, require EPOCHS to be at least 1 and no
greater than head, aborting with a clear error when the constraints fail.
- Around line 54-58: Extend the startup grace period for the forest healthcheck
by adding a start_period or increasing retries so failed checks remain tolerated
through the maximum EPOCHS index backfill duration. Keep the existing test,
interval, timeout, and service_healthy dependency behavior unchanged.
- Around line 22-23: Update the snapshot retrieval commands in the Docker
Compose script: add connection and overall timeouts plus retries to both curl
calls, and download archive content to a temporary file before atomically moving
it to the final path. Ensure interrupted downloads cannot leave a final file
that later passes the existing -f check.
---
Nitpick comments:
In `@scripts/tests/ribasushi-rpc-checks/docker-compose.yaml`:
- Line 39: Replace the mutable :latest tags for both the Forest image and the
RPC-checks image in the Docker Compose configuration with tested release
references or immutable digests, keeping the Forest/checker versions compatible.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d345d1e0-310d-40bd-a2e9-f5fd3b3ae22c
📒 Files selected for processing (1)
scripts/tests/ribasushi-rpc-checks/docker-compose.yaml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@CHANGELOG.md`:
- Around line 40-41: Update the CHANGELOG entry for the ribasushi RPC checks to
reference issue `#7270` instead of PR `#7477`, using the corresponding issue link
while preserving the existing description.
🪄 Autofix
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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5bdcbcfa-ca4f-4b25-97f1-08b5d397f574
📒 Files selected for processing (1)
CHANGELOG.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted filessee 7 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
LesnyRumcajs
left a comment
LesnyRumcajs
left a comment
There was a problem hiding this comment.
Please read the linked issue. Something is missing.
|
|
||
| # only downloads if the snapshot is not present | ||
| [ -f "/snapshots/$$file" ] || | ||
| curl -sSfL --connect-timeout 10 --retry 3 -o "/snapshots/$$file" "$$url" |
There was a problem hiding this comment.
cURL is pretty bad at downloading large files such as snapshots - a better option would be to use aria2c with, e.g., -x5 set.
| - | | ||
| apk add --no-cache curl jq >/dev/null | ||
|
|
||
| # validates DAYS_AGO and EPOCHS |
There was a problem hiding this comment.
pretty complex on its own - best to put it into a dedicated bash script so that shellcheck linter can nicely check it. It's also pretty obscure unreadable bash (not your fault, it's just bash) with regexes, so plenty of comments are needed to assert the logic is sound.
| [ "$$EPOCHS" -ge 1 ] || { echo "EPOCHS must be at least 1"; exit 1; } | ||
|
|
||
| day=$$(date -u -d "@$$(( $$(date -u +%s) - DAYS_AGO * 86400 ))" +%F) | ||
| url=$$(curl -sSf --connect-timeout 10 --retry 3 \ |
There was a problem hiding this comment.
unless a flag is obvious, e.g., rm -rf, I recommend using full versions of the params, e.g., --silent instead of -s. Then a reader doesn't need a PhD in cURL to parse the file.
|
|
||
| # The RPC port opens only after the backfill finishes, so "healthy" means ready. | ||
| forest: | ||
| image: ghcr.io/chainsafe/forest:latest |
There was a problem hiding this comment.
Re-think your options. Hint: latest is a bad option - why?
https://docs.forest.chainsafe.io/knowledge_base/docker_tips#tags
| - | | ||
| . /snapshots/env | ||
| exec forest-tool api serve "/snapshots/$$SNAPSHOT" --chain "$$CHAIN" \ | ||
| --port 2345 --height 0 --index-backfill-epochs "$$BACKFILL" |
| FOREST_RPC_URL: forest:2345/rpc/v1 | ||
| volumes: | ||
| - ./snapshots:/snapshots:ro | ||
| entrypoint: ["/bin/sh", "-euc"] |
There was a problem hiding this comment.
can you just use the image directly?
| healthcheck: | ||
| test: ["CMD", "forest-cli", "chain", "head"] | ||
| interval: 15s | ||
| timeout: 10s | ||
| retries: 480 |
There was a problem hiding this comment.
So... potentially 20h for the probe to fail?
2 participants
Summary of changes
Changes introduced in this pull request:
scripts/ribasushi-rpc-checks/docker-compose.yamlit contains 3 services:forest-archive.chainsafe.devand download based onDAYS_AGOenv var (default 2 days ago)ghcr.io/chainsafe/forestand runforest-tool api serve --index-backfill-epochswhere the amount of epochs to backfill is based onEPOCHSenv var (default 1000 epochs)ghcr.io/chainsafe/forest-rpc-checks:latestto exec thecheck_rpc.rbagains the ribasushi external datasetReference issue to close (if applicable)
Closes #7270
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit