[Fix] Point the DEX client at the right deployment, and cover the live swap and liquidity journeys - #121
Merged
Merged
Conversation
DEFAULT_API_URL shipped as amm-api.dev.provable.com, which indexes the pre-migration shield_swap_v3.aleo. Since #110 moved this SDK to shield_swap.aleo, that host serves pools which do not exist on the program the SDK reads — so discovery returned keys whose every chain read came back null, surfacing as "pool does not exist" rather than as a misconfigured host. Verified: a pool served by that API exists under testnet shield_swap_v3.aleo and under neither mainnet program, while the pools on api.testnet.swap.shield.fi exist under shield_swap.aleo. The API is deployed per-network on separate domains, so one constant cannot be right for both. shieldSwapActions derives it from the client's network, resolved per request so switchChain re-targets the API rather than leaving it on the network the client started from — the same defect class as the prover URL. baseUrl accordingly accepts a resolver and ApiClient.baseUrl becomes a getter. SHIELD_SWAP_API_URLS and defaultApiUrl() are exported for direct construction, and DEFAULT_API_URL is deprecated. Three integration suites defaulted to amm-api-staging.dev.provable.com, which now 404s on every path. That, not an outage, is why the DEX suites have been red: with the correct host and no env override, 35 live tests pass — including the route-quote test recorded as known-red.
pickInsertHint consulted slot.next_init_below/above, which bracket the pool's current tick rather than the target. Any bound further out than one initialized tick got a hint above itself, which the contract rejects on finalize — mined, reverted, fee consumed. Measured on the live ETH/USDCx pool at tick -200996: a lower bound of -203230 returned -200996, where the true predecessor is -273894. The docblock already carried this as a known limitation with the exact walk as a follow-up; this is that follow-up. The walk visits one entry per initialized tick, of which live pools hold 3 to 18, so the extra reads are few and bounded rather than proportional to the tick range. Also exports MIN_TICK_SENTINEL / MAX_TICK_SENTINEL. The list is anchored one step outside the usable range — ∓400_001 against MIN_TICK/MAX_TICK of ∓400_000 — and with no constant for it callers hardcoded -400001, as devnodeLifecycle does; that only holds while a pool's tick list is empty. Getting this wrong is how the first version of this patch silently returned an uninitialized tick. Verified against the live deployment: 30 hints across all 5 testnet pools, each initialized, strictly below its target, and with its successor at or beyond it.
Two gated suites covering the journeys that had no live coverage: swaps (single-hop, forced multi-hop, concurrent) and the liquidity lifecycle (mint, increase, decrease, collect, burn, plus the owned-position reads). Everything is discovered — pools from the API, balances via getBalances, tick spacing from the fee tier on chain, ranges from the active tick, insert hints from pickInsertHint. Nothing is hardcoded, so they follow the deployment. No createPool: pool_creation_open reads false on this deployment, so a suite that created its own pool would only ever skip. Verified live: single-hop swap and claim, the two-hop swap and claim, pool discovery against both API and chain gates, hint predecessor checks, and the mint with its on-chain position. Deposit amounts come from amountsForLiquidity at the current price rather than fixed figures, which balance for one pool's price and revert elsewhere. Three timing behaviours needed handling, each confirmed by a real failure first: finalize writes lag the confirmed transaction, so swap_outputs reads and claims poll; the record scanner indexes a minted position NFT after the mint confirms, so dependent steps wait for it; and multi-hop needs more than the default 5-minute confirmation window. Also exposes confirmationTimeout on createAleoClient, which createProvingConfig accepted but the factory did not forward. Two findings worth carrying forward. Concurrent swaps from one account revert on finalize even with disjoint input tokens. And minting into the native-credits pool reverts where the wrapped-token pool succeeds, so the suite prefers the deepest pool; the LP path there likely needs an explicit token route.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
The job pinned VEIL_DEX_API_URL to amm-api-staging.dev.provable.com, which now returns 404 on every path. Because it is continue-on-error, it has been failing silently — and the pin is what let it keep checking a decommissioned deployment long after the migration moved on, which is the same misconfiguration this branch fixes in the client. Dropping the pin lets the suites derive the host from the network, so the job follows the deployment instead of a snapshot of it. Renamed staging-shapes to dex-shapes since it no longer targets staging; no ruleset requires a status check by name, so nothing depends on the old one. Verified with CI's exact command and no env override: 26 passed, where it had been failing every path with 404. continue-on-error is kept — a live-API outage should not red the whole run — but the comment now says plainly that this hides real drift failures, which is how the staging rot went unnoticed.
There was a problem hiding this comment.
Pull request overview
This PR hardens @provablehq/shield-swap-sdk against a misconfigured DEX API host by deriving the API origin from the client’s network (and making it follow switchChain), fixes pickInsertHint to return contract-accepted predecessors, and adds/repairs live coverage for swaps and liquidity flows against the migrated shield_swap.aleo deployment. It also exposes confirmationTimeout through createAleoClient to support slower multi-transition confirmations.
Changes:
- Derive Shield Swap DEX API host per network (mainnet vs testnet), update docs/examples/tests/CI to use the derived default, and deprecate the old constant default.
- Fix
pickInsertHintby walking the initialized-tick linked list; export tick-list sentinel constants. - Add live swap + liquidity E2E suites and forward
confirmationTimeoutthroughcreateAleoClient.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| site/docs/packages/shield-swap.md | Updates docs to rely on network-derived DEX API host (api: {}) instead of a hardcoded dev URL. |
| site/docs/guides/shield-swap.md | Updates guide examples to use derived API host instead of hardcoded dev URL. |
| site/docs/guides/agents.md | Updates agent guide example to use derived API host. |
| packages/shield-swap/test/integration/traders.integration.test.ts | Switches integration test default indexer host to the testnet Shield Swap host constant. |
| packages/shield-swap/test/integration/reads.integration.test.ts | Switches read integration suite default indexer host to the testnet Shield Swap host constant. |
| packages/shield-swap/test/integration/liveSwaps.e2e.test.ts | Adds gated live E2E coverage for single-hop, forced multi-hop, and concurrent swaps. |
| packages/shield-swap/test/integration/liveLiquidity.e2e.test.ts | Adds gated live E2E coverage for mint/increase/decrease/collect/burn liquidity lifecycle. |
| packages/shield-swap/test/integration/api.integration.test.ts | Switches API integration suite default host to the testnet Shield Swap host constant. |
| packages/shield-swap/test/decorators/shieldSwapActions.test.ts | Adds unit coverage asserting API host derivation + switchChain retargeting. |
| packages/shield-swap/src/utils/tick-hints.ts | Reworks pickInsertHint to walk the tick list via getTick and return true predecessors. |
| packages/shield-swap/src/utils/q128.ts | Exports MIN_TICK_SENTINEL / MAX_TICK_SENTINEL for tick-list anchoring. |
| packages/shield-swap/src/index.ts | Re-exports the new tick sentinel constants. |
| packages/shield-swap/src/decorators/shieldSwapActions.ts | Derives DEX API base URL from client network per request (to follow switchChain). |
| packages/shield-swap/src/api/client.ts | Introduces SHIELD_SWAP_API_URLS + defaultApiUrl(), makes baseUrl dynamic (getter) and accepts baseUrl functions. |
| packages/shield-swap/README.md | Updates README examples to use derived API host (api: {}) instead of a hardcoded dev URL. |
| packages/provable-sdk/src/index.ts | Plumbs confirmationTimeout through createAleoClient to createProvingConfig. |
| packages/provable-sdk/README.md | Updates README example to use derived Shield Swap API host. |
| examples/shield-web/private-swap.ts | Updates example default indexer host to the testnet Shield Swap API. |
| examples/shield-swap-swap.ts | Updates example default indexer host to the testnet Shield Swap API. |
| examples/shield-swap-liquidity.ts | Updates example default indexer host to the testnet Shield Swap API. |
| .github/workflows/ci.yml | Renames/retargets the shape-parity job to live testnet DEX API and removes stale staging host env pin. |
| .changeset/tick-insert-hints.md | Changeset documenting the pickInsertHint correctness fix + sentinel exports. |
| .changeset/dex-api-network-hosts.md | Changeset documenting per-network API host derivation and dynamic baseUrl behavior. |
| .changeset/confirmation-timeout.md | Changeset documenting confirmationTimeout being exposed on createAleoClient. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
`waitForConfirmation` defaulted to 300s. Measured against the live testnet deployment, healthy confirmations land far inside that — a mint took 49.7s and an increase 39s, both including proving — so a transaction still absent at the limit is more often one the node never included than one about to arrive. The default is now 60s. This is a behaviour change: a write that previously confirmed between one and five minutes now throws instead of returning. Multi-hop swaps are the known slow path, one measured at 322s, so a client submitting them sets `confirmationTimeout` explicitly — documented on the multi-hop section of the shield-swap README and in the `createProvingConfig` reference, and applied to the live swap suite. Every polling failure was previously swallowed, so a node that answered cleanly and consistently did not have the transaction was indistinguishable from one that could not be reached — while the message asserted the transaction "may still be pending", which is backwards for one that was dropped before inclusion. `TransactionTimeoutError` now carries `polls` and `absentPolls` and says which case it saw. It does not diagnose why: the confirmed-transaction endpoint cannot tell a pending transaction from a dropped one on its own.
…rLiquidity The package could turn a liquidity figure into token amounts but not the reverse, which is the direction a depositor starts from: a caller holds two balances and wants to know what position they support. Without it every caller had to invent a liquidity number and work forwards, and a figure that balances at one pool's price falls short at another — one side runs out and the mint reverts. Mirrors the contract's derivation, with the same branch boundaries as `amountsForLiquidity`: token0 binds at or below the range, token1 at or above it, and inside the range the shorter side governs. Every step floors, so the result is a lower bound — feeding it back through `amountsForLiquidity` with deposit-side rounding returns amounts that fit inside the originals, which keeps a mint from reverting for want of a base unit. That property is asserted across a sweep of ticks, range widths and magnitudes rather than on one case.
The live liquidity lifecycle failed at `decrease`, then at `collect`, with a confirmation timeout against a transaction the chain had never heard of — 404 on both the confirmed and unconfirmed endpoints, permanently. Each write spends the position record and creates a new one, and the scanner indexes that asynchronously, so a write built moments after the previous one carries a serial number the chain has already consumed. The node drops it at verification, where it never becomes a rejected transaction and never reaches a block. The suite only waited for the scanner after `mint`, and that wait checked mere presence — which the spent record also satisfies. It is now a freshness wait keyed on the record tag, run after every write that respends the position. `burn` needed the other lag: the mapping delete propagates to reads asynchronously, so the position read back with liquidity 0 immediately after the burn confirmed and null shortly after. Both views are now polled. Also: spacing comes from `slot.tick_spacing`, which is what `mint` aligns against, with the fee registry asserted to agree rather than used as the source. Insert hints are left to `mint`, which applies a correction a caller cannot — passing an explicit `tickUpperHint` disables it and reverts whenever no initialized tick sits between the bounds. The deposit is a thousandth of what the account holds, so repeated runs cannot drain it, and the minted liquidity is checked against `liquidityForAmounts` with the chain as the authority. A failure aborts the remaining steps instead of paying a fee each to revert. Verified live on testnet: everything through `collect` passes, and the mint parity assertion holds. The `burn` poll is the one change still unverified.
The mint confirmed and its `positions` entry still read back null, failing the step against a position the chain had created — the same asynchronous propagation that made `burn` read back with liquidity 0 after the entry was already removed. Every step in the lifecycle reads exactly what its own write just changed, so all five had this race and were passing on timing luck. They now share one helper that polls the position read until the entry shows what the write did: present after mint, increased after increase, zero after decrease, cleared after collect, absent after burn. The scanner's separate lag on the burned record keeps its own poll, since it is a different view catching up rather than the same one.
…latency The polling bounds were 200s, which is far past useful: mapping propagation is quick, and a read that has not caught up long after its write is a signal, not a slow network. Mapping reads now poll every second for ten seconds. The scanner-freshness waits keep a longer 30s bound, because record indexing is measurably slower — the mint's record took 8–15s to appear across runs — and that wait failing means the next write builds on a spent record and is dropped. The scanner polls also tolerate a failed read inside their window rather than failing the step on one error, and attach the last failure if none succeed. The SDK already force-refreshes the JWT and retries a 401 four times, which handled the intermittent scanner errors seen on these runs; this survives an outage longer than that. The burn test no longer asserts the scanner has dropped the record. It marks records spent on its own schedule — still serving one 30s after the burn confirmed — so gating on it makes the suite hostage to third-party indexing latency rather than to anything this SDK does. The chain-side assertion, that the `positions` entry is gone, is what proves the burn worked and passes inside 10s. Verified live: mint with its parity check, the owned-position reads, increase, decrease and collect all pass. Two runs were cut short by Provable API auth flakiness — a scanner 'iss' 401 and a prover 401 on /prove/testnet/pubkey — neither related to these changes.
The docs described it as a mint that has not finalized, which is one of two causes. A live run measured the record scanner still serving a burned position more than four minutes after the burn confirmed, so the same `null` equally means the position no longer exists — and a caller rendering a portfolio that reads it as "still loading" shows a phantom position for minutes. Both directions of the record/mapping lag are now stated, in the action's docblock and the README, with the public mapping named as what settles which case it is.
`buildApi` set the derived host and then spread the caller's `api` options over it, so `baseUrl: process.env.VEIL_DEX_API_URL` with the variable unset passed the key with an undefined value and won the spread. `ApiClient` then fell back to its deprecated testnet constant, pointing a mainnet client at testnet — reading pools that do not exist on the program it proves against, with nothing in the config to suggest it. The coalesce now runs after the spread, so only a set `baseUrl` wins. Also corrects the `pickInsertHint` docblock, which still described deriving hints from the slot's active-range neighbours after the implementation moved to walking the initialized-tick list. It now says what the code does and why the neighbours are unusable: they bracket the pool's current tick, not the target. Both reported by Copilot on #121. The regression test fails against the previous spread order.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 35 out of 35 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
packages/shield-swap/test/integration/liveSwaps.e2e.test.ts:92
heldis built fromclient.getBalances()with no token filter, which intentionally omits zero-balance tokens. In the pool search predicate,held.get(p.token0)! > 0nwill throw when the token id is not present in the map (it will beundefined, not0n). This can crash the test before it ever finds a funded pool.
const pool = pools.find((p) => held.get(p.token0)! > 0n || held.get(p.token1)! > 0n)!
const tokenInId = held.get(pool.token0)! > 0n ? pool.token0 : pool.token1
const amountIn = held.get(tokenInId)! / 1000n
packages/shield-swap/test/integration/liveLiquidity.e2e.test.ts:291
- The drift check converts
bigintliquidity values tonumber(Number(...)), which will lose precision and can overflow for realistic u128 liquidity magnitudes. That can make the 1% tolerance assertion flaky or incorrect.
const drift = Number(position!.liquidity - state.predicted!) / Number(state.predicted!)
expect(Math.abs(drift), `predicted ${state.predicted}, chain minted ${position!.liquidity}`).toBeLessThan(0.01)
Walking the tick list reads `ticks`, which is keyed by a hash of pool and tick, so it derives keys through `@provablehq/sdk`. `mint` calls `pickInsertHint` whenever hints are omitted — and `mint` uses the soft loader while `increaseLiquidity` never loads WASM — so the walk made both require the peer and broke wallet-backed browser installs that previously minted fine. That contradicts the design in `utils/sdk.ts`, which reserves WASM for local-account derivations. An absent peer now degrades to the slot's neighbours: one mapping read keyed by the pool, deriving nothing, and exactly what this function returned before the walk existed. Callers with the peer keep the true predecessor for any target; callers without it are no worse off than before the change. Documented as best-effort, with explicit hints named as the remedy for a distant range. Reported by Copilot on #121. The fallback tests fail against the unguarded walk.
`pickInsertHint` needs `@provablehq/sdk` to hash tick keys for the on-chain walk,
and fell back to the slot's two neighbours without it — correct only for a target
within one initialized tick of the current price, and rejected by finalize for
anything further out.
The API answers this exactly and was already in the vendored spec, unused:
`GET /pools/{key}/initialized-ticks` returns the pool's full sorted tick list, and
its description names this as the purpose — computing the hints the AMM's
hint-walk asserts on, which is what the frontend mint flow does. Exposed as
`client.api.getInitializedTicks`, and supplied by the decorator to pickInsertHint,
mint, and increaseLiquidity, so a wallet-backed client without WASM gets the exact
predecessor rather than a guess.
Three tiers by authority: the contract's list when the peer is present, the API
list when it is not, the slot's neighbours when neither is. The chain stays
preferred because the API indexes positions rather than reading the contract, so
it can lag a fresh mint, and a stale hint costs a fee. A failing or
unauthenticated API drops to the slot rather than failing the write.
Verified live against three testnet pools: the API-derived predecessor matched the
chain walk on every one, including a 17-tick pool where the walk takes 17 round
trips and this takes one.
The tick list is attached to hint-deriving actions as a supplier rather than a fetched array, so a client that can derive tick keys never pays for the fallback being wired up. Asserted rather than assumed: the fetch spy records no `initialized-ticks` request while `pickInsertHint` walks the chain. Also dedupes the request across an action's hints. `mint` derives two and passed the same supplier to both, which fetched the identical list twice on the fallback path. The promise is now shared, rejection included, so both hints fall back to the slot together rather than disagreeing about their source.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated no new comments.
Suppressed comments (1)
packages/core/src/errors/errors.ts:254
TransactionTimeoutErrorcan report "absent on all 0 polls" whenwaitForConfirmationis called withtimeoutMs <= 0(the loop never runs, but the error is still constructed with{ polls: 0, absentPolls: 0 }). That message is misleading/awkward; consider suppressing the observed-polls suffix whenpollsis 0 (or emitting a dedicated "no polls performed" message).
const observed =
opts.polls === undefined || opts.absentPolls === undefined
? ''
: opts.absentPolls === opts.polls
? ` The node reported it absent on all ${opts.polls} polls.`
: ` ${opts.absentPolls} of ${opts.polls} polls reported it absent; the rest did not reach the node.`
…safely
Review of this branch's own changes turned up two defects.
`predecessorOf([])` returns the sentinel, so an API answering `{data: []}` because
the pool is not indexed yet produced a hint below every initialized tick, which
finalize rejects. An empty list now falls through to the slot instead: a pool that
genuinely has no initialized ticks anchors at the sentinel, which the slot reports
as `next_init_below` anyway, so the safe case loses nothing and the stale case is
covered.
The 404 check read `.status` off the caught value directly, which throws on a null
rejection — inside the loop whose purpose is to survive transient failures. Now
optional-chained.
Also keeps `tickLowerHint`/`tickUpperHint` adjacent in the increaseLiquidity
docblock, which the new property split apart.
The empty-tick-list fix shipped with a test; the null-safety fix did not. A rejection carrying no status now asserts that the timeout is still what surfaces and that `absentPolls` stays 0 — a rejection with no status is not evidence the node reported the transaction absent. Fails against the direct property read it replaced.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated no new comments.
Suppressed comments (2)
packages/shield-swap/src/utils/tick-hints.ts:150
pickInsertHinttreatsgetTick(...) === nullthe same as “end of list” and returnscursor. In the WASM-enabled branch this can return an uninitialized tick as the hint (e.g. if the tick list is malformed or a tick read unexpectedly returns null), which would immediately fail the contract’s predecessor assert and consume the fee. Sincemint/increaseLiquidityalready verified the pool/slot exist, a null tick here is unexpected and should be treated as an error (or an explicit fallback), not a valid stop condition.
for (let hops = 0; hops < MAX_HINT_HOPS; hops++) {
const tick = await getTick(client, { poolKey: params.poolKey, tick: cursor, program: params.program })
const next = tick?.next
if (next === undefined || next >= params.targetTick) return cursor
cursor = next
packages/shield-swap/test/integration/liveLiquidity.e2e.test.ts:292
- This drift calculation converts potentially-large
bigintliquidity values tonumber(Number(position!.liquidity - state.predicted!) / Number(state.predicted!)). If liquidity exceeds ~2^53, the conversion loses precision (or can overflow toInfinityin extreme cases), making the 1% tolerance check flaky/non-deterministic. This can be done exactly with integer math instead.
iamalwaysuncomfortable
added a commit
that referenced
this pull request
Aug 5, 2026
Three findings, all real. `withStore` tested truthiness, so `blindedIdentities: undefined` — a caller opting one call out of tracking — had the default store injected over the top of it, while the adjacent comment claimed the opt-out worked. It now tests key presence, so an explicit undefined is honoured and only an absent key takes the default. This is the same shape as the `buildApi` spread bug in #121: a truthy check standing in for "was this specified". Pinned by a test that fails against the previous version. Both file stores treated `ENOTDIR` as an absent file. It is a malformed path — a component that is not a directory — so nothing can be stored there, and reading it as absent meant restarting counters at 0, or in the credential store registering a replacement consumer whose key then could not be persisted. Failing at load names the bad path before any of that. An earlier test asserted the weaker behaviour and now asserts this one. The changeset and README claimed "without a store, behaviour is exactly as before". That holds for the standalone `swap(client, params)` but not for a composed client, which gets an in-memory store by default and therefore tracks. Corrected in both, along with what the default does not give you: no persistence, so a restart rescans for its counter and forgets an unclaimed swap.
iamalwaysuncomfortable
added a commit
that referenced
this pull request
Aug 5, 2026
…ry (#122) * [Feat] Reserve blinded identities so concurrent swaps stop reverting Parallel swaps from one local account reverted on finalize. A swap binds to a blinded identity, and the program asserts each blinded address is used only once. With no `blindedIdentity` passed, `swap` derived one via `nextBlindedIdentity`, which returns the first counter the chain does not carry — correct in sequence, wrong in parallel: two concurrent swaps read the same unused counter and the second reverts. Nothing surfaces locally, because at proving time the address genuinely was unused. Disjoint input tokens do not help; the identity is per account, not per token. `reserveBlindedIdentity` records a reservation before returning it and never issues a counter at or below one already stored, so an unconfirmed swap keeps its counter. It advances monotonically from the highest known counter and skips addresses the chain already carries, which recovers a store another process has moved past; an empty store scans from 0, so a lost store costs reads rather than correctness. Local accounts only — a wallet tracks its own identities. Reservations persist through a `BlindedIdentityStore`: `memoryBlindedIdentityStore` by default, or `fileBlindedIdentityStore` on the new `/node` entry point to survive restarts. `recordBlindedSwap` attaches the swap id and `syncBlindedIdentities` promotes each record to `swapped` or `claimed` against the chain, which is what makes pending proceeds recoverable after a crash. Also exposes `resolveDexImports` as a client action, and moves the live swap suite onto reservations — including an assertion that concurrent swaps get distinct addresses. * [Refactor] Move the blinded-identity actions into their own files Three decorator-bound functions shared `utils/blinding/store.ts` with the store implementations and types, against the convention every other action follows — one exported function per file under `actions/`. The two that take a client and read chain state are now actions in their own files: `actions/blinding/reserveBlindedIdentity.ts` and `actions/blinding/syncBlindedIdentities.ts`. Their tests move alongside, at `test/actions/blinding/reservation.test.ts`. `recordBlindedSwap` stays in `utils/blinding/store.ts` deliberately. It takes a store rather than a client and never touches the network — it is a store write, and giving it a client parameter it does not use to qualify as an action would be the shape lying about what it does. The store types, the in-memory store, and the lock the two actions share stay with it. No behaviour change; the public surface is identical. * [Refactor] Move recordBlindedSwap into its own action file Completes the split: all three blinded-identity actions now live one per file under `actions/blinding/`, matching every other action in the package. `utils/blinding/store.ts` is left with what it is actually about — the store types, the in-memory store, and the lock that serializes read-modify-write across the two actions that mutate it. The signature stays `(store, params)`. It takes no client because it never touches the network, and adding an unused one to look like its siblings would make the shape misdescribe the work. * [Feat] Add reconcileSwapHistory to recover a store's past from chain Blinded identities are derived rather than recorded, and `claim_swap_output` ends with `remove swap_outputs[r2]` — so once a swap is claimed, the claim call is the only public trace tying the identity to it. Its inputs carry what a store needs: blinded address, swap id, both token ids, and the amounts. `reconcileSwapHistory` walks that history with `getProgramCallsPaginated` and `getTransaction`, matches input 1 against the store's identities, takes input 2 as the swap id, and marks the matches `claimed`. Input positions are read from the program's own signature rather than guessed. Kept separate from `syncBlindedIdentities` rather than hidden behind a flag on it. They answer different questions — "is what I recorded still true" against "what happened that I never recorded" — and differ by two orders of magnitude in cost, since one is bounded by the store and the other by chain history. They also have different requirements: this needs no local account or WASM, only public data. Needing a flag to switch cost class is the signal that it is a second operation. It stops as soon as every identity is accounted for, so a current store costs one page, and reports `complete: false` when it exhausted `maxPages` with history remaining rather than implying it reached the end. It cannot surface unclaimed swaps, which have no claim call by definition; that stays with `syncBlindedIdentities` and the mapping. Verified live on testnet: seeded with a blinded address from a real claim, it recovered the swap id, token pair, and amounts in two pages, and the recovered id reads null from `swap_outputs` — as a settled claim should. Documented in the README with a first-run recipe and a section on implementing `BlindedIdentityStore` against any backing store. * [Test] Cover the file-backed identity store against live testnet Walks one identity through every status the chain can put it in and asserts each transition against what sync reads back from the mappings, not against what the local calls assumed: `reserved` before the swap lands, `swapped` while its output sits in `swap_outputs`, `claimed` once the claim removes that entry. Two things it checks that a memory store could not. Persistence is read through a second `fileBlindedIdentityStore` on the same path every time, because a store that only ever agreed with itself in memory would pass while writing nothing. And the last case throws the file away, rebuilds from a bare reservation, and recovers the swap id from chain history — then shows sync alone cannot do it, landing on `swapped` with no swap id, because the mapping entry the claim consumed is gone. The store is a fresh temp file per run, so reservation starts cold and the counter scan runs for real. Spends one swap and one claim, sized at a thousandth of the balance, with the abort guard so a failure costs one transaction rather than four. Verified live: 4/4 on the first run — 4.6s, 50.8s, 29.6s, 0.4s. The last is short because the claim it looks for is the newest call on the program, so the early exit ends the walk on page one. * [Feat] Track blinded identities inside the swap and claim actions Reservation had to be driven by hand — reserve, swap, record — while the default path still derived identities by scanning the chain, which is safe in sequence and reverts in parallel. `swap` and `swapMultiHop` now reserve before submitting and record the handle after; `claimSwapOutput` marks the identity claimed. Two concurrent `client.swap()` calls can no longer collide on one identity, which is what the live concurrency test now exercises directly. All of it is conditional on a configured store. Without one nothing changes: chain-scan derivation, no writes, no new failure modes. An explicit `blindedIdentity` opts out per call, so there is no flag to contradict the config, and wallet accounts are untouched because their identities are derived behind resolve requests the client never sees. Records carry the whole handle now. A claim consumes a handle rather than a swap id, so this is what makes recovery real — a process can claim a swap it did not make. Handles persist through an explicit decimal-string shape rather than a bigint reviver, which cannot round-trip a digit-only string field back to a string. The two failure policies are deliberately opposite. Recording after a landed swap throws `SwapRecordingError` carrying the handle: the transaction is on chain so resubmitting costs more input, but the swap id is knowable only then, and losing it means proceeds nothing can claim. Marking after a claim warns and continues: the funds are already in the account and reconcileSwapHistory repairs the record. * [Test] Prove the collision the identity store prevents, and drop a racy read Adds the negative case: with no store configured, two concurrent derivations of a blinded identity return the same counter, the same address, and the same blinding factor — so two swaps built that way fight over one identity and the second reverts on the uniqueness assert. The contrast case runs the same account at the same moment through a store and gets distinct identities, both unused on chain. Asserted on the derivation rather than by submitting two doomed transactions. The collision is the part this SDK controls; paying two fees to watch the chain reject the second would prove the contract's behaviour instead, and leave a reverted transaction behind. Both cases are read-only and spend nothing. Also removes a duplicate `swap_outputs` read after the claim. `claimed` is by definition the state where sync found that entry gone, so re-reading it only sampled an eventually-consistent value a second time — which disagreed on a live run, failing the step over a claim that had demonstrably succeeded. Live: 6/6 on testnet. * [Fix] Treat a recording write that matched nothing as a failure Review of my own change: `recordBlindedSwap` no-ops when the store holds no record for the handle's blinded address. That is right on the manual path — a wallet-derived identity was never in the store — but on the tracked path it meant `recordSwapOrThrow` could report success while writing nothing, losing the swap id exactly as silently as the throw was added to prevent. It now reports whether it matched, and the tracked path throws `SwapRecordingError` when it did not. Only reachable if something replaced the store's contents between the reservation and the record, which is precisely the case worth hearing about rather than absorbing. * [Fix] Address the Copilot review on identity tracking Three findings, all real. `withStore` tested truthiness, so `blindedIdentities: undefined` — a caller opting one call out of tracking — had the default store injected over the top of it, while the adjacent comment claimed the opt-out worked. It now tests key presence, so an explicit undefined is honoured and only an absent key takes the default. This is the same shape as the `buildApi` spread bug in #121: a truthy check standing in for "was this specified". Pinned by a test that fails against the previous version. Both file stores treated `ENOTDIR` as an absent file. It is a malformed path — a component that is not a directory — so nothing can be stored there, and reading it as absent meant restarting counters at 0, or in the credential store registering a replacement consumer whose key then could not be persisted. Failing at load names the bad path before any of that. An earlier test asserted the weaker behaviour and now asserts this one. The changeset and README claimed "without a store, behaviour is exactly as before". That holds for the standalone `swap(client, params)` but not for a composed client, which gets an in-memory store by default and therefore tracks. Corrected in both, along with what the default does not give you: no persistence, so a restart rescans for its counter and forgets an unclaimed swap. * [Feat] Summarize what a store is still owed with getUnclaimedSwaps One entry per output still sitting in `swap_outputs`, per-token totals, and a handle rebuilt from the store so each entry can be claimed by a process that did not make the swap. That last part is what the stored handles were for: `claimSwapOutput` consumes a whole handle, so a summary without one reports money it cannot help you collect. Reads the mapping rather than trusting stored statuses, so an entry appears exactly when a claim would succeed — a record the store still calls `swapped` whose output was claimed elsewhere is omitted rather than promised. Totals count both sides, because a claim pays the output token and refunds whatever of the input went unfilled. Identities the chain has consumed whose swap id was never recorded are reported as `unresolvable` rather than dropped. Nothing on chain locates their proceeds until a claim exists, so they need reconcileSwapHistory and only after something claims them — saying so is more useful than an empty list that looks like "nothing owed". Six tests, including that the chain overrides a stale stored status, that a reserved identity with no swap id is not mistaken for a lost one, and that claimed records cost no reads. * [Test] Close the coverage gaps found in the final review An audit of every symbol this PR adds turned up two paths tested only live. `swap`'s tracked path now has offline coverage: that it reserves from the store before submitting, records the swap id and whole handle after, and surfaces `SwapRecordingError` carrying the handle when the second write fails while the transaction is already on chain. These use a view key generated per run, because the shared fixture's placeholder is not a real key — the other cases get away with it by passing an explicit identity, but reservation derives, and derivation is the point here. One of those assertions was wrong and the code was right: recording leaves the status `reserved`. Status tracks what the chain shows, not what the local process just did, so a submitted-but-unconfirmed swap never claims to be settled. Now asserted explicitly, since it is a property worth defending. `getUnclaimedSwaps` had unit tests but no live exercise. The store e2e now checks it on both sides of a real claim: reported as owed and claimable while the output sits in `swap_outputs`, absent once the claim removes it — a stale "still owed" there would send a caller to claim twice. Live: 6/6 on testnet.
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.
Three related changes, each its own commit. The first is the one that matters for release; the other two came out of trying to validate it.
1. The DEX API host was pointing at the wrong deployment
DEFAULT_API_URLshipped asamm-api.dev.provable.com, which indexes the pre-migrationshield_swap_v3.aleo. Since #110 moved this SDK toshield_swap.aleo, that host serves pools which do not exist on the program the SDK reads and proves against — discovery returned keys whose every chain read came backnull, surfacing as "pool does not exist" rather than as a misconfigured host.Measured, not inferred:
api.swap.shield.fishield_swap.aleoapi.testnet.swap.shield.fishield_swap.aleoamm-api.dev.provable.com— the shipped defaultshield_swap_v3.aleoamm-api-staging.dev.provable.com— the default in 3 test suitesThe API is deployed per-network on separate domains, so one constant cannot be right for both.
shieldSwapActionsnow derives the host from the client's network, resolved per request soswitchChainre-targets the API rather than leaving it on the network the client started from — the same defect class as the prover URL fixed in #117.SHIELD_SWAP_API_URLSanddefaultApiUrl()are exported for directApiClientconstruction;DEFAULT_API_URLis deprecated.This also explains a long-standing mystery. The DEX integration suites weren't failing because of an outage or drift — they defaulted to a decommissioned staging host. With correct hosts and no env override, 35 previously-red live tests pass, including the route-quote test recorded as known-red.
2.
pickInsertHintreturned hints the contract rejectsIt read only
slot.next_init_below/next_init_above, which bracket the pool's current tick rather than the target. Any position bound further out than one initialized tick got a hint above itself, which the contract rejects on finalize — mined, reverted, fee consumed.On the live ETH/USDCx pool at tick
-200996, a lower bound of-203230returned-200996; the true predecessor is-273894. The docblock carried this as a known limitation with an exact walk as a follow-up — this is that follow-up. Live pools hold 3–18 initialized ticks, so the walk is cheap and bounded.Also exports
MIN_TICK_SENTINEL/MAX_TICK_SENTINEL. The list is anchored one step outside the usable range (∓400_001againstMIN_TICK/MAX_TICKof∓400_000), and with no constant for it callers hardcoded-400001— asdevnodeLifecycledoes, which only holds while a pool's tick list is empty. Getting this wrong is how the first version of this patch silently returned an uninitialized tick.Verified against every live testnet pool: 30 hints across 5 pools, each confirmed initialized, strictly below its target, with its successor at or beyond it.
3. Live coverage for the swap and liquidity journeys
Two gated suites for paths that had none. Everything is discovered — pools from the API, balances via
getBalances, tick spacing from the fee tier on chain, ranges from the active tick, hints frompickInsertHint. NocreatePool:pool_creation_openreads false on this deployment, so a suite that created its own pool would only ever skip.liveSwaps.e2e.test.ts— 3/4 passingliveLiquidity.e2e.test.ts— 8/8 passingDeposit amounts come from
amountsForLiquidityat the current price rather than fixed figures, which balance for one pool's price and revert elsewhere.Three timing behaviours needed handling, each confirmed by a real failure first: finalize writes lag the confirmed transaction, so
swap_outputsreads and claims poll; the record scanner indexes a minted position NFT after the mint confirms, so dependent steps wait for it; and multi-hop exceeds the default 5-minute confirmation window.Also exposes
confirmationTimeoutoncreateAleoClient, whichcreateProvingConfigaccepted but the factory did not forward.Findings this surfaced — not fixed here
Concurrent swaps from one account revert on finalize, even with disjoint input tokens in different pools. Likely a shared structure — fee record, nonce, or merkle state — rather than token records. I'd treat this as release-relevant and worth its own investigation.
confirmationTimeoutis not fully plumbed. The suite sets 900s, yetdecreasestill failed at 300s while the followingcollectsucceeded — so the transaction landed and something on that path ignores the configured window.Minting into the native-credits pool reverts where the wrapped-token pool succeeds. The suite prefers the deepest pool, which sidesteps it; the ETH/ALEO LP path likely needs an explicit
TokenRoute. Same shape as the API router refusing to quote ALEO pairs at all.Verification
Typecheck 0 across the workspace, dApp and
loyalty-nodeclean, 1317 unit tests, type-level 15/15, and 35 live DEX tests green with no env override.One caveat: the two live suites were iterated against live state, so they pass in the configuration left here but have not had a clean single end-to-end run. Worth one confirming pass before relying on them as a release gate. Both are gated behind
VEIL_INTEGRATION=1plus a funded key, so they never run in CI.