Skip to content

dig-wallet: select_xch is a fourth largest-first rival and disagrees with its three siblings on overflow direction #428

Description

@MichaelTaylor3d

Task

There are now FOUR largest-first coin selectors in this repo, and the extraction of the canonical one
made the divergence between them ASYMMETRIC rather than merely duplicated. select_xch is the odd
one out and it differs in overflow direction, which is a behaviour difference on a money path,
not a style difference.

Context

The four:

# symbol overflow on the running total
1 crates/dig-wallet/src/sage/selection.rs:57 select_largest_first the canonical one, the extraction
2 crates/dig-wallet/src/sage/offers.rs:438 select_cats
3 crates/dig-wallet/src/sage/rpc.rs:4814 select_cat_rows
4 crates/dig-wallet/src/sage/offers.rs:412-429 select_xch sum += c.amount — plain +=

select_xch:420 accumulates with a bare +=. In a debug build that PANICS on overflow; in a release
build it WRAPS, and a wrapped sum can satisfy the sum >= need break, returning a coin set whose
real total is far below need. The consumer of mirror::funding (dig-node#421,
funding.rs:204-206) uses saturating_add on the same running total and refuses on shortfall
instead.

Two different wrong answers for the same input is worse than two identical implementations:
duplication costs maintenance, disagreement means one of them is wrong, and the one that ships in
release mode is the one that wraps.

Scope

Per CLAUDE.md §2.0's centralize-rival-implementations rule, the finding is the DISAGREEMENT and the
survivor must be NAMED. select_largest_first (#1) is the canonical home; the expected outcome is
that #4 — and, on inspection, #2 and #3 — call it and are DELETED, rather than being individually
corrected into agreement, which produces a fourth agreeing rival.

Before deleting: check whether each caller's tie-break and its failure type are genuinely the
canonical one's. select_xch sorts b.amount.cmp(&a.amount).then(a.coin_id().cmp(&b.coin_id())),
which looks like the canonical key but must be confirmed rather than assumed.

A regression test must exist for the wrap specifically — a fixture whose amounts sum past u64::MAX
— or the fix is unproven in the build that has the defect.

Evidence

Raised by the pre-merge security gate on dig-node#423 as a non-gating follow-up. Not absorbed into
that PR, whose re-gate scope was the reservation defect.

Parent: #421

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions