Skip to content

Fix: claim the credited swap-to-WIRE payout instead of waiting on a balance - #62

Open
heifner wants to merge 8 commits into
masterfrom
sec-150-claim-flows
Open

Fix: claim the credited swap-to-WIRE payout instead of waiting on a balance#62
heifner wants to merge 8 commits into
masterfrom
sec-150-claim-flows

Conversation

@heifner

@heifner heifner commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Companion to Wire-Network/wire-sysio#558 (SEC-150). The swap-to-WIRE flow fails against that branch until this lands.

sysio.reserv::paywire now credits a claimable balance rather than pushing sysio.token::transfer. It settles inside the never-throw consensus dispatch chain, where a pushed transfer let the recipient's on_notify handler abort the delivery — the chain runs notified receivers with no exception isolation.

flow-swap-to-wire polled the recipient's WIRE balance until a deadline, which can no longer become true: the payout sits in sysio.reserv custody until pulled. It now waits on the claimable balance and calls claimwire, so bridging into WIRE is settle-then-claim.

WireClient gains claimWire / claimPay and their claimable-balance readers. Those read the tables raw rather than through the typed contract-table accessor, because the new tables do not reach the typed surface until @wireio/sdk-core publishes the regenerated SysioContractTypes (Wire-Network/wire-libraries-ts#66). The doc comment names the exact call to switch to once that version is released.

Flows deliberately unchanged, having been checked:

  • flow-swap-from-wire never exercises a revert, so refundwire is not on its path — only the escrow leg, which still pushes from the user.
  • flow-reserve-lifecycle's WIRE assertions cover the matchreserve escrow, also still a user push.
  • flow-emissions-soak covers sysio.dclaim staker claims, which were already pull-based.

The custody assertion in the swap-to-WIRE flow still holds because the claim happens in the preceding step; that dependency is now stated in a comment rather than left implicit.

…balance

sysio.reserv::paywire now credits a claimable balance rather than pushing
sysio.token::transfer. It settles inside the never-throw consensus dispatch chain,
where a pushed transfer let the recipient's on_notify handler abort the delivery --
the chain runs notified receivers with no exception isolation.

The flow polled the recipient's WIRE balance until a deadline, which can no longer
become true: the payout sits in sysio.reserv custody until pulled. It now waits on the
claimable balance and calls claimwire, so bridging into WIRE is settle-then-claim.

WireClient gains claimWire / claimPay and their claimable-balance readers. Those read
the tables raw rather than through the typed contract-table accessor, because the new
tables do not reach the typed surface until @wireio/sdk-core publishes the regenerated
SysioContractTypes.

The other flows need no change: flow-swap-from-wire never exercises a revert, so
refundwire is not on its path; flow-reserve-lifecycle's WIRE assertions cover the
matchreserve escrow, which still pushes from the user; and flow-emissions-soak covers
sysio.dclaim staker claims, which were already pull-based.
Conflict in packages/flow-swap-to-wire/src/SwapToWireScenario.ts, in the
custody-settled assertion. Master reworked Output.wireLegFee from a scalar into a
WireFee object and narrowed the custody math to subtract only rewardShare,
because the winning underwriter's half stays in sysio.reserv as a uwfees accrual
until that account calls claimuwfee, which this flow never does.

Took master's accounting. This branch's note about the fee halves went with the
scalar it described and would now be wrong, so only its still-true point
survives: the payout leg leaves custody solely because the recipient-paid-exact
step above already claimed it.

@jglanz jglanz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: inline lambda await should use a factory pattern

Review nit on #62: the poll predicate was an inline async lambda that awaited
inside itself. `claimableReached` now sits with the other reads beside
`underwritersActive`, and the call site passes `() => claimableReached(...)` —
the shape every other read-backed poll in this scenario already uses.

No behaviour change.
@heifner

heifner commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

Nit addressed in 5742a26: the claimable poll is now a named claimableReached(ctx, account, target) beside underwritersActive, with the call site passing () => claimableReached(...) — matching the shape the other read-backed polls in this scenario already use. No behaviour change; prettier-clean and no new type errors.

Flagging for a re-approval since require_last_push_approval is on and the branch was CLEAN before this push.

@heifner
heifner requested a review from jglanz August 13, 2026 17:43
Comment on lines +92 to +98
async function claimableReached(
ctx: SwapScenarioContext,
account: string,
target: bigint
): Promise<boolean> {
return (await ctx.wire.getWireClaimable(account)) >= target
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't have try to fix my nit picking lol

function claimableReachedPredicate(
  ctx: SwapScenarioContext,
  account: string,
  target: bigint
): AsyncFunction<boolean> {
  return () => ctx.wire.getWireClaimable(account).then(amount => amount >= target)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, ok

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 2ca9733claimableReachedPredicate(ctx, account, target) returns the bound predicate and pollUntil takes it directly, no inline lambda.

One substitution: AsyncFunction isn't exported anywhere in the tree (not @wireio/shared, not @wireio/cluster-tool, not the deps), so the return type is spelled () => Promise<boolean>pollUntil's own predicate parameter type. getWireClaimable returns Promise<bigint> and target is a bigint, so the .then narrows as expected.

underwritersActive is left as-is: it predates this PR on master, so converting it would widen the diff past the review.

`claimableReached` was an async predicate the call site wrapped in an inline lambda. `claimableReachedPredicate` binds `ctx`, `account`, and `target` and hands `pollUntil` the predicate directly — `() => Promise<boolean>`, which is `pollUntil`'s own predicate parameter type.
@heifner
heifner requested a review from jglanz August 13, 2026 18:02
Change-Id: I4124d81f3a486306146bd81284c2427a43b0e3c5
`eslint` bans inline object types (STYLE.md "Interface Design"), and the two
claimable-balance readers each passed one as the `getTableRows` generic. Both now
use `WireClient.ClaimableRow`, declared in the companion namespace where the
client's other sub-types live.

Nothing had caught this: wire-tools-ts runs no PR checks, and lint only executes
inside the e2e gate's `//wire-tools-ts:build` — where it would have failed the
run before a single flow started.

`claimPay`'s doc also stops advertising a category-bucket share. `payepoch`
transfers to `sysio.ops` / `sysio.gov` directly, because ROA zeroes net/cpu for
every sysio-prefixed account and neither carries a contract that could emit the
claim inline, so neither could ever authorize one.

build + lint green; jest unchanged against master's baseline on this host.

Change-Id: I3e1a147f9cef30553b5b3d8537d3bb10d780a852
…ually parse

`getWireClaimable` and `getPayClaimable` passed the bare account name as
lower_bound/upper_bound. With json=true the node runs each bound through
`fc::json::from_string` and then `be_key_codec::encode_key`, so `wirercpt` is not
even valid JSON -- nodeop answered

  parse_error_exception: Unexpected char '119' in "wirercpt"  at /v1/chain/get_table_rows

and flow-swap-to-wire's recipient-paid-exact step died on the first poll. That
path had never run in CI before: every earlier run of this stack wedged at
bootstrap, so this is the first time the claim read was reached.

Three things were wrong, per chain_plugin.cpp:2349-2360 and database_utils.hpp:
the bound must be JSON; `encode_key` calls `.get_object()` and looks each field
up BY NAME from the ABI's key_names; and the leaf is uint64, encoded through
`val.as_uint64()`. `WireClient.nameKeyBound(field, account)` builds exactly that.

The field name is a parameter because the two tables disagree --
`wireclaims.account` vs `payclaims.account_name` -- and the uint64 rides as a
decimal string: a name's raw value (wirercpt = 16406239207934132224) is far past
Number.MAX_SAFE_INTEGER, and `as_uint64` parses a string variant.

Unit tests cover the object shape, the per-table field name, the decimal-string
type, and that distinct accounts produce distinct bounds. 23/23 green; lint clean.

Change-Id: If9d5f30c44355b5b220e16d2279efc53214f004a
The encoded bound from c50b630 made the query parseable, but it still could not
return anything: the node's upper bound is EXCLUSIVE --

  chain_plugin.cpp:  if (has_upper && kv >= ub_sv) break;

(the exclusive increment above it applies to `find`, not to lower/upper) -- so
lower == upper describes an empty range. flow-swap-to-wire stopped erroring at
`recipient-paid-exact` and started timing out there instead, 120s of polling a
query that could never match.

Reads now pass a lower bound only. Keys encode big-endian, so iteration is
numeric order and the first row at-or-after the key is this account's IF it has
one -- when it does not, the walk yields the NEXT account's row. The identity
check on the returned row is therefore load-bearing, not defensive: without it an
account with no claim reads back a stranger's balance.

Both readers now share one `claimableBalance` helper rather than duplicating the
block, since this is the second defect to be fixed in two copies of it.

Verified end to end: flow-swap-to-wire SUCCEEDED locally against
nodeop v1.0.0-a0aceec3c3 (wire-sysio#558 merged with master), zero failed steps.
Unit tests cover the absent upper bound, the matching row, a foreign row reading
back 0n, an empty table, and payclaims' distinct key + row field names.
28/28 green; lint clean.

Change-Id: I43005967d4f8ff171c89cc94d01836f031ca6da7
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.

2 participants