MF2-M02: accept pre-finalize escrow version in ongoing check

[philanton]

Summary

• EnsureNoOngoingEscrowOperation now status-gates the one-behind branch for TransitionTypeEscrowDeposit. The signed N+1 finalize state sits off-chain while the on-chain channel stays at INITIATE version N until the purge queue fires; that is the protocol-intended steady state, so we cannot require Closed. The branch now allows status ∈ {Open, Closed} and blocks otherwise.
• Fixes the stall reported in MF2-M02: HandleEscrowDepositsPurged marks the escrow channel Closed while preserving StateVersion at the initiate version, which previously caused EnsureNoOngoingEscrowOperation to report escrow deposit finalization is still ongoing and block transfer receives and app-session releases.
• Addresses #discussion_r3264742604: Challenged one-behind is now blocked. Co-signed N+1 is value-binding between parties, but on-chain resolution is still racing — finalize tx may not land, the escrow chain may settle at INITIATE, and replaying N+1 later could violate engine invariants. Stacking receiver-side state on N+1 in that window risks encumbering a credit that may rewind.
• Comparison uses *v + 1 == StateVersion to avoid uint underflow when version is 0.
• No change to HandleEscrowDepositsPurged or StateVersion handling. Other transitions (escrow_lock, mutual_lock, escrow_withdraw) untouched.

Test plan

• go vet ./nitronode/store/database/...
• go test ./nitronode/store/database/... -count=1
• TestDBStore_EnsureNoOngoingEscrowOperation cases:
  • chain caught up → allow
  • Open one-behind (pre-purge happy path) → allow
  • Closed one-behind (post-purge) → allow
  • Challenged one-behind → block
  • chain more than one version behind → block
  • escrow channel missing from DB → block

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refines the escrow-deposit finalization gate in EnsureNoOngoingEscrowOperation to accept on-chain state exactly one version behind the signed state (in addition to exact equality), clarifies this behavior in documentation, and expands test coverage to validate the "one behind" allow case, "more than one behind" block case, and purge-close scenario.

Changes

Escrow Deposit Validation Refinement

Layer / File(s)	Summary
Escrow Deposit Version Rule and Tests nitronode/store/database/db_store.go, nitronode/store/database/db_store_test.go	Updated the escrow_deposit settlement rule documentation to specify exact-match and one-behind cases are both settled; modified the validation logic to check *EscrowChannelVersion + 1 == StateVersion with nil guards; and added test cases covering the one-behind allow scenario, multi-version-behind block scenario, and purge-close closed-channel scenario.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• layer-3/nitrolite#735: Introduces and uses EnsureNoOngoingEscrowOperation method that this PR refines to unblock receiver operations.

Suggested reviewers

• ihsraham
• nksazonov
• dimast-x

Poem

🐇 A version behind is not too far,
When purge queues waltz where channel states are,
The deposit settles in one neat hop,
No waiting for exact—the chain can stop! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: modifying EnsureNoOngoingEscrowOperation to accept an earlier escrow version (pre-finalize) in the ongoing check, which is the primary purpose of this PR.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/nitronode-mf2-m02

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[nksazonov]

Good job! The EnsureNoOngoingEscrowOperation fix correctly closes the blocking path described in the audit finding, and the new test cases cover both the purge scenario and the in-flight finalize case.

Out-of-diff note — secondary finding from the audit (not in this diff):

The audit finding also noted: "state_version returned by GetEscrowChannel() and GetChannels() will not be up to date with the finalize state while the escrow channel is closed." HandleEscrowDepositsPurged (event_handlers/service.go) still only sets channel.Status = ChannelStatusClosed without advancing StateVersion, and the doc comment confirms this is intentional. However, since API consumers querying a purged escrow channel via GetEscrowChannel or GetChannels will see state_version = N (initiate version) rather than the finalize version N+1, it would be worth adding a note to the handler's doc comment or the GetEscrowChannel response docs explaining that state_version on a purged escrow reflects the initiate state, not the finalize state. That way callers aren't silently surprised by the discrepancy.

[philanton]

Yeah, fair point. Added a doc note on the nitronode GetEscrowChannel handler, the Go and TS SDK client methods, and the get_escrow_channel entry in docs/api.yaml calling out that state_version stays at the initiate version (N) on a purge-closed escrow. In 40caa39.

[ihsraham]

Looks close, but I’d hold approval on one boundary in the escrow gate. The one-version-behind case needs to be tied to a terminal escrow channel.

[ihsraham]

[P1/blocking] This accepts the one-version-behind escrow_deposit case without checking the escrow channel is actually terminal. An active Open or Challenged escrow at version N with a signed finalize state at N+1 has the same shape as the purged case, so receiver-side issuance can proceed while the escrow finalization is still ongoing and then hide that escrow state behind a newer receiver state.

Could we include ec.status in this query and only allow the N+1 vs N case when the escrow channel is Closed? Exact version match can stay allowed, and tests should keep Open/Challenged one-behind cases blocking while Closed one-behind allows.

[philanton]

Agree the shape-collision is real for one status, disagree on the fix.

Closed-only would block receiver-side issuance for ~3h after every escrow deposit, since the happy path is Open at N + signed N+1 sitting off-chain until the on-chain purge queue fires. Per the protocol, finalize is not enforced on-chain in the happy path — the co-signed N+1 is terminal off-chain, and HandleEscrowDepositsPurged deliberately preserves state_version at N.

The risky case is Challenged: on-chain dispute is live, node is racing to submit N+1 finalize on the escrow chain, and several failure modes (tx doesn't land in time, reorg, post-expiry replay against an INITIATE-settled escrow chain) can rewind real funds even though both parties co-signed N+1. Receiver-side state stacked on that baseline encumbers a credit that may not hold.

Pushed f7b4a59 — ec.status is now in the query and the one-behind branch allows {Open, Closed} and blocks Challenged (and any other status). Test coverage split accordingly.

[ihsraham]

Thanks, this addresses the escrow status boundary and the added coverage pins the intended Open/Closed vs Challenged behavior.