ci: add hotpath profiling && comment ci#867
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a feature-gated integration test that profiles a full coinswap run and emits full + maker/taker filtered JSON reports. Adds two GitHub Actions: ChangesHotpath profiling benchmark infrastructure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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 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 |
There was a problem hiding this comment.
Pull request overview
Adds CI automation to run Hotpath profiling on a representative integration-test swap and post base-vs-head performance comparisons back to the pull request.
Changes:
- Adds a
hotpath_profile_swapintegration test that runs a swap under Hotpath and writes filtered maker/taker JSON reports. - Exposes a test-only JSON report reader in
hotpath_localto support the integration benchmark harness. - Introduces two GitHub Actions workflows: one to generate/upload metrics, and one to download metrics and comment on the PR.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/integration/main.rs |
Registers the hotpath profiling test module behind the hotpath feature. |
tests/integration/hotpath_profile.rs |
New integration benchmark that runs a swap under Hotpath and emits filtered maker/taker metrics JSON. |
src/hotpath_local.rs |
Adds a test-only public wrapper to read Hotpath JSON reports with retries. |
.github/workflows/hotpath-profile.yml |
New workflow to run the benchmark on head and base and upload metrics artifact. |
.github/workflows/hotpath-comment.yml |
New workflow_run job that downloads metrics and posts comparison comments to the PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/hotpath-profile.yml:
- Around line 3-5: The workflow currently triggers on the broad "on:
pull_request" event; narrow it to avoid running on every commit by replacing or
augmenting that trigger with specific "types" (e.g., types: [opened, reopened,
synchronize]) and/or add "paths" or "paths-ignore" filters so the job only runs
for relevant files (e.g., profiling config or benchmarks); update the "on:
pull_request" block to include the chosen types and/or paths filters to limit
executions.
- Around line 43-59: The base benchmark run ("Base benchmark" step that runs the
cargo test --release --test integration hotpath_profile_swap ...) can reuse
incremental build artifacts from the head run; add cache invalidation by either
running cargo clean right before the Base benchmark step or change the
rust-cache usage to use a distinct shared-key for the base run so it restores an
independent target directory; update the workflow to insert the clean or unique
shared-key in the "Base benchmark" step immediately before executing the cargo
test command.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 438f61ac-18fb-4beb-a33f-c308f7d2abed
📒 Files selected for processing (5)
.github/workflows/hotpath-comment.yml.github/workflows/hotpath-profile.ymlsrc/hotpath_local.rstests/integration/hotpath_profile.rstests/integration/main.rs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #867 +/- ##
==========================================
+ Coverage 68.87% 78.59% +9.71%
==========================================
Files 35 57 +22
Lines 4932 15762 +10830
==========================================
+ Hits 3397 12388 +8991
- Misses 1535 3374 +1839 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/hotpath-profile.yml:
- Around line 44-63: The Base benchmark step currently assumes the test target
hotpath_profile_swap exists after checking out the base commit; add a pre-check
that verifies the test exists in the base commit and skip the benchmark if it
does not. Concretely, before the "Base benchmark" run block (after the "Checkout
base" step) run a lightweight command to list test targets and confirm
hotpath_profile_swap is present (or check for the integration test name in cargo
test --list for the integration test suite), and only execute the cargo test
--release --test integration hotpath_profile_swap ... when that check succeeds;
otherwise emit a clear log message and mark the benchmark as skipped so the
workflow does not fail. Ensure the workflow step names ("Checkout base", "Base
benchmark") and the test target name (hotpath_profile_swap) are used so the
change is easy to locate and validate.
In `@tests/integration/hotpath_profile.rs`:
- Around line 117-131: The test currently starts the coinswap
(taker.start_coinswap) and immediately signals makers to shutdown and joins
maker_threads without verifying swap completion; add a verification step that
waits for the swap referenced by summary.swap_id to reach a completed state (or
fail/timeout) before shutting down makers. Implement this by calling an existing
wait/status helper (e.g., taker.wait_for_swap_completion(&summary.swap_id) or
polling taker.get_swap_status()/is_swap_complete(&summary.swap_id) with a
reasonable timeout and assert the swap completed successfully), and only after
that set maker.shutdown.store(true, Relaxed) and join maker_threads; if no
helper exists, add a small polling loop in the test to check the swap state and
fail the test on timeout.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: bd891a49-6798-4e11-908e-2c81260832c8
📒 Files selected for processing (5)
.github/workflows/hotpath-comment.yml.github/workflows/hotpath-profile.ymlsrc/hotpath_local.rstests/integration/hotpath_profile.rstests/integration/main.rs
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
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.
Pull Request
Description
Adds hotpath to ci for profiling, by introducing a
hotpath-profileworkflow that runs a single integration testhotpath_profileon both PR head and base commits, generates maker and taker JSON metrics, and uploads them as a profile-metrics artifact, plus ahotpath-commentworkflow that triggers on the profile workflow completion, downloads that artifact, and posts maker/taker base-vs-head comparison comments to the PR.Type of Change
Protocol Version(s) Affected
messages.rs,contract.rs,handlers.rs)messages2.rs,contract2.rs,handlers2.rs)Affected Component(s)
docs/)Checklist
Code Quality
cargo +nightly fmt --alland committed the resultcargo +stable clippy --all-features --lib --bins --tests -- -D warningswith zero warningscargo +stable clippy --examples -- -D warningswith zero warningsRUSTDOCFLAGS="-D warnings" cargo +nightly doc --all-features --document-private-items --no-depswith zero warningsln -s ../../git_hooks/pre-commit .git/hooks/pre-commitif not already set)Testing
cargo test)cargo test --features integration-test)#[cfg(feature = "integration-test")]Documentation
docs/folder were updatedSecurity & Privacy (Critical)
integration-testfeature flag is not reachable in production code pathsSummary by CodeRabbit
New Features
Tests