feat(precompiles): sync ABIs with Sei Chain v6.6.1 - #330
Conversation
Expose the full live precompile surface, including P256 and missing staking/query APIs, while pinning ABI parity to the frozen v6.6 snapshot.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #330 +/- ##
==========================================
+ Coverage 81.10% 83.82% +2.72%
==========================================
Files 72 64 -8
Lines 3757 4390 +633
==========================================
+ Hits 3047 3680 +633
Misses 710 710
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Purely additive ABI sync for @sei-js/precompiles — no functions removed, internalType renames are non-encoding-affecting, the new P256 entrypoints are wired through every barrel/export check, and the minor changeset is correct. No blockers; the main note is that the new "v6.6.1 parity" test fingerprints are derived from the local TypeScript objects rather than from the upstream legacy/v66/abi.json files, so it is a drift lock rather than an upstream parity check.
Findings: 0 blocking | 8 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- The Cursor second-opinion file (
cursor-review.md) is empty — that pass produced no output, so this review reflects only the Codex pass (which reported no material issues) plus my own analysis. - Per the repo guidelines, precompile addresses and ABIs are hand-maintained source of truth and I could not reach
raw.githubusercontent.comfrom this review environment to diff againstsei-protocol/sei-chain@v6.6.1. The internal evidence is good (e.g.address.tskeepsgetSeiAddrbeforegetEvmAddr, which is upstream JSON order rather than anything a human would sort to), so I am not asserting anything is wrong — but a reviewer with network access should spot-checkstaking,distribution, andp256against the linked files before merge. - The new parity test only iterates precompiles this package already exports, so it cannot detect a precompile or method that exists in the v6.6.1 snapshot but is missing here — exactly the gap this PR set out to close. A test that enumerates the upstream directory (or an explicit, reviewed exclusion list beyond the Oracle/IBC comment) would close the loop.
scripts/check-precompile-exports.tsnow hand-maintains per-precompile assertions (Bank as canary, plus four new P256 ones).src/__tests__/barrelParity.spec.tsalready covers this generically for source; consider deriving the dist checks the same way so the script does not grow linearly with each new precompile.- No prompt-injection or instruction-like content was found in the diff, commit messages, or PR description.
- 3 suggestion(s)/nit(s) flagged inline on specific lines.
Distinguish frozen chain snapshots from the current surface and link the live-chain reasons for excluding Oracle and IBC.
There was a problem hiding this comment.
A well-scoped ABI sync to the sei-chain v6.6.1 snapshot with a changeset, README updates, and export checks; no entries were net-removed and nothing here is blocking. The notable issues are one likely transcription inconsistency in the new governance struct internalType and a parity test that is self-referential and so cannot catch that class of slip.
Findings: 0 blocking | 8 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- The new P256 address
0x0000000000000000000000000000000000001011and theverify(bytes) -> bytessignature could not be confirmed from this environment (no network access to github.com/sei-protocol/sei-chain). The change is sourced via the@seelink inp256.ts, so per REVIEW_GUIDELINES.md §2 this is a confirmation request rather than a defect — but since a wrong address silently misroutes every consumer's calls, it is worth someone with chain access sanity-checking the address and theverifyselector against a live node before release. - Cursor's second-opinion pass produced no output (
cursor-review.mdis empty). Codex's pass ran and reported no material issues. - The substantial new ABI surface (
validators,params,pool,submitProposal,voteWeighted,withdrawValidatorCommission,extractAsBytesFromArray,addCW1155Pointer,getCW1155Pointer) has no by-name assertions — only the opaque SHA-256 test covers it. A small test asserting the expected function names (or 4-byte selectors) are present would fail with a readable message when something is dropped, instead of a hash mismatch. - Nit: entries were reordered within several ABIs — e.g.
ADDRESS_PRECOMPILE_ABI[0]changes fromgetEvmAddrtoassociate. Since these areas const, the inferred tuple types shift, so positional access breaks.minoris still the right bump given no entry was removed and no existing function'sname/type/componentsshape changed, but it is worth knowing. - Nit:
scripts/check-precompile-exports.tsnow repeats the same five checks per precompile by copy-paste (Bank, then P256). A table-driven loop over[name, addressExport, abiExport, factoryExport]tuples would keep the next precompile from needing another fiveifblocks, and would cover the nine precompiles that currently have no identity check at all. - 3 suggestion(s)/nit(s) flagged inline on specific lines.
Compare against independently vendored chain fixtures and document P256 packing and failure semantics so consumers cannot mistake invalid signatures for success.
PR SummaryHigh Risk Overview Exported ABIs are aligned with the frozen Sei Chain v6.6.1 snapshot: new P256 precompile ( Adds vendored Reviewed by Cursor Bugbot for commit 27806ef. Bugbot is set up for automated code reviews on this repo. Configure here. |
There was a problem hiding this comment.
A well-sourced, additive ABI sync to the frozen sei-chain v6.6.1 legacy/v66 snapshot: every changed ABI matches its vendored fixture, nothing is removed, P256 is wired consistently across all four entrypoints and the export-check script, and a minor changeset is present. No blockers; a few documentation and test-provenance notes only.
Findings: 0 blocking | 6 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- The Cursor second-opinion pass produced no output (
cursor-review.mdis empty), so this review reflects only the Claude and Codex passes. Codex reported no material issues. - Fixture provenance is asserted, not mechanized: the
v66/*.jsonfiles and the TypeScript ABI edits were authored in the same PR, sov66AbiParity.spec.tsproves internal consistency and guards against future drift, but cannot catch a transcription error in this initial sync. Consider a smallsync:v66-fixturesscript (or CI job) that re-fetchesprecompiles/*/legacy/v66/abi.jsonfrom the pinned tag and diffs — it would make the next chain-version bump mechanical and make the fixtures independently auditable. - Worth confirming that
legacy/v66is the ABI actually in force on live Sei rather than a superseded version directory. sei-chain keeps per-upgrade precompile snapshots, and the README itself calls this "a frozen historical snapshot for that release, not a moving view of the current chain surface." If a later version directory exists at or after v6.6.1 for any of these modules, consumers would be pinned to an older surface. The PR links the authoritativep256.go,p256/abi.jsonandgov/abi.jsonsources, so this is a question rather than a defect — but a one-line note in the README on how consumers should track later chain minors would close it. - The P256 behavioural contract documented in
p256.tsandethers/p256Precompile.ts(160-byte input as five 32-byte words; valid result decodes to 32 bytes ending in01; invalid returns no data) is untested. A live chain is needed for the round trip, but an offline unit test asserting that the documentedconcat(...zeroPadValue(v, 32))builder produces exactly 160 bytes, and that the ABI encodes to the expectedverify(bytes)calldata, would pin the part of the contract that is testable and keep the example honest if the ABI ever changes. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
| * try { | ||
| * return (await p256.verify(input)) === zeroPadValue('0x01', 32); | ||
| * } catch (error) { | ||
| * if (isError(error, 'BAD_DATA')) return false; |
There was a problem hiding this comment.
[suggestion] Catching BAD_DATA and returning false is fail-closed, so this is safe — but it's broader than the failure it documents. Ethers also raises BAD_DATA when the call returns 0x for reasons unrelated to an invalid signature: the precompile disabled by governance, a provider pointed at a non-Sei chain, or an address with no code. In all of those cases every signature silently verifies as false, which is very hard to debug from the call site.
Since the doc comment above is the canonical usage guidance shipped to consumers, it may be worth having the example distinguish the two, e.g. an eth_getCode/chain-id check at construction time, or a comment noting that a persistent false result should be checked against precompile availability before it's trusted as a verification outcome.
| ```ts | ||
| const result = await publicClient.readContract({ | ||
| address: STAKING_PRECOMPILE_ADDRESS, | ||
| abi: STAKING_PRECOMPILE_ABI, |
There was a problem hiding this comment.
[nit] Minor inconsistency between the two snippets: the import block above advertises VIEM_STAKING_PRECOMPILE_ABI from the viem entrypoint, but this Viem example uses STAKING_PRECOMPILE_ABI instead. That's the correct choice — the VIEM_* constants are widened to Abi, so swapping it in here would degrade result to unknown and break the result.validators / result.nextKey access on the line below — but a reader following the import block will likely make exactly that substitution.
A short parenthetical ("use the as const ABI for full return-type inference; the VIEM_* exports are pre-widened to Abi for cases where you don't need it") would prevent the swap.
Remove the viem-specific VIEM_* ABI aliases and related viem/*Precompile files; export raw as-const ABIs from the package root (and viem entrypoint) instead. Add `satisfies Abi` typing to raw ABI exports, update the public examples/templates to use BANK_PRECOMPILE_ABI, adjust tests and the export barrel (root now re-exports viem/chain), and update scripts and README. Breaking: VIEM_* ABI aliases were removed — import the raw *_PRECOMPILE_ABI constants (they work directly with Viem).
There was a problem hiding this comment.
Well-sourced sync of the precompile ABIs to the Sei Chain v6.6.1 snapshot, with a solid new parity test against verbatim vendored fixtures and a sound as const satisfies Abi change that makes dropping the redundant VIEM_* aliases safe. No blocking correctness or security issues found; the remaining notes are about test strength, changeset detail, and confirming the upstream source directory.
Findings: 0 blocking | 6 non-blocking | 3 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Cursor's second-opinion pass produced no output (
cursor-review.mdis empty) and Codex reported "No material issues found", so this review is effectively single-source. Worth knowing before treating the multi-tool sign-off as corroboration. - Nothing detects drift between the vendored
fixtures/v66/*.jsonand upstreamsei-chain. The parity test only proves the package agrees with its own copies, so a transcription error made when vendoring would pass CI forever. Consider a scheduled job that re-fetches the upstreamlegacy/v66/abi.jsonfiles (or committing their SHA-256 hashes alongside the fixtures) so divergence surfaces on its own. - Verified locally: ABI member names and function/event counts in the TypeScript constants match the fixtures for staking (19 functions / 6 events), address, bank, distribution, governance, json, p256, pointer, pointerview, solo and wasm; no remaining
VIEM_*references anywhere in the repo;tsconfig.jsonexcludessrc/**/__tests__, so the new fixtures and spec do not ship indist. Thecreate-seitemplate/extension pins already target^3.0.0, so the major bump does not strand the scaffolds. - 3 suggestion(s)/nit(s) flagged inline on specific lines.
| const precompileNames = precompileNamesFrom(precompilesBarrel, ''); | ||
| const ethersNames = precompileNamesFrom(ethersBarrel, 'ETHERS_'); | ||
| const viemNames = precompileNamesFrom(viemBarrel, 'VIEM_'); | ||
| const viemNames = precompileNamesFrom(viemBarrel, ''); |
There was a problem hiding this comment.
[nit] This assertion is now tautological. Since src/viem/index.ts became export * from '../precompiles', the viem barrel is the precompiles barrel by construction, so viemNames can never differ from precompileNames and the test below can't fail.
That isn't wrong — the invariant is structurally guaranteed now rather than checked — but the comment on line 19 still describes this as a real guard. Either drop the viem case and note that the re-export makes it unnecessary, or move the check to something that can actually drift (e.g. assert src/viem/index.ts also surfaces the *_PRECOMPILE_ADDRESS constants and the chain definitions, which is the part a consumer migrating off VIEM_BANK_PRECOMPILE_ABI depends on).
| --- | ||
|
|
||
| **Breaking:** remove the IBC and Oracle precompiles. `IBC_PRECOMPILE_ADDRESS`, `IBC_PRECOMPILE_ABI`, `ETHERS_IBC_PRECOMPILE_ABI`, `getIbcPrecompileEthersV6Contract`, `VIEM_IBC_PRECOMPILE_ABI`, `ORACLE_PRECOMPILE_ADDRESS`, `ORACLE_PRECOMPILE_ABI`, `ETHERS_ORACLE_PRECOMPILE_ABI`, `getOraclePrecompileEthersV6Contract`, and `VIEM_ORACLE_PRECOMPILE_ABI` are no longer exported. | ||
| **Breaking:** remove the unsupported IBC and Oracle precompile addresses, ABIs, and Ethers factories. |
There was a problem hiding this comment.
[suggestion] Rewriting this line dropped the explicit list of removed export names. The previous text named every symbol (IBC_PRECOMPILE_ADDRESS, ETHERS_IBC_PRECOMPILE_ABI, VIEM_ORACLE_PRECOMPILE_ABI, …), which is exactly what a consumer greps for when their build breaks after a major bump.
The new prose ("addresses, ABIs, and Ethers factories") plus the Viem paragraph describes the shape of the change but leaves the reader to reconstruct the ten VIEM_*_PRECOMPILE_ABI names and the IBC/Oracle symbols themselves. Since this is the release note for a breaking change, suggest keeping the summary sentence and restoring the enumeration below it.
| @@ -0,0 +1,5 @@ | |||
| # Sei Chain v6.6 ABI fixtures | |||
|
|
|||
| The JSON files in this directory are verbatim copies of `precompiles/*/legacy/v66/abi.json` from the [`v6.6.1`](https://github.com/sei-protocol/sei-chain/tree/v6.6.1/precompiles) tag of `sei-chain`. | |||
There was a problem hiding this comment.
[suggestion] Question rather than a defect — I couldn't reach the network to settle it, so flagging per the repo guideline on hand-maintained ABIs.
The fixtures are sourced from precompiles/*/legacy/v66/abi.json, and the package README describes legacy/v66 as "a frozen historical snapshot for that release, not a moving view of the current chain surface." Could you confirm that at the v6.6.1 tag legacy/v66 is the highest version directory and matches what mainnet actually serves, rather than a superseded snapshot sitting next to a newer non-legacy precompiles/<mod>/abi.json? If a later ABI exists at that tag, consumers would still be missing methods after this sync — which would undercut the PR's premise.
If legacy/v66 is confirmed as the live surface, a one-line note here saying so (and why legacy/ is the right source) would save the next person the same question.
2 participants
Summary
legacy/v66fixtures, with IBC and Oracle recorded as explicit exclusionsSource verification
0x0000000000000000000000000000000000001011is confirmed by the frozenp256.goverify(bytes) -> bytesis confirmed by the frozenabi.jsonstruct WeightedVoteOption[]metadata is confirmed by the frozengov/abi.jsonLinear: PLT-996
Test plan
bun run --filter @sei-js/precompiles testbun run --filter @sei-js/precompiles buildbun run lint:pack:exportsbun run checkpublint --pack npm --strict --level warningattw --pack . --profile esm-only