H-L03: fix(contracts/ChannelHub): skip disputed escrow during purge

[nksazonov]

Description

The _purgeEscrowDeposits(...) function breaks immediately when it encounters a DISPUTED escrow (L408), preventing all subsequent escrows from being processed. Unlike FINALIZE escrows which are skipped with continue (L392), DISPUTED escrows cause the loop to exit entirely. This leaves the escrowHead pointer stuck before the DISPUTED escrow, blocking all subsequent escrows from being purged. The purge mechanism doesn't check challengeExpireAt or automatically handle expired disputes, requiring manual intervention via finalizeEscrowDeposit to unblock the queue.

Impact

Node liquidity is locked in all escrows queued after a DISPUTED escrow until manual intervention occurs. The automated purge mechanism fails completely when DISPUTED escrows exist, even after their challenge period expires. This requires off-chain monitoring to detect and resolve, breaking the protocol's automated design. The minimum blockage period is 1 day (CHALLENGE_DURATION), but could extend indefinitely if not actively monitored. While the node can self-resolve by calling finalizeEscrowDeposit, this represents an operational failure of the intended automated system.

Summary by CodeRabbit

Release Notes

• New Features

  • Made escrow deposit statistics function publicly accessible, enabling queries for unlockable deposit counts and aggregated amounts across the deposit queue.

• Bug Fixes

  • Enhanced the escrow deposit purge mechanism to correctly identify and skip both finalized and disputed deposits during queue iteration, preventing them from unnecessarily blocking subsequent purge operations.

• Tests

  • Added comprehensive test coverage for escrow deposit statistics queries and purge operations across various queue configurations and edge cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8cd42f3a-b8bf-4407-95bc-2d2cda7137af

📥 Commits

Reviewing files that changed from the base of the PR and between 1ba20be and c77bf74.

📒 Files selected for processing (5)

• contracts/src/ChannelHub.sol
• contracts/test/ChannelHub_escrowDepositPurge/ChannelHub_EscrowDepositPurge_Base.t.sol
• contracts/test/ChannelHub_escrowDepositPurge/ChannelHub_getUnlockableEscrowDepositStats.t.sol
• contracts/test/ChannelHub_escrowDepositPurge/ChannelHub_purgeEscrowDeposits.t.sol
• contracts/test/TestChannelHub.sol

---

📝 Walkthrough

Walkthrough

Modified ChannelHub's escrow deposit purge and stats logic to make getUnlockableEscrowDepositStats() public and introduce a _isEscrowDepositSkippable() predicate that treats both FINALIZED and DISPUTED escrows as skippable during queue iteration, alongside comprehensive test infrastructure and coverage.

Changes

Cohort / File(s)	Summary
Core Logic Refactor contracts/src/ChannelHub.sol	Made getUnlockableEscrowDepositStats() public; added _isEscrowDepositSkippable() predicate to skip both FINALIZED and DISPUTED escrows during stats computation and purge operations; updated loop logic in _purgeEscrowDeposits() to use the new predicate.
Test Harness contracts/test/TestChannelHub.sol	Added external test-harness functions: workaround_setEscrowDeposit(), workaround_addEscrowDepositId(), harness_escrowDepositIds(), and harness_purgeEscrowDeposits() to enable direct manipulation and inspection of escrow state in tests.
Test Base Contract contracts/test/ChannelHub_escrowDepositPurge/ChannelHub_EscrowDepositPurge_Base.t.sol	Created abstract base test contract with TestChannelHub and MockERC20 initialization, deterministic address setup, and internal helper methods to create escrows in specific states (INITIALIZED unlockable/not-yet-unlockable, FINALIZED, DISPUTED).
Stats Tests contracts/test/ChannelHub_escrowDepositPurge/ChannelHub_getUnlockableEscrowDepositStats.t.sol	Comprehensive test suite for getUnlockableEscrowDepositStats() verifying empty queue returns zero, single/multiple entries are correctly counted and aggregated, FINALIZED/DISPUTED entries are skipped, iteration stops at non-unlockable entries, and stats computation advances from updated escrow head.
Purge Tests contracts/test/ChannelHub_escrowDepositPurge/ChannelHub_purgeEscrowDeposits.t.sol	Extensive test coverage for purgeEscrowDeposits() behavior: empty queue handling, single-entry purge/skip scenarios, mixed queue skipping of FINALIZED/DISPUTED with subsequent purging of unlockable entries, maxToPurge limiting, multi-node/token accounting correctness, and proper event emissions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• YNU-797: feat(contracts/ChannelHub): add NodeBalanceUpdated event #621: Modifies escrow finalize/purge flows and adds NodeBalanceUpdated event emissions during purge operations, directly intersecting with this PR's purge logic changes.
• YNU-729: feat(ChannelHub): add cross-chain deposit, withdrawal, migration #498: Adds cross-chain escrow functionality and escrow-related getters/state flows, overlapping with escrow status handling and EscrowStatus enum usage.

Suggested reviewers

• ihsraham
• philanton
• dimast-x

Poem

🐰 Disputed, finalized—both skip the queue today,
Stats count only unlockable, no delay,
Public now the stats that once hid away,
With tests galore to show the proper way! 🎉

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'skip disputed escrow during purge' accurately reflects the main change: fixing the purge function to skip disputed escrow deposits instead of breaking the loop, allowing subsequent escrows to be processed.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/audit-h-l03

---

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]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.