Skip to content

[simplex]: refuse decimals the asset registry contradicts, remove overfill protection - #1414

Merged
seunlanlege merged 6 commits into
fix/simplex-filler-guardsfrom
fix/simplex-decimals-overfill-cap
Oct 21, 2026
Merged

seunlanlege merged 6 commits into
fix/simplex-filler-guardsfrom
fix/simplex-decimals-overfill-cap

Conversation

@seunlanlege

@seunlanlege seunlanlege commented Oct 16, 2026 •

Copy link
Copy Markdown
Member

Stacked on #1410 (stack #1417). Review only this PR's commits.

Merge this together with #1421. It drops the settlement hold release because #1421 removes limit-order holds. On its own, this PR's settleFilledLimitOrder no longer gives holds back when settlement fails before it commits, which would leave a limit order's reserved inflated.

This PR fixes c18 (high, decimals half) from the 2026-10-15 simplex audit, and removes the overfill protection machinery.

Decimals cross-check (ContractInteractionService.getTokenDecimals)

  • Why: getTokenDecimals read decimals() once, from one endpoint of the plain fallback client, and cached it for the life of the process. The asset registry was only consulted when that read failed. One RPC answering 18 for a 6-decimal USDC inflated a bid by 10^12.
  • Fix: a successful read that disagrees with the asset registry is logged, refused (… refusing to price it) and not cached, so that order is skipped. The check runs outside the read's error handler, so the registry fallback can't swallow it. Tokens the registry doesn't know are cached as before, and a failed read still falls back to the registry.
  • Scope: this covers every token in the asset registry (USDC, USDT, cNGN and the other built-ins). A token the registry doesn't know, such as a custom [assets] entry, still takes the chain's answer.

Overfill protection removed

  • Why: the clamp was disabled in [simplex]: Raise default maxOverfillBps to 5% and bump patch version #964 and dropped from the docs in f0aac53, but the machinery stayed. This removes it.
  • Config: [simplex.overfillProtection] is no longer read, and a config that still sets it is ignored with a warning rather than refused. simplex init wrote the section uncommented for existing operators, and refusing it would crash-loop their deployments. validateConfig accepts the section in any form. Boot logs [simplex.overfillProtection] is ignored: overfill protection was removed … when simplex.overfillProtection !== undefined, so a library caller passing { overfillProtection: undefined } is not warned.
  • simplex init: an update run does not write the section into the new file, and logs Dropped [simplex.overfillProtection] once that file is written. "Start the filler with it as-is" leaves the file untouched, so boot's warning is what the operator sees.
  • Removed public API:
    • Library: Simplex.isHalted(), Simplex.resetHalt(), SimplexStatus.halted, FillerConfigService.getMaxOverfillBps() / getMaxConsecutiveClamps(), FXFiller.isHalted() / resetHalt(), HaltControl and haltControls.
    • HTTP: POST /api/reset-halt (now a 404), and halted in /api/status.
    • Dashboard: the halt banner and "Reset halt".
    • Init wizard: the "Overfill protection" step.
  • Docs: the SDK and HTTP API references no longer list any of the removed items.
  • Bidding is unchanged: a bid still carries the limit order's whole offer.

Settlement hold release removed (IntentFiller.settleFilledLimitOrder): the code that returned a claim's holds when settlement threw before its transaction committed is gone, because limit-order holds are being removed in #1421. This is why the two PRs merge together.

Validation

Run in sdk/packages/simplex at the PR head:

  • Touched tests: write-step (2) and update-run-preservation (14) passed; the decimals, filler-toml-validate and boot-bundler-preflight suites are in the runs below.
  • CI's test job commands: simplex lint exit 0 (warnings only); test:data 146 passed; test:unit 267 passed; test:filler 70 passed, 5 skipped.
  • Core, data, orderbook and services suites (vitest --maxConcurrency=1 src/tests/core src/tests/data src/tests/orderbook src/tests/services): 745 passed, 4 skipped.
  • Typecheck: tsc shows the 14 errors already on main, none in changed files.
  • Tests cover:
    • a mismatching decimals read throws and is re-read, not cached; an agreeing read and an unknown token are both cached;
    • validateConfig accepts {}, { maxOverfillBps }, { maxConsecutiveClamps } and the example TOML with the section appended;
    • boot warns about a leftover section and not about undefined;
    • an update run drops the section from the written file and logs it only after the write, and logs nothing when the old file had none;
    • status has no halted, and /api/reset-halt is a 404.

Not covered: no mainnet run.

@seunlanlege
seunlanlege added this pull request to stack #1417 October 16, 2026 11:32
@seunlanlege seunlanlege changed the title [simplex]: refuse decimals the asset registry contradicts, cap a bid at the ask plus maxOverfillBps [simplex]: refuse decimals the asset registry contradicts, return holds on a failed settlement, remove overfill protection Oct 16, 2026
@seunlanlege seunlanlege changed the title [simplex]: refuse decimals the asset registry contradicts, return holds on a failed settlement, remove overfill protection [simplex]: refuse decimals the asset registry contradicts, remove overfill protection Oct 16, 2026

@Wizdave97 Wizdave97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Verdict: changes needed before merge. The decimals cross-check is correct; the overfill removal needs one change.

The decimals check:

  • The lookup is by chain plus lowercased address, not by symbol.
  • The check runs after the try/catch, so the registry fallback can't swallow it.
  • Every caller turns the throw into "skip this order".
  • All 38 mainnet registry entries, plus BSC Chapel and Amoy, match on-chain decimals().
  • One unrelated registry bug: Gnosis Chiado USDC is registered as 6 decimals but returns 18 on chain. Simplex doesn't use that chain.

Must fix

  1. Rejecting [simplex.overfillProtection] stops existing configs from booting (filler-toml.ts:308).
    • On main, simplex init → Fine-tune → "Overfill protection" writes the section uncommented (emit-toml.ts:111-117 on main). That has shipped since #1051, and carryPrefillExtras keeps the section on later init runs.
    • After upgrading, those operators' simplex run throws, and under Docker's --restart unless-stopped it crash-loops.
    • simplex init can't migrate the file either: it rejects the file and only offers "Start fresh?", which loses the prefilled answers.
    • Through the desktop app the reason isn't shown at all. The error happens before the UI socket opens, so the user only sees "exited before its UI became ready".
    • The setting has done nothing since #964, so rejecting it adds no safety.
    • Suggestion: warn and ignore it at boot, as is done for [[pairs]], and delete the key when init loads the file, so emitFillerToml drops it on the next write.
    • The in check also trips on { overfillProtection: undefined } from library callers; !== undefined is safer.

Should fix

  1. A stale safety comment (ContractInteractionService.ts:302-303).
    • It says a token the registry doesn't know is safe because "FXFiller bounds its bid against the order's own output". That bound was removed in 27d82a4, and targetOutput now has no ceiling against output.amount.
    • So a custom [assets] token whose RPC answers 18 decimals for a 6-decimal token still bids 10¹² too much, limited only by wallet balance and funding venues.
    • The PR description is honest about this scope. Please correct the comment, and consider cross-checking [assets] entries that declare decimals.

Notes:

  • A token that fails the check is re-read on every evaluation: about 2–4 eth_calls plus an error log line per order. That's acceptable; a short negative cache would only cut the log noise.
  • getFeeTokenWithDecimals still caches its decimals without the check. The effect is limited to profit ranking and logs, because the cost and spread gates compare raw amounts.
  • The release should be a minor version bump that calls out the config change and the removed library and HTTP API.

Cross-PR: 7da2f5b removes the settlement hold give-back on the premise that #1421 removes holds. The new decimals throw is one more way settleFilledLimitOrder can fail after claimExecutedBid and leave reserved inflated. This PR shouldn't ship without #1421.

CI: the failing live job is unrelated. In the SDK, intentGatewayCancel.test.ts hit "insufficient funds for gas" because the test wallet ran dry, which #1420 addresses.

seunlanlege added a commit that referenced this pull request Oct 19, 2026
- A config that still carries [simplex.overfillProtection] is accepted, not
  rejected: validateConfig no longer throws, boot logs a warning that the
  section is ignored, and simplex init drops the key (assembleConfig also
  clears it) so the next write omits it.
- Correct the getTokenDecimals comment that claimed FXFiller bounds a bid
  against the order's output; [assets] entries declare no decimals, so an
  unknown token is limited only by the wallet balance and funding venues.
- Revise the note to match.
@seunlanlege
seunlanlege force-pushed the fix/simplex-decimals-overfill-cap branch from 7da2f5b to 58eff72 Compare October 19, 2026 12:28
seunlanlege added a commit that referenced this pull request Oct 19, 2026
- A config that still carries [simplex.overfillProtection] is accepted, not
  rejected: validateConfig no longer throws, boot logs a warning that the
  section is ignored, and simplex init drops the key (assembleConfig also
  clears it) so the next write omits it.
- Correct the getTokenDecimals comment that claimed FXFiller bounds a bid
  against the order's output; [assets] entries declare no decimals, so an
  unknown token is limited only by the wallet balance and funding venues.
- Revise the note to match.
@seunlanlege
seunlanlege force-pushed the fix/simplex-decimals-overfill-cap branch from 58eff72 to 80b6ff0 Compare October 19, 2026 13:26
seunlanlege added a commit that referenced this pull request Oct 19, 2026
- A config that still carries [simplex.overfillProtection] is accepted, not
  rejected: validateConfig no longer throws, boot logs a warning that the
  section is ignored, and simplex init drops the key (assembleConfig also
  clears it) so the next write omits it.
- Correct the getTokenDecimals comment that claimed FXFiller bounds a bid
  against the order's output; [assets] entries declare no decimals, so an
  unknown token is limited only by the wallet balance and funding venues.
- Revise the note to match.
@seunlanlege
seunlanlege force-pushed the fix/simplex-decimals-overfill-cap branch from 80b6ff0 to 789296b Compare October 19, 2026 15:08

@Wizdave97 Wizdave97 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approved. Old configs that still contain [simplex.overfillProtection] now boot from every entry point, with one warning. simplex init and the dashboard rewrite both drop the key, and the stale comment is fixed.

Non-blocking follow-ups:

  • PR description and commit title. The description still says the section "is rejected by validateConfig" and lists rejection tests. The title of commit 3ec6f5e4e says the same. Worth fixing before the squash.
  • Premature log line. simplex init logs "Dropped [simplex.overfillProtection]…" (cli/init/index.ts:108) before the operator has picked an action. If they choose "Start the filler with it as-is", nothing is actually dropped.
  • Merge order. Merge this together with #1421, because this PR drops the settlement hold release on the basis that #1421 removes holds.

seunlanlege added a commit that referenced this pull request Oct 20, 2026
- A config that still carries [simplex.overfillProtection] is accepted, not
  rejected: validateConfig no longer throws, boot logs a warning that the
  section is ignored, and simplex init drops the key (assembleConfig also
  clears it) so the next write omits it.
- Correct the getTokenDecimals comment that claimed FXFiller bounds a bid
  against the order's output; [assets] entries declare no decimals, so an
  unknown token is limited only by the wallet balance and funding venues.
- Revise the note to match.
seunlanlege added a commit that referenced this pull request Oct 20, 2026
@seunlanlege
seunlanlege force-pushed the fix/simplex-decimals-overfill-cap branch from 789296b to e940553 Compare October 20, 2026 11:46
- A config that still carries [simplex.overfillProtection] is accepted, not
  rejected: validateConfig no longer throws, boot logs a warning that the
  section is ignored, and simplex init drops the key (assembleConfig also
  clears it) so the next write omits it.
- Correct the getTokenDecimals comment that claimed FXFiller bounds a bid
  against the order's output; [assets] entries declare no decimals, so an
  unknown token is limited only by the wallet balance and funding venues.
- Revise the note to match.
@seunlanlege
seunlanlege force-pushed the fix/simplex-decimals-overfill-cap branch from e940553 to 2b095cb Compare October 20, 2026 12:30
@seunlanlege
seunlanlege merged commit b5c6228 into main Oct 21, 2026
15 of 16 checks passed
@seunlanlege
seunlanlege deleted the fix/simplex-decimals-overfill-cap branch October 21, 2026 09:52
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