refactor(balances): share leaf parsing between bulk + single-chain paths; repair the push pipeline#315
Closed
BitHighlander wants to merge 1 commit into
Closed
Conversation
…n paths; repair the push pipeline
getBalance was a ~470-line hand-mirrored fork of getBalances and every one
of the six drift bugs the refresh audit found lived in a mirrored leaf.
Extract the leaves into shared module helpers so the paths cannot diverge
again (locality kept — orchestration stays per-handler):
- deriveChainAddress(): param shape, ripple quirk, { address } vs
{ publicKey } result shapes (hive), unknown-message capability recording
+ stale-cache-row drop — used by both handlers (getBalance previously
never recorded misses, so it kept re-sending rejected messages — the
#310 USB-detach class)
- isTokenEntry / parseTokenEntry / CONTRACT_CAIP_RE: token classification
+ parsing (getBalance and refreshWatchOnlyBalances regain the
denom|bank contract match — TCY/RUJI — and the caip-prefix networkId
fallback)
- dedupeTokensByCaip: the byte-identical dedup loop existed 5 times
- mapServerDefiPosition: the DeFi mapping existed 3 times
- fetchShieldedZecToken: zcash shielded zZEC merge — getBalance now
attaches it too (a single-chain zcash refresh used to wipe the cached
shielded sub-row via its tokens_json overwrite)
getBalance also now:
- honors response meta: degraded Pioneer data surfaces on the warning
banner and is treated as UNCONFIRMED (no force cache overwrite — a
degraded zero must not clobber a good cached balance)
- applies the same firmware/feature-flag chain gates as getBalances
- takes an optional forceRefresh param (default TRUE — user refresh /
post-send / tx pushes must bypass Pioneer's cache)
Push pipeline repair (socket leg):
- Parse JSON-stringified payloads and resolve the chain from
caip/networkId (the server's 'chain' field is symbol-ish, so genuine
watchtower 'transaction:incoming' events were mostly dropped); map the
server's payload types into the schema union ('confirmation_update' →
'confirmed') so the incoming toast works from this leg.
- Deliberately consume ONLY 'transaction:incoming'. Pioneer's
'balance:update'/'balance:cache:update' fire from INSIDE its
GetPortfolioBalances controller per request, to the requester's own
socket — consuming them creates a self-sustaining refresh feedback
loop (caught in adversarial review; documented in
docs/handoff-pioneer-hive-push-refresh.md §7 for the server-side fix).
- Dedup pushes by txid across the SSE and socket legs (one deposit no
longer double-toasts and double-fetches); merge pending debounced
payloads so a confirmation update can't eat an incoming-payment toast.
- Debounce map hoisted to module scope and cleared on disconnect (pending
timers used to fire getBalance at an unplugged device); SSE tx:confirmed
forwards now debounced per network (they re-fire per confirmation, each
costing a forced fetch).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BitHighlander
added a commit
that referenced
this pull request
Jul 20, 2026
Unblocks Hive internal-market swaps. Firmware clear-signs both ops (keepkey-firmware #315); the vault had no serializer, so hive_broadcast failed before the request reached the device. Byte layout mirrors the firmware parser and its unit-test vectors (Hive.LimitOrderCreateRetainsEveryDisplayedField / LimitOrderCancelParses). The firmware's degenerate-order rejections — same symbol on both sides, zero amount, non-HIVE/HBD symbol, non-0/1 fill_or_kill — are mirrored host-side so the user gets a clear error instead of a device reject. Requires firmware >= the release cut from #315. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
BitHighlander
added a commit
that referenced
this pull request
Jul 20, 2026
…sion tests keepkey-sdk had zero Hive coverage — no address getter, no sign methods — despite the vault exposing /addresses/hive and /hive/sign-operations since 7.15.0. Adds address.hiveGetAddress and hive.hiveSignOperations, mirroring the existing osmosis/tron/ton client blocks exactly. Scoped to what the new tests need; sign-transfer/sign-message coverage is a separate follow-on (see handoff-keepkey-sdk-clearsign-coverage.md). Adds two test suites, following the tests/thorchain/ruji.js pattern (live Vault + attached device/emulator, no mocks): - tests/hive/phase3-ops.js — limit_order_create/cancel (fw #315, vault #373) sign successfully, plus the host-side degenerate-order rejections (same-symbol, zero-amount, VESTS-on-market, mixed-tier) fire without a device round-trip. Requires firmware >= rc15, the vault's hive_enabled setting, and live Pioneer for TaPoS header data — the endpoint is explicitly STAGED in rest-api.ts pending this smoke pass. - tests/osmosis/format-amount-regression.js — send/delegate/undelegate still sign with amounts beyond a float's ~7 significant digits, proving the atof()+"%.6f" removal (fw #315) didn't break signing. Does not (and cannot, from the SDK) verify the OLED renders the exact digits — that's Gate-3 OLED proof, still owed. lib/*.js and openapi/swagger.json are the rebuilt output (npm run build); src/ is the source of truth.
BitHighlander
added a commit
that referenced
this pull request
Jul 20, 2026
…d on rc15
The STAGED marker said this endpoint must not be treated as released until a
live-device sign smoke passed. It has: firmware 7.15.0-rc15 (revision
23ef39c0, EmulatorZcash variant) signed the phase-1/2/3 op table end to end,
including limit_order_create / limit_order_cancel, via
keepkey-sdk tests/hive/phase3-ops.js (7/7).
Kept the half of that comment that is still true, reworded as what it
actually is: requireChainSupport('hive') can only compare ">=7.15.0", which
every 7.15.0-rc reports, so an rc predating the HiveSignOperations handler
(fw #307/rc10) or the phase-3 ops (fw #315/rc15) passes the gate and rejects
on-device. No finer-grained capability flag exists to check.
No behaviour change — the hive_enabled and firmware gates are untouched.
test-sign-gating 77 pass / test-rest 26 pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #314 — retarget to
developafter it merges.Why
getBalancewas a ~470-line hand-mirrored fork ofgetBalances, and every one of the six drift bugs the refresh audit found lived in a mirrored leaf (the hivepublicKeyfallback fixed in #314 being the user-facing one). This PR extracts the leaves into shared module helpers so the paths cannot diverge again — orchestration stays per-handler, only the leaf logic is shared.Shared helpers (
bun/index.tsmodule scope)deriveChainAddress(){ publicKey }fallback; unknown-message capability recording (getBalance never recorded misses → kept re-sending rejected messages, the #310 USB-detach class)isTokenEntry/parseTokenEntry/CONTRACT_CAIP_REdenom|bankcontract match (TCY/RUJI) and the caip-prefix networkId fallbackdedupeTokensByCaipmapServerDefiPositionfetchShieldedZecTokentokens_jsonoverwrite carried no shielded merge)getBalance behavior fixes
meta: degraded Pioneer data surfaces on the warning banner and is treated as UNCONFIRMED — no force cache overwrite, so a degraded zero can't clobber a good cached balance.getBalances(returns the empty-balance sentinel rather than deriving a flag-off chain).forceRefreshparam (default TRUE — user refresh / post-send / tx pushes must bypass Pioneer's cache).Push pipeline (socket leg)
caip/networkId(the server'schainfield is symbol-ish, so genuine watchtowertransaction:incomingevents were mostly dropped before); maps server payload types into the schema union (confirmation_update→confirmed) so the incoming toast works from this leg.transaction:incoming. Adversarial review caught that Pioneer'sbalance:update/balance:cache:updatefire from inside its GetPortfolioBalances controller, per request, to the requester's own socket — consuming them creates a self-sustaining refresh feedback loop (every balance query schedules the next one, with device USB traffic per cycle). Documented indocs/handoff-pioneer-hive-push-refresh.md§7 for the server-side fix.getBalanceat an unplugged device); SSEtx:confirmedforwards debounced per network (they re-fire per confirmation, each costing a forced fetch).Review
Same multi-agent adversarial pass as #314 (4 review dimensions → 12 findings → 8 confirmed after refutation → all fixed across the two PRs; 4 refuted as pre-existing on develop). Typecheck differential vs develop baseline (619 pre-existing errors): no new error classes — see #314's note about the broken
@types/minimatchmaking plaintsca silent false-green.🤖 Generated with Claude Code