Repository navigation
test: pin hashlock payment count and reserved escrow keys - #808
Conversation
…variant (issue accensa#622) Appends three tests to hashlock_test.rs that assert, after every add/resolve/refund transition, that HashlockReserved equals the sum of the still-pending payments' amounts; that HashlockPaymentCount hands out distinct sequential ids that a resolve never rewinds or reuses; and that a reveal and a timed-out HTLC refund each release exactly their own amount, with double reveal/refund rejected and the reserve left at the pending sum. Test-only: hashlock.rs, lib.rs and sibling test files are untouched. Verified with a mutation check: stalling the resolve's set_reserved and making the cursor jump by two both turn the new tests red. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
…ccensa#622) The changelog gate requires an entry for any contracts/**/src/** change; the PR carries the skip-changelog option only for changes with no user-facing record, and this one pins two published storage keys. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@aojomo Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
MergeKeeper review unavailable AI provider failed: gemini-3.5-flash-lite (503) No approval or merge action was taken. |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 31 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check |  |
Issue [#622] defines a hard file boundary: changes belong in contracts/state-channel/src/hashlock_test.rs and needed test fixtures, and files outside those regions are not included. The PR adds a `C… |
Remove the CHANGELOG.md change, or obtain an explicit issue-scope exception from the maintainer before merging. Keep the implementation changes confined to the permitted test files. |
-
Check name Status Explanation Description check The description clearly explains the test-only changes, affected files, verification results, related issue, and contract-safety impact. It does not reproduce the template checklist headings or checkb… Title check The title clearly identifies the main change: tests that pin hashlock payment-count allocation and reserved escrow keys. Linked Issues check Issue [ #622] requires tests for sequential payment IDs, the pending-payment reserve invariant, exact resolve/refund releases, rejected repeat settlement, reuse ofsetup(), and a green workspace test…Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (1 skipped: 1 … Full details: Description check
-
Explanation
The description clearly explains the test-only changes, affected files, verification results, related issue, and contract-safety impact. It does not reproduce the template checklist headings or checkbox state, but it provides the required information in prose.
Full details: Linked Issues check
-
Explanation
Issue [
#622] requires tests for sequential payment IDs, the pending-payment reserve invariant, exact resolve/refund releases, rejected repeat settlement, reuse ofsetup(), and a green workspace test. The PR summary reports three named tests that cover these cases. The reviewedhashlock_test.rscontainssetup(), the reserve recomputation helper, and the sequential-ID test. The reported workspace test result is green. No dependency, contract-source, sibling-test,epoch.rs, orROADMAP.mdchange is reported.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 1 files. (1 skipped: 1 unsupported.)
-
Full details: Out of Scope Changes check
-
Explanation
Issue [
#622] defines a hard file boundary: changes belong incontracts/state-channel/src/hashlock_test.rsand needed test fixtures, and files outside those regions are not included. The PR adds aCHANGELOG.mdentry. The entry documents the test work, but it is still outside the issue's permitted regions. The supplied whole-PR diff was unavailable, so this conclusion relies on the PR summary and the reviewedCHANGELOG.mdandhashlock_test.rsblobs.Resolution
Remove the
CHANGELOG.mdchange, or obtain an explicit issue-scope exception from the maintainer before merging. Keep the implementation changes confined to the permitted test files.
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
… (issue accensa#622) Fixtures only this change needs: the three new hashlock bookkeeping cases record their golden invocation snapshots alongside the existing hashlock_test goldens. Other crates' re-recorded goldens are deliberately left alone. Generated with Codebuff 🤖 Co-Authored-By: Codebuff <noreply@codebuff.com>
|
MergeKeeper review Scope: in scope for linked issue The PR successfully implements the required test coverage for issue #622 by pinning hashlock payment counts and reserved escrow keys in contracts/state-channel/src/hashlock_test.rs, including appropriate test snapshot JSON files and a minimal changelog update. All changes adhere strictly to the defined file ownership boundaries and operational constraints. Reviewed commit: |
![mergekeeper[bot]](https://avatars.githubusercontent.com/u/173216192?s=60&v=4){kind=small}
There was a problem hiding this comment.
Approved
The PR successfully implements the required test coverage for issue #622 by pinning hashlock payment counts and reserved escrow keys in contracts/state-channel/src/hashlock_test.rs, including appropriate test snapshot JSON files and a minimal changelog update. All changes adhere strictly to the defined file ownership boundaries and operational constraints.
|
Merged Merged with |
Closes #622
Summary
Appends a reserve-bookkeeping section to
contracts/state-channel/src/hashlock_test.rs, reusing the file's existingsetup()harness. Test-only change:hashlock.rs,lib.rsand the sibling test files (htlc_test.rs,multi_asset_test.rs,watchtower_test.rs) are untouched, and no error variants or gates were relaxed.Three new cases:
count_key_allocates_distinct_sequential_ids— pinsDataKey::HashlockPaymentCount(lib.rs:126) as the allocation cursor: ids start at 1, advance by exactly one peradd_hashlock_paymentso two payments on one channel never collide, and a resolve neither rewinds the cursor nor reuses a settled slot (the next add still gets the next id).hashlock_reserved_equals_pending_sum_after_every_transition— after every add and resolve, recomputes the pending sum from theDataKey::HashlockPaymentrecords themselves and assertsDataKey::HashlockReserved(lib.rs:129) equals it exactly — so a resolve that leaves the reserve stale, in either direction, fails. A double reveal is rejected withHtlcNotPendingand the reserve is left exactly at the pending sum; a second reveal attempt can neither re-release nor keep escrow reserved.refund_and_resolve_each_release_exactly_their_own_amount— a hashlock invoice (120) and an HTLC hop (300) share one channel's escrow under separate reserve keys. Refunding the timed-out hop releases exactly its own 300 (the hashlock reserve and the pending invoice are untouched, and a second refund is rejected); revealing the invoice releases exactly its own 120 into the receiver's balance.Verification
All three CI gates were run locally on this branch and are green:
cargo fmt --all -- --checkcargo clippy --locked --all-targets -- -D warningscargo test --locked --workspace— full workspace green;state-channelsuite: 105 passed, 0 failed, naming the new caseshashlock_test::count_key_allocates_distinct_sequential_ids,hashlock_test::hashlock_reserved_equals_pending_sum_after_every_transition,hashlock_test::refund_and_resolve_each_release_exactly_their_own_amount.The test that proves
contracts/state-channel/src/lib.rs:129(HashlockReserved) is handled:hashlock_test::hashlock_reserved_equals_pending_sum_after_every_transition. It derives the expected reserve from the payment records after every transition and assertsget_hashlock_reservedmatches — andcount_key_allocates_distinct_sequential_idspinslib.rs:126's cursor the same way.Mutation check (temporary, reverted;
hashlock.rs/lib.rsare byte-identical tomainin this PR's diff):set_reserved(...)fromresolve_payment→ the pending-sum test fails (stale reserve detected);Notes
hashlock_test.rs, but CONTRIBUTING.md's changelog gate fails any PR touchingcontracts/**/src/**without aCHANGELOG.mdentry, and theskip-changelogescape-hatch label does not exist on this repository for an outside contributor to apply. A minimal entry was therefore added under## [Unreleased]— the same approach PR docs(keys): mark keys.rs namespace partition reserved and pin its folding with tests (Closes #626) #796 took for chore: resolve the storage namespace module that no contract imports #626. Happy to drop it if a maintainer prefers to apply the label instead.ROADMAP.mdis untouched: the maintainer records the entry when this PR merges, per the issue.SECURITY.mdscope isReceiptAnchor/RefundVault; this change touches neither, and adds no new on-chain surface.Summary by CodeRabbit