Skip to content

[NONEVM-5645] Migrate all contracts to abigen bindings - #808

Open
patricios-space wants to merge 14 commits into
mainfrom
ref/abigen-migration
Open

[NONEVM-5645] Migrate all contracts to abigen bindings#808
patricios-space wants to merge 14 commits into
mainfrom
ref/abigen-migration

Conversation

@patricios-space

@patricios-space patricios-space commented Jul 25, 2026

Copy link
Copy Markdown
Collaborator

@patricios-space patricios-space changed the title Ref/abigen migration [NONEVM-5645] Migrate all contracts to abigen bindings Jul 27, 2026
@patricios-space
patricios-space marked this pull request as ready for review July 30, 2026 13:38
@patricios-space
patricios-space requested a review from a team as a code owner July 30, 2026 13:38
value: 0,
dest: userAddress,
body: OnRamp_MessageValidated<RemainingBitsAndRefs> {
body: OnRamp_MessageValidated_FromOnRamp {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

hmm, why this _FromOnRamp suffix?

// --- Outgoing messages ---

// So that ABIgen can generate codec for OnRamp_MessageValidated<RemainingBitsAndRefs>
type OnRamp_MessageValidated_FromOnRamp = OnRamp_MessageValidated<RemainingBitsAndRefs>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Consider renaming as OnRamp_MessageValidated_Any

@@ -0,0 +1,46 @@
import * as morph from 'ts-morph'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Executive Summary

This PR migrates all CCIP TON contract wrappers from hand-written TypeScript to auto-generated abigen bindings produced by the acton Tolk toolchain, post-processed via a ts-morph-based transform pipeline. This is a significant architectural change that replaces ~7.8k lines of manual wrapper code with generated equivalents plus ~400 lines of ts-morph transform logic. The migration covers Router, OnRamp, OffRamp, FeeQuoter, SendExecutor, ReceiveExecutor, MerkleRoot, Token Pools (BurnMint, LockRelease, JettonLockBox), MCMS/Timelock, and supporting types.

Verdict: The approach is sound and the migration is comprehensive, but there are several security and stability concerns with the ts-morph post-processing pipeline that should be addressed before merge.


Architecture Overview

Pipeline Flow

Acton.toml → acton wrapper --ts → raw .ts files → ts-morph transforms → final wrappers/gen/

Key Components

  1. contracts/scripts/abigen.ts — Entry point: reads manifest, invokes acton wrapper --ts per contract, feeds output through transform pipeline
  2. contracts/scripts/abigen/toml.ts — Minimal TOML parser for Acton.toml manifests
  3. contracts/scripts/abigen/transforms.ts — ts-morph AST transforms (SnakedCell, CellRef, QueryID, Dict→Map, Error sorting)
  4. contracts/wrappers/gen/** — Generated output (not committed to git ideally, but committed here)
  5. contracts/wrappers/ccip/*.ts — Hand-written "manual" wrappers (reduced to opcodes/constants only)
  6. contracts/wrappers/gen/index.ts — Setup hooks for custom serializers (CrossChainAddress codec)

Security Analysis

🔴 HIGH: Custom Serializer Registry is Module-Level Mutable State

Every generated file has:

let customSerializersRegistry: Map<string, [CustomPackToBuilderFn<any> | null, CustomUnpackFromSliceFn<any> | null]> = new Map;

Each module gets its own registry, which is not shared across files. The setupGenBindings() function in gen/index.ts calls Router.registerCustomPackUnpack(...), OnRamp.registerCustomPackUnpack(...), etc. — one per module.

Risk: If a test or deployment script forgets to call setupGenBindings() (or calls it but misses a module), ensureCustomSerializerRegistered will throw at runtime with a cryptic error. This is a silent configuration failure — no type-checking catches it.

Recommendation:

  • Add a test that exercises the ensureCustomSerializerRegistered failure path to ensure error messages are actionable
  • Consider making the registry initialization lazy with a clear "not initialized" sentinel state
  • Document in a README that setupGenBindings() MUST be called before any generated wrapper is used

🔴 HIGH: ts-morph AST Transforms Are Code-Generation-At-Build-Time

The entire type safety of the wrappers hinges on the correctness of the ts-morph transforms. If a transform mutates the wrong AST node, the generated code will silently produce incorrect serialization — potentially corrupting on-chain messages.

Key transforms of concern:

  1. transformSnakedCell — Rewrites type SnakedCell<T> = c.Cell to type SnakedCell<T> = T[], then injects storeSnakedCellOf/loadSnakedCellOf helpers. This is a semantic-level change to serialization behavior. If the AST matching logic (getObjectMethod, field detection) is off-by-one or matches the wrong node, it corrupts serialization.

  2. transformCellRef — Similar pattern: rewrites CellRef<T> to T with implicit wrapping. The transformSnakedStackBoundary function uses string matching on text content ('cell') to identify stack boundary objects — fragile.

  3. sortErrorsBlocks — Uses replaceWithText with hand-constructed string interpolation. While low risk (only affects ordering), it demonstrates that the transforms operate at a text level within the AST, not purely semantically.

Recommendation:

  • Add round-trip tests for each transform: generate a known input file, run the transform, and diff against an expected output. Commit these golden files.
  • Consider adding // GENERATED BY TRANSFORM: <transform_name> comments at the start of each mutated block for traceability
  • The transformSnakedCell transform modifies both type aliases AND inserts functions — if SNAKED_HELPERS is inserted multiple times (e.g., if a source file is processed twice), it will produce duplicate function declarations

🟡 MEDIUM: Generated Files Use any Types

The custom serializer registry uses any:

let customSerializersRegistry: Map<string, [CustomPackToBuilderFn<any> | null, CustomUnpackFromSliceFn<any> | null]> = new Map;

While this is pragmatic for generated code, it means type-checking won't catch mismatches between the registered callback signatures and the types they're invoked with.

Recommendation: Acceptable for generated code, but worth noting in documentation. The error messages from ensureCustomSerializerRegistered provide a safety net.

🟡 MEDIUM: TOML Parser is Ad-Hoc

contracts/scripts/abigen/toml.ts is a minimal TOML parser that handles only a subset. It's documented as such, which is good. However:

  • It strips comments but doesn't handle inline comments within arrays
  • It treats unrecognized values as raw strings (including potentially malicious values if the manifest is untrusted)
  • No schema validation on parsed output — missing output-dir throws, but wrong types might silently pass

Recommendation: The manifest (Acton.toml) is an internal/trusted file, so this is low risk. The comment in the code ("it is not a general-purpose TOML implementation") is sufficient documentation.

🟢 LOW: Generated Code Disables ESLint

All generated files have /* eslint-disable */. This is standard practice for generated code. The transforms run on AST level, so linting would run after generation. Acceptable.


Stability Analysis

🔴 HIGH: Build Pipeline Fragility

The build pipeline has these tight coupling points:

  1. acton wrapper --ts must produce output compatible with the ts-morph transforms
  2. If the acton toolchain changes its output format, transforms will silently fail or produce incorrect code
  3. The useInMemoryFileSystem: true in ts-morph means file system state is never synced — if a transform crashes mid-way, the in-memory state is lost but disk writes still happen (the final fs.writeFileSync)

Risk: A future acton upgrade could break all transforms without any test catching it (the transforms don't validate their own preconditions).

Recommendation:

  • Pin acton version in the manifest or CI
  • Add integration tests that regenerate wrappers and compare against committed golden files
  • Consider adding --check mode to abigen that validates transforms without writing output

🟡 MEDIUM: Transform Order Dependency

Transforms are applied sequentially:

sortErrorsBlocks(sourceFile)
transformSnakedCell(sourceFile)
transformCellRef(sourceFile)
transformQueryId(sourceFile)
transformDictionaryMaps(sourceFile)

The order matters. If transformSnakedCell runs before transformCellRef, a field that is both SnakedCell<CellRef<T>> could be misprocessed. The code comments don't document the dependency order.

Recommendation: Document transform ordering in comments or add an assertion that detects if a transform was applied to already-transformed code.

🟡 MEDIUM: Test Migration Coverage

The PR modifies 98 files including ~50 test files. While all tests use the new bindings, there's a risk that some test assertions are now less precise because the generated bindings abstract more away.

Specifically:

  • contracts/tests/Logs.ts was reduced from 387 lines to a much smaller file — log assertion helpers may have lost edge cases
  • contracts/tests/ExitCode.spec.ts was reduced by ~264 lines — exit code cross-validation between old and new modules is still present but condensed

Recommendation: Verify that the CI test suite runs with full coverage and that gas reports are within acceptable variance.

🟡 MEDIUM: Shell.nix source ~/.zshrc

shellHook = ''
    source ~/.zshrc  # <-- This is new

This is a developer convenience change but introduces non-determinism: different developers' ~/.zshrc will modify the shell environment differently.

Recommendation: Remove source ~/.zshrc from shell.nix — it should be committed separately or handled outside the repo. This is outside the scope of this PR and should be reverted.


Code Quality Observations

Positive

  1. Modular transform design — Each transform is isolated in its own function with clear pre/post conditions
  2. Comprehensive migration — All CCIP contracts, token pools, and governance (MCMS/Timelock) are covered
  3. Good separation of concerns — Hand-written wrappers now contain only constants/opcodes, while generated files handle all serialization
  4. Proper test migration — Tests use the new wrappers/gen/ imports consistently
  5. registerCustomPackUnpack pattern — Clean extensibility for custom types like CrossChainAddress

Concerns

  1. No changelog or migration guide — Developers switching to the new bindings need to know about setupGenBindings() and the wrappers/gen/ import paths
  2. Generated files are committed — This is debatable. Committing generated code is safe for reproducibility but means every acton upgrade requires a PR with thousands of lines of generated changes
  3. contracts/wrappers/ccip/Logs.ts deletion — The old Logs.ts contained hand-crafted log assertion logic. The new contracts/tests/Logs.ts uses generated types but may have lost some of the nuanced matching logic

Specific PR Feedback (Publishable)

Required Before Merge

  1. Remove source ~/.zshrc from shell.nix — This introduces non-deterministic developer environment behavior and should not be in the repo.

  2. Add golden-file tests for the abigen transform pipeline — Create a test that regenerates at least one wrapper (e.g., Router.ts) and diffs against a committed expected output. This ensures future acton version upgrades don't silently break the transforms.

  3. Document setupGenBindings() requirement — Add a README.md or comment in contracts/wrappers/gen/index.ts explaining that setupGenBindings() must be called before any generated wrapper is used.

  4. Add a transform precondition check — Each transform should verify it's operating on untransformed acton output (e.g., check that type SnakedCell<T> = c.Cell exists before trying to transform it). If the pattern doesn't match, log a warning rather than silently skipping.

Nice to Have

  1. Consider a // GENERATED header — Add a comment at the top of each generated file indicating the acton version and transform commit that produced it, for traceability.

  2. Add .gitattributes for generated files — Consider linguist-generated=true so Git ignores generated files in blame and contribution graphs.

  3. Gas report comparison — Include a diff of gas reports before/after this PR to confirm the generated wrappers produce equivalent code paths.

  4. Transform ordering documentation — Add a comment in abigen.ts documenting why transforms are applied in a specific order and what the dependency graph looks like.


Conclusion

This is a well-scoped and well-executed migration that significantly reduces the maintenance burden of hand-written TON contract wrappers. The ts-morph approach is the right architectural choice for post-processing acton output. However, the build-time code generation pipeline introduces new security and stability surface area that needs guarding with golden-file tests, transform precondition checks, and clear documentation of the setupGenBindings() requirement.

Recommendation: Request changes on the 4 required items above, then approve.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Huge PR, nice work!

The bindings usage updates look good, but I do worry about the stability of the shim and how to support it. Also all code might get audited and looked at so it's additional cost to understand, support, and review for the team.

See above for potential improvements.

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