Skip to content

chore(uniswapx-sdk): add ABI provenance manifest + deterministic refresh tooling - #649

Open
claude[bot] wants to merge 2 commits into
mainfrom
claude/uniswapx-sdk-drift-prevention
Open

chore(uniswapx-sdk): add ABI provenance manifest + deterministic refresh tooling#649
claude[bot] wants to merge 2 commits into
mainfrom
claude/uniswapx-sdk-drift-prevention

Conversation

@claude

@claude claude Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Requested by Bruno Nunes · Slack thread

PR Scope

chore(uniswapx-sdk): — internal tooling/docs change, no runtime behavior change and no release (empty changeset included). No abis/*.json or src/contracts files are modified.

Description

The JSON files in sdks/uniswapx-sdk/abis are compiled contract artifacts (and a few hand-authored ABIs) copied from upstream contract repos — mainly Uniswap/UniswapX, plus Uniswap/permit2 and a handful of third-party/interface ABIs. The typechain build step turns them into the ethers bindings committed under src/contracts, which are then compiled into dist. abis/ itself is a build-time input only (not in "files", so never shipped directly).

Before: the ABIs were hand-copied with no record of which upstream commit each came from and no refresh path. Investigating this package confirmed the smell:

  • The artifacts span solc 0.8.160.8.30, come from at least two repos, and mix Foundry and Hardhat artifact formats — clear evidence of piecemeal manual copying over time. Their in-repo git history is a single squashed migration commit, so the source commits are unrecoverable.
  • Nothing re-derives them, so an upstream reactor/resolver/hook can change its ABI while the copy here keeps describing the old interface and the generated bindings keep encoding/decoding against the stale shape — silently.
  • There is no drift check and no way to add the strong one v2-sdk/v3-sdk use (recompute a hash from a published @uniswap/*-core npm artifact and assert equality), because no UniswapX contract artifact is published to npm to diff against (verified: @uniswap/uniswapx, @uniswap/uniswapx-contracts, etc. all 404).

This is the same mechanism as the liquidity-launcher stale-bytecode incident fixed in #648 — a hand-copied upstream artifact pinned to nothing, with no refresh path. The blast radius here is smaller (the embedded bytecode only deploys mocks in integration tests; the ABIs drive typed encode/decode rather than a CREATE2 prediction holding user funds), but the treatment is the same: record provenance + make refresh a single deterministic command.

After:

  • abis/PROVENANCE.md — records, for every artifact, its upstream repo, source path, contract, solc version, and whether it is script-refreshed or hand-maintained; explains the drift risk and links the chore(liquidity-launcher-sdk): add deterministic lock-bytecode regeneration tooling #648 incident. Also documents a pre-existing orphan (see Follow Ups).
  • scripts/abis.manifest.json — machine-readable version of the same, driving the script (15 scripted foundry artifacts from UniswapX/permit2; 7 scripted: false third-party/interface/hand-authored ABIs left untouched).
  • scripts/regenerate-abis.ts — a deterministic, one-command regenerator. Given local checkouts (UNISWAPX_REPO / PERMIT2_REPO, optional *_COMMIT pins), it checks out the commit, initializes submodules, runs forge build, copies the mapped out/<Contract>.sol/<Contract>.json artifacts into abis/, regenerates src/contracts via typechain, and records the resolved commit back into the manifest and PROVENANCE.md. It validates every artifact before writing anything, fails loudly and writes nothing partial if forge is missing / a build fails / an artifact is absent, and exposes escape hatches (FORGE_BIN, SKIP_CHECKOUT, SKIP_SUBMODULES, SKIP_BUILD). It uses only Node builtins — no new dependency.
  • package.jsonregenerate:abis script alias.
  • README.md — a "Maintaining the contract ABIs" section: what the ABIs are, why they silently go stale (with chore(liquidity-launcher-sdk): add deterministic lock-bytecode regeneration tooling #648 as motivation), and the exact refresh command.

No ABI bytes and no generated bindings change in this PR; the tooling is for future refreshes.

How Has This Been Tested?

  • End-to-end script run in SKIP_BUILD mode against synthesized out/ trees reconstructed from the current artifacts: the script validated and refreshed all 15 scripted artifacts, then regenerated src/contracts via typechain with zero diff — typechain is idempotent against the committed ABIs. The only artifact diffs were formatting-only (verified: abi and bytecode.object byte-identical to HEAD for all 15); those rewrites were then reverted so this PR leaves abis/ untouched.
  • Fail-loud paths confirmed to exit non-zero and write nothing: no repo path provided, missing out/ artifact, and a deployable contract with empty bytecode.
  • bun x tsc -p tsconfig.base.json --noEmit — clean (after building the @uniswap/permit2-sdk workspace dep).
  • bun run lint — clean.
  • bun test src/ — 328 pass / 0 fail.

Are there any breaking changes?

No. No public API, types, ABIs, or generated bindings change; dist output is unaffected (only new tooling/docs and a README/package.json edit).

(Optional) Follow Ups

Not in this PR:

  • Stronger, automatic drift detection is only possible once UniswapX publishes versioned contract artifacts (an npm package or a versioned release bundle of out/*.json). Once that exists, this SDK could adopt the same real drift-check v2-sdk/v3-sdk already use — recompute a hash from the published artifact and assert equality, so a changed upstream ABI fails CI on bun install. This manifest + script is the interim, deterministic bridge. Worth filing an issue on Uniswap/UniswapX to publish those artifacts.
  • Orphaned generated bindings: src/contracts contains DutchOrderReactor.ts and DutchLimitOrderReactor.ts (plus their factories) that have no backing ABI in abis/ — leftovers from ABIs removed without a clean regenerate (typechain does not delete). A clean rm -rf src/contracts && typechain reproduces all 47 other typings byte-for-byte but drops these 4, and EventWatcher.ts still imports the FillEvent type from ../contracts/DutchOrderReactor, so removing them requires a small code change. Left as a follow-up; the refresh script is additive (like the build's own typechain) and does not touch them.
  • abis/ formatting is inconsistent (12 of 22 files don't round-trip through a single JSON pretty-print), a side effect of piecemeal hand-copying. The first forge-backed run of the new script will normalize this — formatting-only, no semantic change — and is best done as its own reviewed refresh commit.

Generated by Claude Code

…esh tooling

The files in abis/ are compiled contract artifacts hand-copied from upstream
(mainly Uniswap/UniswapX, plus Uniswap/permit2 and a few third-party ABIs) and
turned into the src/contracts ethers bindings by typechain at build time. Nothing
recorded which upstream commit each artifact came from and nothing re-derives them,
so they can silently drift out of sync with the deployed contracts. This is the same
class of problem as the liquidity-launcher stale-bytecode incident (#648),
with a smaller blast radius (the embedded bytecode only deploys test mocks).

- abis/PROVENANCE.md: records the upstream repo, source path, contract, and solc
  version of every artifact, and which are script-refreshed vs hand-maintained.
- scripts/abis.manifest.json: machine-readable version of the same, driving the script.
- scripts/regenerate-abis.ts: one-command deterministic refresh of the UniswapX/permit2
  artifacts from local checkouts (forge build -> copy mapped out/ artifacts -> typechain),
  fails loudly and writes nothing partial, with FORGE_BIN/SKIP_* escape hatches.
- package.json: regenerate:abis script alias.
- README: "Maintaining the contract ABIs" section documenting the drift risk and refresh.

No runtime behavior change: no abis/*.json or src/contracts files are modified, and the
abis are build-time inputs not shipped in the published package.
@claude
claude Bot requested review from a team as code owners July 17, 2026 02:56
@github-actions

github-actions Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

● Reviewed · against 54ebd86 · 2026-07-17 03:02 UTC · 2 reviews · view run ↗

Note

Approved.

Adds an ABI provenance manifest, a PROVENANCE.md, and a deterministic regenerate-abis.ts regenerator for the uniswapx-sdk contract ABIs. No ABI bytes, generated bindings, or runtime behavior change; the changeset is empty.

Assessment

Build-time maintainer tooling with no runtime surface. All subprocess calls use execFileSync with array-form argv, so operator-supplied refs and paths can't inject a shell; every input originates from the operator's own environment or the in-repo manifest. The script validates all artifacts before writing anything and fails loud on missing or malformed inputs, so a partial refresh can't leave abis/ half-written.

Iteration history · 2 reviews
2026-07-17 03:02 UTC · ✅ approved · 0 findings · 54ebd86 · run ↗

(no findings)

2026-07-17 02:58 UTC · ✅ approved · 1 finding · bf7f12f · run ↗
  • sdks/uniswapx-sdk/scripts/regenerate-abis.ts:134 — info · correctness

Tip

Teach the reviewer. React 👍 on findings that helped, 👎 on false positives. Reply to push back or add context — we aggregate this weekly to tune the bot.

Comment @request-claude-review to re-run.

skippedRepos.push(`${key} (set ${spec.envVar} to refresh its ${artifacts.length} artifact(s))`)
continue
}
const commit = process.env[`${key.toUpperCase()}_COMMIT`]

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.

🔵 info · correctness — the commit env var is derived from the manifest key (${key.toUpperCase()}_COMMIT) while the repo path comes from spec.envVar. These match today only because uniswapx/permit2 happen to align with the *_REPO prefixes. A future entry whose key differs from its envVar prefix would silently ignore the documented <REPO>_COMMIT pin. Derive both from spec.envVar (swap the _REPO suffix) to keep them in lockstep.

@github-actions github-actions Bot 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.

Note

Approved — see full review in the sticky comment ↑

@graphite-app
graphite-app Bot requested a review from a team July 17, 2026 02:59
@graphite-app

graphite-app Bot commented Jul 17, 2026

Copy link
Copy Markdown

Graphite Automations

"Request reviewers once CI passes on sdks monorepo" took an action on this PR • (07/17/26)

2 reviewers were added to this PR based on Siyu Jiang (See-You John)'s automation.

Derive both the repo-path var and the per-entry <REPO>_COMMIT var from a
single source (spec.envVar) so they cannot drift for a future manifest
entry whose key differs from its envVar prefix. Behavior is unchanged for
the current uniswapx/permit2 entries.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hkw9wpfbpaigd258uUmkYr
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.

1 participant