Skip to content

fix(trade): screen addresses for sanctions before quoting and executing swaps - #664

Merged
gulshngill merged 2 commits into
mainfrom
fix/trade-sanctions-screening
Sep 21, 2026
Merged

gulshngill merged 2 commits into
mainfrom
fix/trade-sanctions-screening

Conversation

@gulshngill

Copy link
Copy Markdown
Contributor

Summary

nansen trade quote and nansen trade execute now run the same pre-trade compliance screening that nansen bridge and nansen perp already perform, so DEX swaps are no longer the one trading surface in the CLI without it — for same-chain and cross-chain (--to-chain) swaps alike.

Both commands reuse the existing fail-closed screenOrThrow() helper (POST /api/v1/sanctions/screen), exactly as bridge execute does:

  • trade quote screens the signing wallet plus any distinct destination wallet (--to-wallet, or the destination auto-derived for a cross-chain swap) before the quote request goes out, so a flagged wallet never receives a signable quote.
  • trade execute screens once, immediately before the per-quote signing loop — covering the resolved signing wallet, the wallet the quote was built for, and any distinct recipient. A quote stays valid for up to an hour and the signed transaction is broadcast with no further check, so this is the last gate before funds move. Screening outside the loop means a refusal is final rather than "try the next quote".

A flagged address aborts with code SANCTIONED; a screening call that cannot complete (or a response that doesn't cover every requested address) aborts with SCREENING_UNAVAILABLE. In both cases nothing is signed or broadcast and the quote is left unconsumed, so a fresh attempt is still possible once the situation is resolved.

Two smaller behaviour notes:

  • A local wallet file with no key for the quote's chain now fails with the same actionable "cannot confirm the quote was built for this wallet" message the signing branch gives, instead of proceeding to a screen it cannot satisfy.
  • A quote that records no wallet at all (only reachable from a hand-edited or truncated quote file) is refused with SCREENING_UNAVAILABLE and a prompt to re-quote, rather than silently skipping the check.

Screening requires API access — the same requirement bridge and perp already have — so an unauthenticated call fails closed rather than trading unscreened.

No new commands or flags, so src/schema.json is unchanged.

Changes

  • src/trading.js — import screenOrThrow; new exported tradeScreeningAddresses() helper (drops blanks, de-duplicates EVM addresses case-insensitively while keeping base58 addresses distinct by exact spelling); screen in the quote handler before the quote request; resolve the signer ahead of the signing loop and screen in the execute handler before it.
  • src/__tests__/trading-sanctions-screening.test.js — new: the gate for both commands across local, WalletConnect and provider-signed paths, plus the address-list helper.
  • src/__tests__/trading.test.js, src/__tests__/trading-quote-reuse.test.js, src/__tests__/trade-validation.test.js, src/__tests__/blank-option-values.test.js — existing trade tests pass an API double whose screen reports clean (they previously passed null, since these handlers made no API-instance calls).
  • skills/nansen-trading/SKILL.md — document the screen and its two error codes under Notes; note that the API key is needed for it.
  • .changeset/trade-sanctions-screening.md — patch.

Validation

npm test
Test Files  77 passed (77)
     Tests  3215 passed | 2 skipped (3217)
  Duration  144.72s
npm run lint

Clean, no output.

All network calls in the new tests are mocked (API instance double + stubbed fetch for the trading backend and RPC); no real trade is quoted or executed. The tests assert that the screen call precedes the /quote and /execute calls, that a flagged signer or destination aborts with no transaction sent and the quote left unconsumed, and that a screening failure or an incomplete screening response aborts the same way.

Checklist

  • Tests pass (npm test)
  • src/schema.json updated if new commands or flags were added — n/a, no new commands or flags
  • README.md updated if new top-level commands or categories were added — n/a, no new commands
  • Changeset added (.changeset/) — only required for user-facing changes (new commands, flags, bug fixes, breaking changes)

🤖 Generated with Claude Code

…ng swaps

`trade quote` and `trade execute` talked to the trading backend directly and
never ran the pre-trade compliance screen that `bridge` and `perp` already
perform client-side, leaving DEX swaps (same-chain and cross-chain) as the one
trading surface with no such check.

Both handlers now call the same fail-closed `screenOrThrow()` helper:

- `quote` screens the signing wallet plus any distinct destination wallet
  (`--to-wallet`, or the auto-derived cross-chain destination) before the quote
  request goes out, so a flagged wallet never receives a signable quote.
- `execute` screens once, before the per-quote signing loop, covering the
  resolved signer, the wallet the quote was built for, and any distinct
  recipient — so a refusal is final rather than "try the next quote".

A flagged address aborts with code `SANCTIONED`; a screening call that cannot
complete aborts with `SCREENING_UNAVAILABLE`. Nothing is signed or broadcast in
either case, and the quote is left unconsumed. A local wallet file with no key
for the quote's chain now fails with the same actionable message the signing
branch gives instead of reaching the screen at all.

Adds trading-sanctions-screening.test.js covering both commands across local,
WalletConnect and provider-signed paths; existing trade tests pass an API double
whose screen reports clean. All network calls are mocked.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@nansen-pr-reviewer

nansen-pr-reviewer Bot commented Sep 19, 2026 •

Copy link
Copy Markdown

pr-reviewer Summary for #f409934

✅ No issues found

The code review completed successfully with no findings.

Review effort: 3/5 (Moderate)

Summary

This PR correctly extends the existing fail-closed compliance screening (screenOrThrow) to trade quote and trade execute, closing the last unscreened trading surface in the CLI. The implementation is consistent with how bridge and perp already work, and the tightening of screenOrThrow itself (strict === true boolean check, case-sensitive base58 matching, boolean-verdict gate for the coverage check) is sound and well-motivated.

All three signer paths (local, WalletConnect, Privy) are handled correctly: local and WalletConnect resolve signerAddress before the screen; Privy correctly falls back to the quote's stored walletAddress since its address is fetched from the provider inside the signing loop. The empty-address guard (screenAddresses.length === 0) catches the hand-edited/truncated-quote edge case before the API call is made. The tradeScreeningAddresses deduplication helper correctly distinguishes EVM (case-insensitive) from base58/Solana (case-sensitive) addresses.

Test coverage is thorough: the new 417-line trading-sanctions-screening.test.js pins the pre-trade ordering, all four error paths (SANCTIONED, SCREENING_UNAVAILABLE via network error, empty response, incomplete response), all three signer types, cross-chain destination screening, and the Privy no-wallet edge case — with every network call mocked. The screenApi double added to four existing test files is straightforwardly correct. The changeset is present with the right package name and patch semver bump.


Token usage: 3,074 input, 3,295 output, 319,594 cache read, 46,272 cache write | Usage Guide

New pushes are reviewed automatically with a 10-minute cooldown between reviews. To request a review at any time, comment @nansen-pr-reviewer re-review.

@gulshngill

Copy link
Copy Markdown
Contributor Author

Merge position: 6 of 9. Merge before #667.

Both this and #667 change src/trading.js and skills/nansen-trading/SKILL.md. This one inserts the pre-trade check in the quote and execute paths; #667 adds a confirmation gate slightly later in the same execute path. Landing screening first means #667 rebases onto the final ordering rather than the reverse.

@gulshngill
gulshngill requested a review from kome12 September 21, 2026 07:33
@gulshngill
gulshngill merged commit d034ae1 into main Sep 21, 2026
9 checks passed
@gulshngill
gulshngill deleted the fix/trade-sanctions-screening branch September 21, 2026 13:54
@github-actions github-actions Bot mentioned this pull request Sep 21, 2026
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