Repository navigation
test(cross-chain): pin the LayerZero delivery-guard keys - #815
Conversation
Issue accensa#619 asks for the trusted-peer, nonce and settled-dispute keys to be covered in layerzero_test.rs. The four behaviours it lists were already pinned by ddd24bb, so this adds the parts of the issue's own "What done looks like" that were genuinely missing, all read back through the getters the crate already exposes. - `is_trusted_peer` had no coverage at all; the peer set is now asserted to be keyed by the whole `(chain_id, sender)` pair, not by either half. - `get_peer_nonce` is asserted absent before any delivery, then taken from the accepted packet, and every nonce at or below the mark stays refused after a forward jump — pinning `nonce <= last` rather than `nonce < last`. - A settled dispute now has its accepted delivery asserted to publish exactly one event, with the channel still advancing for a dispute that has not been settled. A refused invocation is rolled back by the host, so the module only claims what can actually fail: the accepted-path assertions. That was checked by fault injection — writing PeerNonce before the settled-dispute check, and publishing a duplicate event on the refusal path, both leave refused-path assertions passing, while a duplicate publish on the accepted path is caught. Verified: cargo fmt --all -- --check, cargo clippy --locked --all-targets -- -D warnings and cargo test --locked --workspace are all clean, and the cross-chain suite goes from 36 to 40 tests. Closes accensa#619 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
|
@olatecki 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 Scope: in scope for linked issue The PR successfully adds comprehensive test coverage for LayerZero trusted peer, nonce, and settled dispute keys in contracts/cross-chain/src/layerzero_test.rs along with required snapshots and a changelog entry, fulfilling issue #619 correctly without modifying read-only contract source files. Reviewed commit: |
|
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 |  |
The PR adds an [Unreleased] entry to CHANGELOG.md. Issue #619 limits the implementation to contracts/cross-chain/src/layerzero_test.rs and test or fixture files needed by that change. It also st… |
Remove the CHANGELOG.md change, or obtain an explicit scope exception that permits this changelog entry. |
-
Check name Status Explanation Title check The title is concise, specific, and accurately describes the added LayerZero delivery-guard tests. Description check The description provides a detailed summary, related issue, contract-safety information, changelog rationale, file scope, and verification results. It does not use every template heading or checkbox, … Linked Issues check Issue #619is addressed inlayerzero_test.rs. The existing tests cover unregistered peers, first nonce storage, stale or replayed nonces, higher nonce advancement, and duplicate settlement refusal. …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. (5 skipped: 5 … Full details: Description check
-
Explanation
The description provides a detailed summary, related issue, contract-safety information, changelog rationale, file scope, and verification results. It does not use every template heading or checkbox, but it covers the required information.
Full details: Linked Issues check
-
Explanation
Issue
#619is addressed inlayerzero_test.rs. The existing tests cover unregistered peers, first nonce storage, stale or replayed nonces, higher nonce advancement, and duplicate settlement refusal. The appendeddelivery_guard_keysmodule adds exact(chain_id, sender)trust coverage, verifies the nonce high-water mark after a forward jump and rejects every nonce at or below it, and verifies one event for an accepted settlement plus refusal of a repeated settlement. The PR reports the required format, clippy, and workspace test checks as passing. The scope restrictions on entry points, errors, setup helpers,layerzero.rs, andROADMAP.mdare respected.
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. (5 skipped: 5 unsupported.)
-
Full details: Out of Scope Changes check
-
Explanation
The PR adds an
[Unreleased]entry toCHANGELOG.md. Issue#619limits the implementation tocontracts/cross-chain/src/layerzero_test.rsand test or fixture files needed by that change. It also states that files outside those regions are not in scope. The changelog entry is related documentation, but it is still outside the explicit file boundary. The repository diff was unavailable, so this finding relies on the supplied change summary and the reviewedCHANGELOG.mdblob.Resolution
Remove the
CHANGELOG.mdchange, or obtain an explicit scope exception that permits this changelog entry.
- Fix all pre-merge checks with AI
✨ 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.
![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 adds comprehensive test coverage for LayerZero trusted peer, nonce, and settled dispute keys in contracts/cross-chain/src/layerzero_test.rs along with required snapshots and a changelog entry, fulfilling issue #619 correctly without modifying read-only contract source files.
|
Merged Merged with |
Closes #619
What changed
contracts/cross-chain/src/layerzero_test.rsdelivery_guard_keysmodule appended — 4 testscontracts/cross-chain/test_snapshots/layerzero_test/delivery_guard_keys/*.jsoncontracts/cross-chain/test_snapshots/CHANGELOG.md[Unreleased]entrylayerzero.rsis unmodified (read-only per the issue), no entry point was added,no
Errorvariant changed, the existing setup helpers and the mock endpoint areuntouched, and
ROADMAP.mdis not edited.On
CHANGELOG.md: thechangelog & version checkjob fails any PR touchingcontracts/*/src/without aCHANGELOG.mdchange unless it carries theskip-changeloglabel, which I can't apply. PR #808 — also test-only — added anentry the same way. Happy to drop that file if a maintainer prefers to label it.
The four behaviours #619 lists were already pinned
Worth saying up front, because it changes what this PR contains.
layerzero_test.rsdoes not nameTrustedPeer,PeerNonceorDisputeSettled— but only because the existing tests read state back through the getters,
exactly as the issue asks them to. The behaviours themselves landed with
ddd24bbon 2026-09-28, twelve days before the issue was filed:(chain_id, sender)rejecteduntrusted_peer_is_rejected,wrong_chain_is_rejected_even_for_a_trusted_peerPeerNonceauthorized_endpoint_delivers_dispute_resolution(get_peer_nonce == 1)replayed_and_stale_nonces_are_rejected(nonce5again, then4)later_nonces_are_accepted_and_advance_the_channel(1 → 7)DisputeSettledsettled_dispute_cannot_be_settled_again(Error::AlreadyRefunded)So this does not restate them in a second harness. It closes what the issue's own
"What done looks like" left open:
layerzero.rs:116—TrustedPeer.is_trusted_peerhad no coverageanywhere in the suite.
only_the_registered_pair_is_trustedasserts the peerset is keyed by the whole
(chain_id, sender)pair: the registered pair istrusted, and neither the same sender on another chain nor another sender on
the same chain is.
layerzero.rs:118—PeerNonce.the_mark_holds_the_highest_accepted_nonceasserts the mark reads
0before any delivery, takes the accepted packet'snonce, accepts a strictly higher nonce that skips packets never delivered
(
2 → 9), and then refuses every nonce at or below it,3..=9. That loop iswhat pins
nonce <= lastrather thannonce < last, including the gap thejump skipped.
layerzero.rs:120—DisputeSettled.a_settled_dispute_publishes_one_event_and_stays_settledasserts the accepted delivery publishes exactly one event, that a second packet
naming that dispute is refused with
AlreadyRefunded, and that the channelstill advances for a dispute that has not been settled.
a_packet_from_an_unregistered_channel_is_refusedcovers the wrong-chainrefusal end to end: a packet naming a chain the peer was never registered on is
Unauthorized, and the trusted channel still takes its own first packet after.No getter is missing —
is_trusted_peer,get_peer_nonceandis_dispute_settledcover all three keys, so none was added.
TrustedPeeris readable only as aboolean, which is all the assertions above need.
What these tests cannot prove
One criterion in the issue — a second settle "emits no duplicate event" — is not
observable, and the module says so instead of carrying an assertion that cannot
fail. Soroban rolls back a refused invocation, storage and events, so a refusal
can never leave either behind. Verified by fault injection rather than assumed:
PeerNoncebefore the settled-dispute check → the refusal assertions still pass (the write rolls back);"Settles exactly once" is therefore pinned by the accepted-path event count plus
the settled-dispute ledger, not by the refusal.
Verification
cargo fmt --all -- --check— cleancargo clippy --locked --all-targets -- -D warnings— cleancargo test --locked --workspace— PASS: 1124 tests across all binaries, 0 failurescargo test --locked -p accensa-cross-chain— 40 passed (36 before, 4 new)layerzero.rs:118is handled:layerzero_test::delivery_guard_keys::the_mark_holds_the_highest_accepted_nonceSummary by CodeRabbit