Skip to content

M-L01: return correct errors during state advancement validation#660

Merged
philanton merged 1 commit into
fix/audit-findingsfrom
fix/clearnode-l-01
Apr 9, 2026
Merged

M-L01: return correct errors during state advancement validation#660
philanton merged 1 commit into
fix/audit-findingsfrom
fix/clearnode-l-01

Conversation

@philanton
Copy link
Copy Markdown
Contributor

Description

The Ledger.Equal() method in pkg/core/types.go formats error messages with l1 labeled as "expected" and l2 labeled as "proposed":

func (l1 Ledger) Equal(l2 Ledger) error {
    if l1.TokenAddress != l2.TokenAddress {
        return fmt.Errorf("token address mismatch: expected=%s, proposed=%s", l1.TokenAddress, l2.TokenAddress)
    }
    // ... same pattern for other fields
}

However, both call sites in pkg/core/state_advancer.go pass the proposed state as the receiver and the expected state as the argument:

proposedState.HomeLedger.Equal(expectedState.HomeLedger)   
proposedState.EscrowLedger.Equal(*expectedState.EscrowLedger)

This causes error messages to show the expected and proposed values swapped, making debugging harder.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 8, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3d33c778-d598-4f07-aeb5-bce322d88b4c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/clearnode-l-01

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

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

@philanton philanton merged commit a2a2263 into fix/audit-findings Apr 9, 2026
3 checks passed
@philanton philanton deleted the fix/clearnode-l-01 branch April 9, 2026 11:38
nksazonov added a commit that referenced this pull request Apr 16, 2026
H-H01: fix(contracts): disallow challenge with CLOSE or FIN_MIG intents (#631)

H-M01: fix(contracts/ChannelHub): remove transferred amount check (#636)

fix(contracts/ChannelHub): allow unblocking escrow ops after migration (#637)

M-C01: feat(contracts): add token check between states (#639)

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

M-H04: feat(contracts/ChannelHub): purge during escrow challenge finalization, count both skip and purge (#641)

M-M02: fix(contracts): require zero allocs on close (#643)

M-C02: fix(rpc/core): apply finalize escrow deposit correctly (#644)

H-L02: fix(contracts/ChannelHub): revert on withdrawFromVault failure (#651)

M-H03: fix(clearnode): log correct fields on failure (#647)

M-C03: fix(pkg/core): disallow negative amount transitions (#645)

H-L01: fix(contracts/ChannelHub): add CH address to prevent val addition replay (#650)

M-H06(clearnode): restrict max channel challenge duration (#654)

M-M03(clearnode): revert EscrowLock on insufficient home user balance (#655)

M-M04(clearnode): support default signer even if it is not approved (#656)

M-M05(clearnode): require correct intent on FinalizeEscrowWithdrawal (#657)

M-M09: reject asset decimals exceeding its token's decimals (#659)

M-L01: return correct errors during state advancement validation (#660)

M-L03: limit number of related ids per session key state (#662)

M-I02: normalize input hex addresses (#663)

M-H01: feat(contracts/ChannelHub): restrict to one node (#649)

M-H07: fix(contracts/ChannelHub): emit stored, not arbitrary candidate state (#664)

M-I01: feat(contracts/ChannelHub): remove updateLastState flag (#665)

H-L06: fix(contracts/ChannelHub): remove payable from methods, clarify in create (#666)

H-L09: feat(contracts/ChannelEngine): add non-home migration version check (#669)

YNU-839: fix blockchain listener lifecycle (#658)

M-H09: use channel signer for all channel state node sigs (#667)

H-L07: fix(contracts/ChannelHub): restrict createChannel to non-existing channels (#668)

H-I02: docs(contracts): add a note that rebasing tokens are not supported (#670)

H-I03: fix(contracts/ChannelHub): use CIE pattern in depositToHub (#671)

M-H11: revert empty signatures on quorum verification (#672)

M-H11: reject issuance of receiver state during escrow ops (#674)

H-I01: docs(contracts): note that fee-on-transfer tokens are not supported (#675)

M-L04: docs: mention liquidity monitoring (#677)

fix(contracts/ChannelHub): fix initialize escrow deposit dos (#679)

M-H08: enforce strict transition ordering after MutualLock and EscrowLock (#680)

fix: run forge fmt (#681)

M-L05: docs(contracts): document native token deposits (#685)

fix(clearnode): resolve a set of audit findings (#686)

M-H13: feat(contract): add validateChallengeSignature, revert in SK validator (#688)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants