test: ibc integration test#138
Conversation
WalkthroughThe GitHub Actions workflow for integration testing was renamed and restructured into two jobs: Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Liveness Job
participant Rollkit Chain
participant Carol
participant Bob
participant IBC Job
participant Celestia Chain
participant Hermes Relayer
GitHub Actions->>Liveness Job: Start liveness job
Liveness Job->>Rollkit Chain: Scaffold and start chain
Liveness Job->>Carol: Create account, capture mnemonic
Liveness Job->>Bob: Use existing account
Bob->>Carol: Send tokens (transaction test)
Liveness Job->>GitHub Actions: Upload artifacts (gm binary, gmd home, Carol's mnemonic)
GitHub Actions->>IBC Job: Start ibc job (after liveness)
IBC Job->>GitHub Actions: Download artifacts
IBC Job->>Celestia Chain: Start chain, recover Carol's account
IBC Job->>Hermes Relayer: Configure and start relayer
IBC Job->>Rollkit Chain: Start chain
IBC Job->>Hermes Relayer: Create IBC connection & channel
Rollkit Chain->>Celestia Chain: Send ICS20 tokens via IBC
Celestia Chain->>Rollkit Chain: Send ICS20 tokens back
IBC Job->>GitHub Actions: Output test results and logs
Possibly related issues
Possibly related PRs
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 (
|
julienrbrt
force-pushed
the
julien/test-ibc
branch
from
83e035a to
242a303
Compare
julienrbrt
force-pushed
the
julien/test-ibc
branch
from
24cd720 to
f0de142
Compare
![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 (2)
.github/workflows/integration_test.yml (2)
70-70: Address the remaining TODO before merging
The placeholder for pinning the Rollkit dependency (# TODO(@julienrbrt) this should be replaced with a specific commit hash) needs resolution to ensure build reproducibility.Would you like me to help replace
mainwith a stable commit hash here?
273-278: Use the capturedGMD_HOMEenv var when starting Rollkit
You’ve setGMD_HOMEfrom thelivenessjob’s output but then hardcode--home ./gmd/.gm. Consider replacing this with--home $GMD_HOMEto avoid path assumptions.- ./gmd/go/bin/gmd start ... --home ./gmd/.gm > chain.log 2>&1 & + ./gmd/go/bin/gmd start ... --home "$GMD_HOME" > chain.log 2>&1 &
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test / Run Unit Tests
- GitHub Check: Test with Rollkit Chain
🔇 Additional comments (2)
.github/workflows/integration_test.yml (2)
17-20: Correct mapping of step outputs to job outputs
Thelivenessjob properly exposescarol_mnemonicandgmd_homefor downstream jobs vianeeds.liveness.outputs.
107-116: Verifyactions/upload-artifactparameters
Theinclude-hidden-filesoption is not officially supported byactions/upload-artifact@v4. Please confirm this flag’s validity or remove it to avoid silent failures when uploading thegmdbinary and home directory.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
.github/workflows/integration_test.yml (3)
242-247: Add checksum verification for Hermes binary
Downloading the Hermes relayer without verifying its checksum is vulnerable to tampering.- wget https://github.com/informalsystems/hermes/releases/download/${HERMES_VERSION}/hermes-${HERMES_VERSION}-x86_64-unknown-linux-gnu.tar.gz + wget https://github.com/informalsystems/hermes/releases/download/${HERMES_VERSION}/hermes-${HERMES_VERSION}-x86_64-unknown-linux-gnu.tar.gz + wget https://github.com/informalsystems/hermes/releases/download/${HERMES_VERSION}/checksums.txt + grep "hermes-${HERMES_VERSION}-x86_64-unknown-linux-gnu.tar.gz" checksums.txt | sha256sum -c - tar -xzf hermes-${HERMES_VERSION}-x86_64-unknown-linux-gnu.tar.gz sudo mv hermes /usr/local/bin/hermes
343-351: Assert on IBC transfer to Celestia
Currently, the step only prints the post-transfer balance. To enforce test failure on mismatch, add pre-balance capture and a comparison exit check.Example diff:
- BALANCE=$(celestia-appd query bank balances $CELESTIA_ADDR --output json --node http://localhost:26654 | jq '.balances') - echo "Celestia balance after IBC transfer: $BALANCE" + PRE_BALANCE=$(celestia-appd query bank balances $CELESTIA_ADDR --output json --node http://localhost:26654 | jq -r '.balances[0].amount') + hermes tx amount=100stake ... + BALANCE=$(celestia-appd query bank balances $CELESTIA_ADDR --output json --node http://localhost:26654 | jq -r '.balances[0].amount') + if [ "$BALANCE" -ne "$((PRE_BALANCE + 100))" ]; then + echo "IBC transfer failed: expected $((PRE_BALANCE + 100)), got $BALANCE" + exit 1 + fi
236-241: Add checksum verification for Celestia binary
Fetching the Celestia App without integrity checks poses a supply-chain risk.Apply this diff:
- wget https://github.com/celestiaorg/celestia-app/releases/download/${CEL_VERSION}/celestia-app_Linux_x86_64.tar.gz + wget https://github.com/celestiaorg/celestia-app/releases/download/${CEL_VERSION}/celestia-app_Linux_x86_64.tar.gz + wget https://github.com/celestiaorg/celestia-app/releases/download/${CEL_VERSION}/checksums.txt + grep "celestia-app_Linux_x86_64.tar.gz" checksums.txt | sha256sum -c - tar -xzf celestia-app_Linux_x86_64.tar.gz sudo mv celestia-appd /usr/local/bin/celestia-appd
🧹 Nitpick comments (2)
.github/workflows/integration_test.yml (2)
107-115: Archive chain binary and home directory
Uploading both thegmdbinary and its home directory is good. For clarity, consider naming the artifact explicitly (e.g.,name: gmd) so that downstream downloads target it directly.
233-235: Download chain artifacts explicitly
actions/download-artifactwithout aname:will fetch all artifacts. To avoid surprises if more artifacts are added, explicitly setwith: name: gmd.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test / Run Unit Tests
- GitHub Check: Test with Rollkit Chain
🔇 Additional comments (13)
.github/workflows/integration_test.yml (13)
1-1: Update workflow name to reflect IBC tests
The workflow name now captures both liveness and IBC integration tests.
17-19: Expose outputs for cross-job usage
Capturingcarol_mnemonicandgmd_homeas job outputs is required for seeding the IBC job.
51-51: Ensure correct address prefix
The--address-prefix gmflag aligns account address formats with test expectations—please verify all downstream steps use thegmprefix.
87-93: Persist Carol’s mnemonic for IBC tests
Generating and exporting Carol’s mnemonic via bothGITHUB_ENVandGITHUB_OUTPUTis essential for hermes key import.
95-106: Capture chain binary and config home
RecordingGM_BINARY_PATHandGMD_HOMEfor artifact exchange looks correct and will facilitate reuse in theibcjob.
174-176: Switch transaction target to Carol
The send command now routes funds from Bob to Carol, matching the updated test scenario. Implementation is correct.
215-219: Introduce IBC test job
Adding theibcjob with a dependency onlivenesscleanly separates chain liveness checks from IBC connectivity tests.
221-226: Pin relayer and Celestia App versions
HardcodingHERMES_VERSIONandCEL_VERSIONis necessary for reproducible tests. Please confirm these versions exist and have no known critical vulnerabilities.
248-253: Initialize Celestia app with recovered validator key
Using--recoverand--keyring-backend testis correct for non-interactive key import. Confirm the genesis account and chain ID align with what your tests expect.
264-270: Start local DA in IBC job
Re-cloning and starting the DA node again ensures the DA layer is running for the IBC tests.
274-281: Verify artifact extraction path
The invocation./gmd/go/bin/gmdassumes thegmdbinary and home directory extracted under./gmd. Please verify the upload/download layout matches this path.
339-342: Establish IBC channel
Creating an unordered transfer channel betweengmandcelestia-localis correct.
371-385: Cleanup all background processes
The final cleanup step kills all PIDs, preventing resource leaks across jobs.
alpe
left a comment
alpe
left a comment
There was a problem hiding this comment.
Nice setup. I can not run this locally. How about moving this to some bash scripts instead that are called by CI? I had a similar approach in my new branch
You can actually, by just calling: |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
.github/workflows/integration_test.yml (2)
236-240: Add checksum verification for Celestia App binary
Downloadingcelestia-appdwithout verifying its checksum or signature risks supply-chain attacks. Introduce a checksum download andsha256sum -cstep before extraction.
243-246: Add checksum verification for Hermes binary
Similarly, verify the integrity of thehermesrelease by downloading the official checksum file or signature and validating withsha256sum -cor GPG.
🧹 Nitpick comments (2)
.github/workflows/integration_test.yml (2)
107-115: Optimize artifact upload size
Uploading the entireGMD_HOMEdirectory directly can produce oversized artifacts. Consider bundling the binary and home directory into a compressed archive before upload, for example:- uses: actions/upload-artifact@v4 + run: | + tar czf gmd-artifact.tar.gz ${{ steps.paths.outputs.GM_BINARY_PATH }} ${{ steps.paths.outputs.GMD_HOME }} + uses: actions/upload-artifact@v4 with: name: gmd - path: | - ${{ steps.paths.outputs.GM_BINARY_PATH }} - ${{ steps.paths.outputs.GMD_HOME }} + path: gmd-artifact.tar.gz
380-392: Graceful process cleanup
Whilekill -9force-terminates processes, consider first sending a gentler signal (e.g.,kill $PID) to allow clean shutdown, then fall back to-9if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/integration_test.yml(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: test / Run Unit Tests
- GitHub Check: Test with Rollkit Chain
🔇 Additional comments (6)
.github/workflows/integration_test.yml (6)
1-1: Trivial Workflow Rename
This update renames the workflow to accurately reflect the expanded scope. No further changes needed here.
17-17: Disable telemetry during CI runs
SettingDO_NOT_TRACK=trueis a good practice to prevent unwanted telemetry in automated environments.
18-20: Expose outputs for dependent jobs
Capturing Carol’s mnemonic and theGMD_HOMEdirectory as job outputs is correct and enables theibcjob to consume these values.
51-51: Chain scaffolding command
Theignite scaffold chain gm --no-module --skip-git --address-prefix gminvocation is appropriate for creating an isolated test chain.
95-106: Capture GMD binary path and home directory
Correctly resolving and exportingGM_BINARY_PATHandGMD_HOMEfor artifact uploads and downstream consumption. No issues detected.
369-376: Enhanced failure logging
Collectingchain.log,celestia.log, andhermes.logon failure significantly improves debugging capabilities.
* test: ibc integration test * enable tx indexing on celestia * fund account * fix address prefix * bech32 * add correct node * updates * updates * do not discard finalise block response
3 participants
Overview
ref: #133, #42
Summary by CodeRabbit