Prefer exact-sum token combinations in selector to avoid unnecessary change outputs Part: k=1

[Effi-S]

Fixes #2404

Part: K=1

Summary

Design proposal for reducing unnecessary change outputs in token selection by preferring exact-sum combinations of available tokens over the current greedy first-fit walk. Follows up on the lock-contention work in #2395 (PRs #2397-#2403) and the amount-blind-selection tracking issue #2017 — this is scoped specifically to change avoidance, not full amount-aware/input-minimizing selection.

This has already been through two rounds of independent adversarial review (checked against the actual sherdlock selector code, the real batched Postgres locking path, and the fetcher/cache implementation) — the design below reflects both rounds of fixes, not a first draft.

Design: Exact-Amount Token Selection (Change Avoidance)

Status: Proposal, not implemented. Tracked under
issue #2017 as a follow-on to the
lock-contention work in issue #2395
(PRs #2397–#2403). This document does not change current selector behaviour; see
selector.md for the implemented algorithm.

Revision history: two independent reviews have run against this document.

• Review 1 found the original mechanism was designed against a per-candidate "about to lock"
  decision point that doesn't exist in the batch (Postgres) locking path, and understated
  round-trip/indexing costs. Fixed by moving the completion logic out of the per-candidate
  lock call and into window construction.
• Review 2 found that fix was still wrong: checking for a completing candidate only at the
  moment the next candidate would overshoot is a strictly narrower rule than the problem
  statement needs — this document's own [30,40,70,200]/100 example is not solved by
  that rule, because by the time the check fires against gap=30, the 30-token has already
  been swept into the window and isn't available to complete it. This revision replaces the
  "check only at the boundary" rule with a full pre-search over the cached candidate set,
  run before any greedy accumulation, which does solve the example (see below).

Problem

Selector.selectInternal (token/services/selector/sherdlock/selector.go) is a greedy
first-fit: it walks candidates ascending by amount (same-amount runs shuffled, per #2399)
and stops as soon as the running sum reaches the requested quantity. It never looks for a
subset of candidates that sums to exactly the requested quantity — it stops at the first
candidate that tips the sum over the target, even when a different, still-unpicked candidate
would have completed the request with zero remainder.

Example: wallet holds [30, 40, 70, 200] (same currency), request is 100. Greedy walk:
30 → sum 30, 40 → sum 70, 70 → sum 140 ≥ 100, stop. Result: 3 inputs, sum 140, and the
transaction must produce a change output of 40 back to the same wallet. But 30 + 70 = 100 was available the whole time: 2 inputs, no change. Note this combination is not
reachable by checking only "is there a completing candidate for whatever gap remains once the
greedy walk is about to overshoot" — by the time the walk is about to overshoot (after 30
and 40 are already committed to the window), the 30 token is no longer available to pair
with 70. Finding 30 + 70 requires looking at combinations before committing to the greedy
order, not patching the greedy order at its last step.

Every unnecessary change output is a new token the wallet immediately has to track, fund a
future lock cycle for, and — per #2395 — a new opportunity for lock contention. Reducing
change outputs is therefore not just a UX/fee concern; it also shrinks the token population
that PRs #2398–#2403 are protecting from collisions.

Goals / non-goals

Goal: when an exact-sum combination of available (unlocked) candidates exists, prefer it
over a combination that overshoots and requires change — without regressing anything #2395's
stack already fixed, and in particular without adding a per-selection database round trip
beyond what the batched Postgres path (#2402) already does.

Non-goals:

• Not a general subset-sum solver. Wallets can hold many tokens; an unbounded search is a
  denial-of-service surface (a request can be crafted, or simply occur naturally under load,
  that makes selection scan or backtrack arbitrarily). The search below is bounded in both
  input count (k) and candidates scanned, and falls back to the existing greedy behaviour on
  any budget exhaustion or outright miss.
• Not a replacement for the ascending-order/bucket-shuffle candidate ordering from fix(selector): anti-join locked tokens and order candidates by amount #2399. That
  ordering exists specifically to spread contention across same-amount candidates; the fallback
  greedy walk is untouched by this proposal.
• Scoped to sherdlock only. simple has no ordered, windowed fetcher and no anti-join —
  building a bounded search on top of it would mean adding most of fix(selector): anti-join locked tokens and order candidates by amount #2399's machinery to a
  driver whose whole point is to stay minimal. If exact-match selection is wanted under
  simple later, that is a separate proposal.

Design

Core idea: a bounded pre-search over the cache, run before greedy accumulation starts

The mechanism is a distinct step that runs before selectInternal's existing window-growth
loop (sherdlock/selector.go:264-351) is entered for a given remaining = quantity - sum, not
a patch applied inside that loop:

1. Pre-search: look for a small set of up to k unlocked, not-yet-blacklisted candidates in
   the fetcher's already-loaded cache for this wallet+currency whose amounts sum to exactly
   remaining. This never calls s.next() and never touches the iterator cursor — it reads a
   snapshot of the cache's sorted-by-amount data (see Fetcher/cache interaction).
2. If found: that subset becomes window directly — the existing window-growth loop is
   skipped entirely for this attempt — and is claimed through the exact same single
   TryLockBatch call, with the exact same win/loss handling (sherdlock/selector.go:301-351)
   that already exists today. No new locking logic, no new race handling: the win/loss/blacklist
   code at lines 322-336 already tolerates a window where not every entry wins the race, so a
   partially-contended exact-sum window degrades exactly the way a partially-contended greedy
   window does today — the losers are blacklisted, sum includes only the winners, and the
   outer loop continues (which will re-run the pre-search against the new, smaller remaining
   before falling back to greedy again).
3. If not found within the scan/depth budget: fall through unchanged to today's window-growth
   loop. Because the pre-search never consumed anything from the iterator, nothing is displaced,
   discarded, or double-counted — the greedy walk starts exactly where it would have without this
   feature.

Running the search before any greedy commitment, over the full cached candidate set rather
than a shrinking suffix, is what makes 30 + 70 = 100 reachable: both tokens are still
"available" from the pre-search's point of view, because neither has been swept into a window
yet.

Algorithm

Given remaining = quantity - sum and the cache's ascending-sorted candidate list for this
wallet+currency (minus already-blacklisted/locked entries):

• k = 1 (single completing token): binary search the sorted cache for amount == remaining. O(log n).
• k = 2 (a completing pair): a standard two-pointer scan over the sorted cache — lo = 0,
  hi = n-1; while lo < hi: if cache[lo]+cache[hi] == remaining, record the pair; if < remaining, lo++; if >, hi--. O(n) in the cache size for this wallet+currency, one pass,
  no nested loop and no per-candidate point-query — this replaces the first revision's "for each
  candidate a, look up remaining - a" sketch, which could not be bounded cleanly against a
  point-query budget. Continue the scan (bounded by maxTieBreakCandidates, default 4) to collect
  a handful of valid pairs rather than stopping at the first, then shuffle among them before
  picking one — the same anti-hotspot rationale as fix(selector): anti-join locked tokens and order candidates by amount #2399's bucket shuffle, now stated as an
  actual rule instead of an open question.
• k > 2: not proposed. Cost grows combinatorially and the marginal benefit over k = 1..2
  is expected to shrink; left as a future, separately-justified change if data says otherwise.

Recommended default: k = 1 first, since it is the cheapest and covers the common
single-completing-token case (including cases the ascending-order fix in #2399 does not
already cover — see the worked example above). k = 2 is the next increment, costed and
bounded above rather than left vague.

Point-query fallback, and why it is now a backstop rather than the common path. The pre-search
above assumes the cache holds the wallet+currency's full spendable set, not a small lookahead
window. That is in fact how the cache is populated: cachedFetcher.update() loads via an
unfiltered SpendableTokensIteratorBy call and buckets the entire result by wallet+currency
key, with no per-key truncation — so for the wired fetcher configuration, the pre-search
ordinarily has the complete candidate set to work with already, in memory, with no additional
query. A point-query fallback (amount = target_gap, same anti-join predicate as
buildSpendableTokensIteratorByQuery, LIMIT a small constant) is kept only as a backstop for
the case the in-cache search comes up empty because the cache is momentarily stale relative to
the DB (a token created after the last refresh) — capped at maxPointQueries (default: 1) per
Select() call, and only ever attempted for k = 1 (the k = 2 two-pointer scan has no
per-candidate point-query fallback at all, by design — see Budgets below).

Costs and tradeoffs

• Indexing: a new index is required, not optional. TokenStore.GetSchema
  (token/services/storage/db/sql/common/tokens.go) defines five indexes on Tokens, none of
  which reference amount; the ORDER BY amount added in fix(selector): anti-join locked tokens and order candidates by amount #2399 is an unindexed sort today,
  affordable only because per-wallet candidate sets are small. The point-query backstop above
  needs a composite index, e.g. (owner_wallet_id, token_type, amount), matching the existing
  partial-index style already used for idx_owner_wallet_part, to stay a cheap lookup rather
  than a sequential scan. This index has a write-path cost — one more B-tree entry maintained on
  every token creation and consumption — that has to be weighed against the read-side benefit in
  the benchmark below; this document does not yet have production insert/delete-rate numbers to
  quantify that tradeoff and treats it as an open rollout question, not a settled one.
• Round-trip cost in the common case is genuinely zero, verified against the real batch-lock
  call. TryLockBatch/the Postgres LockBatch implementation treats its token-ID list as an
  unordered set — multi-row INSERT ... ON CONFLICT and the FOR UPDATE SKIP LOCKED join
  impose no ordering or contiguity requirement — so handing it a pre-search-selected window
  instead of a greedily-grown one is exactly as costly as today's single TryLockBatch call:
  no second lock round trip is introduced. The only added round trip is the point-query
  backstop, and it fires only on a stale-cache miss, which should be rare given the cache
  contains the full candidate set by construction (see above) — but "should be rare" still needs
  the benchmark in Rollout to become "is rare in practice."
• Budgets are split by cost, not conflated. maxLookaheadInputs (k) bounds combinatorial
  depth; a separate maxPointQueries (default 1, k = 1 only) bounds actual DB round trips;
  a separate maxTieBreakCandidates (default 4) bounds how many same-sum pairs the k = 2 scan
  collects before shuffling, all independent of each other so a cheap in-memory scan is never
  throttled by a budget meant for expensive round trips.
• No new race class. A pre-search-selected subset is locked through the exact same
  TryLockBatch call and existing win/loss/blacklist bookkeeping as any other window; a
  concurrent selector finding and racing for the same subset loses exactly the way any other
  contended-token race is lost today.
• Precision. The equality checks (amount == remaining, cache[lo]+cache[hi] == remaining)
  reuse the same token2.Quantity type and arithmetic the existing sum logic already uses
  (fixed-precision, NUMERIC(78,0)-backed, not floats), scoped to the same wallet+currency+type
  candidate set as the surrounding query — no new precision or multi-currency concern.

Fetcher/cache interaction

cachedFetcher.update() loads the entire spendable-token set (unfiltered by wallet/currency)
and shards it into per-wallet+currency slices with no truncation, and UnspentTokensIteratorBy
hands back a fresh permutation of that per-key slice. The pre-search reads that same per-key
slice directly — a plain sorted-by-amount view, not the shuffled permutation the greedy walk
uses for iteration order — so it sees the wallet's complete cached candidate set for the
requested currency and never disturbs the iterator the greedy fallback will use if the
pre-search misses. This requires one small, additive fetcher API surface: a way to read the
current cached slice for a wallet+currency without minting a new permutation or advancing any
cursor (e.g. a PeekSortedCandidates accessor alongside UnspentTokensIteratorBy), which does
not change the existing iterator's public behaviour.

One asymmetry is worth stating plainly: the point-query backstop reads live from the database,
bypassing the Ristretto cache layer entirely, while the cache the pre-search normally uses may
be serving data up to fetcherCacheRefresh stale. The backstop is therefore more current than
the common path, not less — intentional, since it exists specifically to catch what the cache
missed, but worth calling out rather than leaving implicit.

Cost of running the pre-search itself. Because it operates on an in-memory, already-fetched
slice, k = 1's binary search and k = 2's two-pointer scan are cheap relative to any DB
round trip — but not free for a pathological wallet holding a very large number of tokens for
one currency (e.g. many dust outputs), especially since selectInternal's outer loop can re-run
the pre-search on every immediate retry (up to maxImmediateRetries = 5). Bound this with a
maxScanCandidates cap: if the cached slice exceeds the cap, the pre-search only considers its
maxScanCandidates smallest entries (the slice is already sorted ascending, so this is a
deterministic truncation, not a random subset) — this only reduces the feature's recall for
unusually large per-currency candidate sets, it never affects correctness, since the fallback
greedy walk still runs when the pre-search doesn't find anything.

Interaction with existing mechanisms

• Anti-join / already-locked exclusion (fix(selector): anti-join locked tokens and order candidates by amount #2399): unchanged and reused — cached candidates
  were already filtered by the anti-join at fetch time, and the point-query backstop uses the
  same NOT EXISTS predicate, so the pre-search never proposes a candidate someone else already
  holds.
• Bucket-shuffle within same amount (fix(selector): anti-join locked tokens and order candidates by amount #2399): unchanged for the fallback greedy walk. The
  pre-search has its own, analogous shuffle for k = 2 tie-breaking (see Algorithm above); k = 1 has no ties to break (only one candidate can equal remaining).
• Immediate-retry / blacklist (fix(selector): sherdlock per-attempt lock blacklist (Phase 3 of #2395) #2398): a pre-search-selected candidate that loses its lock
  race is added to the same per-Select() blacklist as any other candidate, so a refetch during
  the immediate-retry layer, and the pre-search that runs again after it, won't immediately
  re-attempt it.
• Backoff / StubbornSelector layer: untouched — a full retry still re-runs
  selectInternal from scratch, re-deriving the pre-search fresh each time.
• Release on settlement (fix(selector): release token locks on transaction settlement #2400): unaffected; this proposal only changes which candidates are
  chosen among already-available ones, not the lock lifecycle.
• Lock-acquisition strategy (feat(selector): configurable Postgres lock-acquisition strategies (Phase 6, #2395) #2402): orthogonal — a pre-search-selected window is claimed
  through whichever lockStrategy is configured, exactly like any other window, and the
  round-trip analysis above is specifically about not undoing what feat(selector): configurable Postgres lock-acquisition strategies (Phase 6, #2395) #2402 achieves.

Configuration

Extend the existing token.selector YAML block (all new, all optional, defaulting to
preserving today's behaviour when unset):

token:
  selector:
    exactMatch:
      enabled: false                # default off until benchmarked (see Rollout)
      maxLookaheadInputs: 1         # k: 1 (single completing token) or 2 (completing pair)
      maxScanCandidates: 256        # cheap in-memory cap on cache entries the pre-search considers
      maxTieBreakCandidates: 4      # k=2 only: how many equal-sum pairs to collect before shuffling
      maxPointQueries: 1            # k=1 only: DB round-trip budget per Select() call, separate
                                     # from the free in-memory scan above

simple reads and ignores this block, same pattern as lockStrategy today.

Observability

Add counters alongside the contention metrics already introduced in #2397
(LockConflicts, DistinctTokensAttempted): ExactMatchAttempts, ExactMatchCacheHits,
ExactMatchPointQueryHits, ExactMatchMisses. Splitting cache hits from point-query hits (not
just an aggregate "hit") is what lets the round-trip cost claim above be checked against real
traffic instead of assumed.

Testing

• Unit: table-driven cases over synthetic candidate sets, explicitly including the
  [30,40,70,200]/100 example from Problem (must select {30,70}, not {30,40,70}), plus:
  no completion needed (single candidate already exact), k=1 cache hit, k=1 point-query
  backstop hit, k=2 pair found with multiple ties (verify shuffling occurs across repeated
  runs), a contended pre-search subset that partially loses its race (falls through to a
  second, smaller pre-search / greedy pass), no completion exists at any depth (unchanged
  greedy result, and identical to today's output), maxScanCandidates/maxPointQueries
  exhausted (falls back without hanging or an extra round trip).
• A dedicated test asserting the pre-search path issues exactly one TryLockBatch call (not
  two) for a cache-hit window — the concrete, testable form of the "zero additional round trips
  in the common case" claim.
• Extend TestHotTokenContention-style benchmarks (added in test(selector): close lock-contention testable gaps + extend sherdlock benchmarks (Phases 7-8, #2395) #2403) with an "exact-match enabled"
  variant, measuring RoundTrips() and cache-hit vs. point-query-hit rates, to confirm the added
  cost stays within the claims above under realistic contention.
• Fuzz: none required — this logic never parses untrusted bytes; it operates on amounts already
  validated by the token store.

Rollout

1. Land behind exactMatch.enabled: false (default off), scoped to sherdlock only, including
   the new PeekSortedCandidates-style fetcher accessor and the (owner_wallet_id, token_type, amount) index.
2. Benchmark with the extended TestHotTokenContention variant above, plus a "change-output
   rate" measurement against a synthetic wallet population (the metric this feature is meant to
   move), measuring both the read-side round-trip cost and the write-side index-maintenance cost,
   before flipping the default.
3. Only after that data is in, consider flipping the default to true and revisit whether k = 2 is worth enabling by default given its extra scan/tie-break cost — a separate, data-driven
   decision, not part of this initial rollout.

[github-actions]

📊 Token Validation Benchmark

Comparison of this PR against the base branch. 🟢 improvement · 🔴 regression · ➖ within ±1.0% noise.

Variant	Benchmark	Params	Workers	TPS (base → PR)	Δ TPS
csp	BenchmarkAPIGRPC	f=1, nc=4, w=token-validation-service	4	119 → 118	➖ -0.3%
csp	BenchmarkLocalTokenValidation	out-tokens=2in-tokens=2	4	116 → 116	➖ +0.1%
ipa	BenchmarkAPIGRPC	f=1, nc=4, w=token-validation-service	4	122 → 119	🔴 -2.4%
ipa	BenchmarkLocalTokenValidation	out-tokens=2in-tokens=2	4	76 → 76	➖ +0.1%

[Effi-S]

📊 Token Validation Benchmark

...
ipa BenchmarkAPIGRPC f=1, nc=4, w=token-validation-service 4 122 → 119 🔴 -2.4%
...

Not reproduced in following runs.. seems like a blip..

[AkramBitar]

Two items before merge, plus a couple of minor notes.

1. maxScanCandidates cap is missing (must-fix)
The design doc lists maxScanCandidates: 256 as a bound on the pre-search scan, but the code has no such cap. trySingleTokenExactMatch materialises every candidate from the iterator, sorts them, and binary-searches — on every selectInternal call, including all immediate and backoff retries. For a wallet with many dust tokens this is unbounded. Add a guard before the sort:

if len(candidates) > maxScanCandidates {
    return nil, nil, false
}

with maxScanCandidates defined as a constant alongside maxImmediateRetries.

2. Doc comment missing retry-multiplier note (should-fix)
trySingleTokenExactMatch is called once per selectInternal invocation, and StubbornSelector re-enters selectInternal on every backoff retry. The doc comment should say so, e.g.: "called once per selectInternal invocation; under StubbornSelector this includes each backoff retry."

---

Minor notes (no change required):

• t.Context() requires Go 1.21+ — confirm the module's go directive meets that.
• The UnspentTokensIteratorByCallCount() == 1 assertion in DisabledByDefaultRunsGreedy will break if the greedy fetch strategy ever changes; the meaningful invariant ("pre-search did not add a call") is already covered by the WithExactMatch disabled case — consider relaxing or dropping the count check.

[Effi-S]

@AkramBitar, ready for re-review
Minor note:

• t.Context(): the package is governed by the root go.mod, which is at go 1.27.1, so we're well past the requirement.

Fixed

[AkramBitar]

Reviewed at 43df02d7. go vet and go test ./token/services/selector/sherdlock/... ./token/services/ttx/dep/wrapper/... pass on the branch.

The feature works, but it is unreachable in production and it quietly corrupts the metrics it ships with. Two blockers, both small:

1. WithExactMatch has no production wiring. manager.go:58 calls NewSherdSelector(...) with no opts, and there is no config key, SDK/dig plumbing, or non-test caller anywhere in the tree. s.exactMatch is therefore always false in a deployed node: the three new counters are permanently 0, and docs/development/metrics.md:197-202 + testdata/metrics.golden advertise metrics that can never move. Either add the config/plumbing here, or state in the docs that the pre-search is not yet reachable.

2. The pre-search sits outside the existing metrics/logging contract established in #2395.

• selector.go:219 — trySingleTokenExactMatch is not passed the caller-owned attempted set and never adds to it, so Select's deferred observeDistinctTokensAttempted is skipped when attempted.Length() == 0. Every change-free selection vanishes from ..._distinct_tokens_attempted, and on a miss the pre-search's lock attempts are undercounted — contradicting the documented contract ("distinct tokens a lock was attempted on (won, lost, or rate-limited) per token selection call").
• selector.go:390-401 — the greedy walk distinguishes driver.ErrTokenAlreadyLocked (→ LockConflicts.Add(1) + debug) from a real store failure (→ WarnfContext). This loop does neither: every non-rate-limit error is logged as "candidate [%s] already locked, trying next" at debug. So (a) lock_conflicts_total undercounts by roughly half under exact-match contention, and (b) if the token-lock store is down, every candidate fails with a connection error reported as "already locked" at debug — bypassing the warning #2395 added precisely to make that outage visible.

Also worth folding into this PR

• selector.go:329 — on a miss the pre-search discards a full wallet fetch and the greedy walk's first s.next() immediately re-fetches, so every miss costs two full wallet scans (two per backoff round under StubbornSelector). With the default Mixed fetcher that also burns maxQueriesBeforeRefresh twice as fast, doubling background full-table update() refreshes and double-counting UnspentTokensInvocations. The candidate list is already in memory — feeding it to s.cache via swapCache on a miss removes the second fetch.
• selector_exact_match_test.go:160 — the branch the shuffle exists for (candidate A lost, candidate B acquired, still change-free) has no coverage: MultipleExactCandidatesSelectOne stubs TryLockReturns(true, nil) so the first candidate always wins, and the only test with a failing exact lock has a single candidate. None of the three new counters are asserted either, and they can't be with the current helper — setupMetricsMocks returns one shared FakeCounter, so a setupNamedCounterMocks (analogous to setupNamedHistogramMocks) is needed first.

Minor

• metrics.go:41 / metrics.md:200 — attempts is documented as "Select() calls that ran the pre-search" but increments per retry (selectInternal runs once per backoff round, as the doc comment at selector.go:310 correctly notes). hits / attempts is inflated by up to maxRetriesAfterBackoff; reword to "pre-searches run", or move the increment into Select.
• selector.go:360 — maxScanCandidates = 256 disables the feature for exactly the wallets it exists for (a wallet with 500 accumulated change outputs keeps minting change forever). The stated justification doesn't hold for the lazy/mixed fetchers: lazyFetcher.UnspentTokensIteratorBy already calls collections.NewPermutatedIterator, which materialises and permutes the whole wallet before the pre-search sees a token — so the cap only avoids an O(n log n) sort over an already-in-memory slice. Consider capping the sort rather than the feature (a single linear pass for an exact match is O(n) and needs no cap). Also, the comment at selector.go:33-34 says "unlocked candidates", but the iterator yields tokens locked by other processes too, so a wallet of 300 tokens with 290 held by others still bails.
• selector.go:217 — the pre-search checks isClosed() only at selectInternal entry, while the greedy walk re-checks on every s.next(). If Manager.Close(txID) wins that race the pre-search can still acquire a lock and return a successful selection for a selector already removed from selectorCache — a third outcome TestSelectorCloseDuringRetryIsRaceFree's invariant did not previously admit, leaving the lock to the expiry cleaner.
• selector.go:392 — speculative pre-search locks spend the per-wallet lock budget the greedy walk then needs. The Locker contract (interfaces.go:73-80) permits per-Lock rate limiting, and selector.go:200 treats SelectorRateLimited as a hard abort; with a burst of 3 and three contended exact candidates, the pre-search can exhaust the budget and fail a request that would previously have succeeded with 60+40. Latent today (the shipped limiter is at Select level), but worth a note.

The token/services/ttx/dep/wrapper/dbs_test.go hunk is a clean improvement — d.batcher.window is set before the goroutines spawn, the helpers already exist in status_batcher_test.go, and it removes a genuine wall-clock flake. No issues there.

[Effi-S]

@AkramBitar,
I Added config key exactMatch - off by default
I Also addressed the minor findings.

I'd like to align with you before finishing off the rest of the findings.

Please review that this is acceptable and I'll implement the rest of the selector changes.