ci: add liveness test#81
Conversation
WalkthroughA new GitHub Actions workflow for Rollkit integration testing was added, automating end-to-end validation of chain startup and block production. Additionally, the lint workflow was modified to remove the yamllint job, leaving only Go and Markdown linting active. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Ubuntu Runner
participant Rollkit Repo
participant Ignite CLI
participant Local DA Node
participant GM Chain
GitHub Actions->>Ubuntu Runner: Trigger workflow (push/PR/dispatch)
Ubuntu Runner->>Rollkit Repo: Checkout repo
Ubuntu Runner->>Ubuntu Runner: Setup Go environment
Ubuntu Runner->>Ubuntu Runner: Install Ignite CLI
Ubuntu Runner->>Rollkit Repo: Clone Rollkit
Ubuntu Runner->>Local DA Node: Start DA node (background)
Ubuntu Runner->>Ignite CLI: Scaffold new blockchain
Ubuntu Runner->>Rollkit Repo: Install Rollkit app
Ubuntu Runner->>Ignite CLI: Add Rollkit integration
Ubuntu Runner->>GM Chain: Build and initialize chain
Ubuntu Runner->>GM Chain: Replace module, tidy, rebuild
Ubuntu Runner->>GM Chain: Start chain (background)
Ubuntu Runner->>GM Chain: Poll logs, wait for 5 blocks
GM Chain-->>Ubuntu Runner: Log output
Ubuntu Runner->>Local DA Node: Kill process (on completion/failure)
Ubuntu Runner->>GM Chain: Kill process (on completion/failure)
Suggested reviewers
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
.github/workflows/integration_test.yml (5)
32-42: Add Health Check for Local DA NodeRelying on
sleepmay not guarantee the DA node is ready. Consider polling a known port or health endpoint:run: | go run . & echo "DA_PID=$!" >> $GITHUB_ENV - sleep 3 + # wait for DA to listen on port 26657 (timeout after 15s) + timeout=15 + until nc -z localhost 26657 || [ $timeout -le 0 ]; do + sleep 1 + timeout=$((timeout-1)) + done + if [ $timeout -le 0 ]; then + echo "Local DA did not start in time" + exit 1 + fiThis ensures the DA is actually ready before proceeding.
62-79: Resolve TODO for Rollkit Module ReplacementThe commented-out
go mod edit -replaceforgithub.com/rollkit/rollkitshould be finalized to ensure reproducibility. You can leverage the PR’s SHA:- # go mod edit -replace github.com/rollkit/rollkit=github.com/rollkit/rollkit@main + go mod edit -replace github.com/rollkit/rollkit=github.com/rollkit/rollkit@${{ github.sha }}This removes the TODO and pins the module to the exact head of the branch.
83-113: Enhance Block Detection with Structured ParsingGrep’ing for
"Block executed successfully"can miss log variations. Since JSON logging is enabled, consider usingjqto count events more reliably:- BLOCKS_FOUND=$(grep -c "Block executed successfully" chain.log || true) + BLOCKS_FOUND=$(jq -r 'select(.msg=="Block executed successfully")' chain.log | wc -l || echo 0)This approach ensures you only count valid block events.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
124-135: Graceful Process TerminationUsing
kill -9forces immediate termination. For a cleaner shutdown, send SIGTERM first and then SIGKILL if necessary:- kill -9 $CHAIN_PID || true + kill $CHAIN_PID || true + sleep 1 + kill -9 $CHAIN_PID || trueApply the same pattern for
DA_PID.
100-109: Remove Trailing SpacesLines 102 and 109 have trailing whitespace, which triggers YAML lint errors. Please strip them:
- ATTEMPT=$((ATTEMPT+1)) + ATTEMPT=$((ATTEMPT+1)) ... - fi + fi🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/integration_test.yml(1 hunks).github/workflows/lint.yml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/lint.yml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/integration_test.yml
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test with Rollkit Chain
🔇 Additional comments (4)
.github/workflows/integration_test.yml (4)
1-9: CI Trigger Configuration Looks SolidThe workflow triggers on pushes and pull requests targeting
main, as well as manual dispatch, aligning well with the goal of always testing the main branch. No adjustments needed here.
17-20: Ensure Full Git History for Go ModulesUsing
actions/checkout@v4withfetch-depth: 0correctly pulls the full repository history, which is necessary for module replacements viago mod edit. Nice catch.
21-26: Go Setup with Caching is Correct
actions/setup-go@v5withgo-version-fileandcache: trueoptimizes both version pinning and dependency caching. This is best practice.
44-60: Scaffolding and Chain Preparation Steps Are ClearThe steps to scaffold the chain, install the Rollkit app at a specific commit, build, and initialize are well-ordered and correctly pinned. No concerns here.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/integration_test.yml (2)
29-29:⚠️ Potential issueFix the Ignite CLI install URL
The trailing “!” in the URL will cause a 404 and break the CI.- curl -sSL https://get.ignite.com/cli! | bash + curl -sSL https://get.ignite.com/cli | bash
27-30: 🛠️ Refactor suggestionPin Ignite CLI version or use the official GitHub Action
Installing without a version risks unexpected breakages. Consider one of the following:
- Pin to a release:
- name: Install Ignite CLI - run: | - curl -sSL https://get.ignite.com/cli | bash + uses: ignite-core/cli-action@v1 + with: + version: '1.24.0'
- Or specify the version in the curl URL:
- curl -sSL https://get.ignite.com/cli | bash + curl -sSL https://get.ignite.com/cli@v1.24.0 | bash
🧹 Nitpick comments (1)
.github/workflows/integration_test.yml (1)
102-102: Remove trailing spaces
YAML lint reports trailing spaces on these blank lines—please delete them to clean up the workflow.- # (line 102 has only spaces) - ... - # (line 109 has only spaces) -Also applies to: 109-109
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 102-102: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/integration_test.yml
[error] 102-102: trailing spaces
(trailing-spaces)
[error] 109-109: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test with Rollkit Chain
tac0turtle
left a comment
tac0turtle
left a comment
There was a problem hiding this comment.
what is the reasoning to have this in ci vs a script to run locally too?
I do not want the script to mess with my local environment. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/integration_test.yml (1)
57-75: 🛠️ Refactor suggestionPlease resolve the commented TODO for rollkit replacement
Line 67 leaves a placeholder for replacinggithub.com/rollkit/rollkitwith a specific commit. For reproducibility, either remove this TODO or update it, for example:- # TODO(@julienrbrt) this should be replaced with a specific commit hash - # go mod edit -replace github.com/rollkit/rollkit=github.com/rollkit/rollkit@v0.14.2-... + go mod edit -replace github.com/rollkit/rollkit=github.com/rollkit/rollkit@${GITHUB_SHA}
🧹 Nitpick comments (5)
.github/workflows/integration_test.yml (5)
11-17: Optional: add concurrency control to cancel in-flight jobs
To avoid redundant runs when multiple commits land in quick succession, consider adding:rollkit-test: concurrency: group: integration-test-${{ github.ref }} cancel-in-progress: trueThis will conserve runner resources by cancelling previous tests on the same ref.
29-32: Optional: use the official Ignite CLI GitHub Action
Piping fromcurlworks, but the ignite-core/cli-action provides a cleaner, faster install:- uses: ignite-core/cli-action@latestThis avoids network pipes and ensures the latest CLI is available.
33-43: Optional: add a readiness check for the Local DA
Relying onsleep 3may be fragile if startup varies. Consider polling a health endpoint (e.g., HTTP onlocalhost:26657) or checking the port withncto confirm the DA is ready before proceeding.
101-101: Remove trailing spaces
Line 101 contains trailing whitespace, which causes YAML lint errors. Please trim it.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 101-101: trailing spaces
(trailing-spaces)
108-108: Remove trailing spaces
Line 108 contains trailing whitespace, which causes YAML lint errors. Please trim it.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 108-108: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/integration_test.yml (1)
Learnt from: julienrbrt
PR: rollkit/go-execution-abci#81
File: .github/workflows/integration_test.yml:28-30
Timestamp: 2025-05-07T12:20:24.267Z
Learning: For the rollkit/go-execution-abci repository, the maintainer prefers using the latest version of Ignite CLI rather than pinning to a specific version during the current development phase. Version tags will be added later when Rollkit has a tag.
🪛 YAMLlint (1.35.1)
.github/workflows/integration_test.yml
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
.github/workflows/integration_test.yml (3)
1-9: Workflow triggers look correct
Triggers on pushes, pull requests tomain, and manual dispatch; aligns with the CI objective to ensure liveness tests run consistently.
19-28: Checkout & Go setup is solid
Usingactions/checkout@v4with full history andactions/setup-go@v5with caching enabled ensures reproducible builds.
45-56: Chain scaffolding and app installation look good
Scaffolding with--no-module/--skip-gitand installing the pinnedrollkitapp commit ensures deterministic integration tests.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
.github/workflows/integration_test.yml (5)
1-8: Add concurrency control to prevent overlapping runs
Without a concurrency group, rapid successive pushes can queue up multiple integration tests on the same branch, consuming runner capacity and potentially running stale code. Consider adding aconcurrencyblock:name: Rollkit Integration Test + concurrency: + group: Rollkit-Integration-Test-${{ github.ref }} + cancel-in-progress: true
29-32: Refine Ignite CLI install invocation
The current command pins to a specific version and includes an extraneous!, which may lead to confusion. To align with the maintainer’s preference for always using the latest Ignite CLI and remove the typo, replace with:- curl -sSL https://get.ignite.com/cli@v28.10.0! | bash + curl -sSL https://get.ignite.com/cli | bash
33-44: Harden the Local DA startup script
Adding strict error handling and a cleanup trap will make failures easier to debug and ensure the DA process is always killed on exit:run: | - # clone rollkit repository + set -euxo pipefail + trap '[[ -n "${DA_PID}" ]] && kill -9 "$DA_PID"' EXIT + # clone rollkit repository
82-90: Enhance Chain startup with strict mode
To catch errors early when starting the chain, addset -euxo pipefailat the top of this run block:run: | - cd gm + set -euxo pipefail + cd gm
101-102: Remove trailing whitespace to satisfy YAML lint
Lines 101, 102, and 108 contain trailing spaces that trigger yamllint errors. Please delete the extra spaces at the ends of these blank lines.Also applies to: 108-108
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 101-101: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/integration_test.yml(1 hunks).github/workflows/lint.yml(0 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/lint.yml
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/integration_test.yml (1)
Learnt from: julienrbrt
PR: rollkit/go-execution-abci#81
File: .github/workflows/integration_test.yml:28-30
Timestamp: 2025-05-07T12:20:24.267Z
Learning: For the rollkit/go-execution-abci repository, the maintainer prefers using the latest version of Ignite CLI rather than pinning to a specific version during the current development phase. Version tags will be added later when Rollkit has a tag.
🪛 YAMLlint (1.35.1)
.github/workflows/integration_test.yml
[error] 101-101: trailing spaces
(trailing-spaces)
[error] 108-108: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
.github/workflows/integration_test.yml (3)
19-27: Checkout and Go setup look solid
Usingfetch-depth: 0is necessary for module replacements, and caching with setup-go @ v5 is configured correctly.
45-56: Scaffold Rollkit Chain step is correct
The commands to scaffold the chain, install the app, and add Rollkit integration follow best practices and require no changes.
122-134: Cleanup step looks good
The finalif: always()block correctly ensures both the chain and DA processes are killed on exit.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/integration_test.yml (1)
84-123: 🛠️ Refactor suggestionImprove CI job error handling and cleanup
Adding
set -euxo pipefailand a trap in this block will ensure early failure on errors and cleanup of background processes without relying solely on the final step:run: | + set -euxo pipefail + trap '[[ -n "${CHAIN_PID}" ]] && kill -9 $CHAIN_PID; [[ -n "${DA_PID}" ]] && kill -9 $DA_PID' EXIT cd gm gmd start --rollkit.node.aggregator --log_format=json > chain.log 2>&1 & CHAIN_PID=$! echo "CHAIN_PID=$CHAIN_PID" >> $GITHUB_ENV ...This duplicates a suggestion made earlier but remains unaddressed.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
🧹 Nitpick comments (4)
.github/workflows/integration_test.yml (4)
33-43: Enhance robustness of the Local DA startupConsider adding
set -euxo pipefailat the start of this script block to fail fast on errors, and trap cleanup to ensure the DA process is killed if any step fails:run: | + set -euxo pipefail + trap '[[ -n "${DA_PID}" ]] && kill -9 $DA_PID' EXIT git clone https://github.com/rollkit/rollkit --depth 1 cd rollkit/da/cmd/local-da go run . & echo "DA_PID=$!" >> $GITHUB_ENV sleep 3
57-71: Resolve the TODO and simplify Rollkit module replacementThe
TODOcomment should be removed once a stable tag or commit is used. You can also avoid relying onjqby using Go’s templating forgo list:- # TODO(@julienrbrt) this should be replaced with a specific commit hash - go get github.com/rollkit/rollkit@main - UPGRADED_ROLLKIT=$(go list -m -json github.com/rollkit/rollkit | jq -r .Version) + # pin to latest Rollkit version after tagging + go get github.com/rollkit/rollkit@main + UPGRADED_ROLLKIT=$(go list -m -f "{{.Version}}" github.com/rollkit/rollkit) go mod edit -replace github.com/rollkit/rollkit=github.com/rollkit/rollkit@$UPGRADED_ROLLKIT
100-120: Remove trailing spacesYAMLlint reports trailing spaces around lines 103 and 110. Please remove any extra spaces at the ends of these lines to satisfy
yamllint:- ATTEMPT=$((ATTEMPT+1)) + ATTEMPT=$((ATTEMPT+1)) ... - BLOCKS_FOUND=$(grep -c "Block executed successfully" chain.log || true) + BLOCKS_FOUND=$(grep -c "Block executed successfully" chain.log || true)🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
1-8: Consider adding a concurrency groupTo prevent overlapping runs of this workflow on the same branch or ref, you can add:
concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: trueright under the workflow name.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
.github/workflows/integration_test.yml (1)
Learnt from: julienrbrt
PR: rollkit/go-execution-abci#81
File: .github/workflows/integration_test.yml:28-30
Timestamp: 2025-05-07T12:20:24.267Z
Learning: For the rollkit/go-execution-abci repository, the maintainer prefers using the latest version of Ignite CLI rather than pinning to a specific version during the current development phase. Version tags will be added later when Rollkit has a tag.
🪛 YAMLlint (1.35.1)
.github/workflows/integration_test.yml
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test / Run Unit Tests
- GitHub Check: Test with Rollkit Chain
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
.github/workflows/integration_test.yml (2)
67-70: Resolve the TODO for Rollkit version pinning
ATODOcomment remains on pinning the Rollkit dependency:# TODO(@julienrbrt) this should be replaced with a specific commit hash go get github.com/rollkit/rollkit@mainPlease either remove the
TODOor update it to reflect the intended stability guarantee (e.g., specific tag/commit).
29-31: 🛠️ Refactor suggestionFix Ignite CLI installation command and version handling
The install URL currently includes both a pinned version (@v28.10.0) and a trailing exclamation mark, which will break the curl command. Per the maintainer’s preference, it’s better to install the latest CLI.- curl -sSL https://get.ignite.com/cli@v28.10.0! | bash + curl -sSL https://get.ignite.com/cli | bash
🧹 Nitpick comments (2)
.github/workflows/integration_test.yml (2)
85-91: Improve error detection when starting the chain
Similarly, you should enableerrexit/pipefailat the top of this script block to catch failures early. For example:run: | + set -euxo pipefail cd gm # start the chain and send output to a log file gmd start --rollkit.node.aggregator --log_format=json > chain.log 2>&1 & CHAIN_PID=$! echo "CHAIN_PID=$CHAIN_PID" >> $GITHUB_ENVThis ensures that any failures in
gmd startor directory changes will stop the workflow immediately.
102-104: Remove trailing whitespace
YAMLlint flagged trailing spaces on these lines. Trailing whitespace can lead to lint errors or misparsed YAML:- sleep 2 + sleep 2 ... - BLOCKS_FOUND=$(grep -c "Block executed successfully" chain.log || true) + BLOCKS_FOUND=$(grep -c "Block executed successfully" chain.log || true)Also applies to: 110-114
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 103-103: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/integration_test.yml
[error] 103-103: trailing spaces
(trailing-spaces)
[error] 110-110: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test with Rollkit Chain
2 participants
this thing is 100% AI for now. i'll tweak it as I improve it
Summary by CodeRabbit