sae: Add C-chain custom tx serialization by StephenButtolph · Pull Request #5307 · ava-labs/avalanchego

StephenButtolph · 2026-04-25T16:37:10Z

Why this should be merged

This PR factors out tx serialization from #5303.

How this works

The new tx package still relies on the avalanchego codec for serialization, however some changes were made to the transactions to provide a better QoL when interacting with the custom Txs.

The codec is not exported, no package outside of tx should be manually marshaling or unmarshaling transactions.
The existing codec registered some types which are currently unused: secp256k1fx.Input and secp256k1fx.OutputOwners. Those are removed.
The existing tx definitions allow for the tx.Creds slice to be populated with types that are disallowed by later type-checks: secp256k1fx.Input, secp256k1fx.OutputOwners, secp256k1fx.TransferOutput, and secp256k1fx.TransferInput. The new transaction type disallows this within the codec.
The prior pattern of using atomic.Metadata within the unsigned transaction implementations was removed. We may choose to add caching back in later, but if done, it should be done at the Tx level once, rather than in both Export and Import.

I'm marking myself as the only code-owner of this folder (at least until @ARR4N comes back from PTO)

How this was tested

Included both Import and Export txs from mainnet C-chain as known valid transactions.
Fuzzed canonical binary format
Fuzzed equality of canonical binary format
Fuzzed equality of json format

Need to be documented in RELEASES.md?

No

StephenButtolph · 2026-04-25T16:53:41Z

+var (
+	tests = []struct {
+		name  string
+		old   *atomic.Tx


This package includes both old and new in the tests to ensure compatibility with the prior code. Technically we could remove old and just treat the bytes as canonical... But it made me more confident in the change.

StephenButtolph · 2026-04-25T16:56:14Z

+func FuzzParseCompatibility(f *testing.F) {
+	for _, test := range tests {
+		f.Add(test.bytes)
+	}
+	f.Fuzz(func(t *testing.T, data []byte) {
+		_, oldErr := parseOldTx(data)
+		oldOk := oldErr == nil
+
+		_, newErr := Parse(data)
+		newOk := newErr == nil
+
+		assert.Equal(t, oldOk, newOk, "Parse(b) == parseOldTx(b)")
+	})
+}


The testdata included for this test did fail during development, so those are intentionally kept as regression tests. I actually wasn't aware prior to this that the old tx parsing logic could result in non-secp256k1fx.Credentials being parsed into the credential type.

It is safe, as we verify the types later during verification... But it caused this test to fail.

I have since verified that such type confusion can no longer happen with this package.

Copilot

Pull request overview

This PR introduces a dedicated vms/saevm/cchain/tx package to provide canonical (binary + JSON) serialization/parsing for C-Chain atomic import/export transactions, factoring out logic from the earlier work and tightening codec/type restrictions.

Changes:

Added a new C-Chain atomic tx package with its own unexported codec manager and helpers to marshal/parse single txs and tx slices.
Added unit + fuzz tests to ensure binary/JSON compatibility with the existing coreth atomic tx encoding and known mainnet tx fixtures.
Introduced small supporting API changes: secp256k1fx.Credential.Self(), ids.ShortFromStringOrPanic, and exported atomic.ErrMissingAtomicTxs.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`vms/saevm/cchain/tx/tx.go`	Defines the new signed tx type, ID/Bytes helpers, and slice marshal/parse helpers.
`vms/saevm/cchain/tx/codec.go`	Creates an internal codec manager with explicit type registration order.
`vms/saevm/cchain/tx/import.go`	Adds the unsigned Import tx definition and C-Chain Output type.
`vms/saevm/cchain/tx/export.go`	Adds the unsigned Export tx definition and C-Chain Input type.
`vms/saevm/cchain/tx/tx_test.go`	Adds known-vector tests plus fuzzers for compatibility and round-trips.
`vms/saevm/cchain/tx/testdata/fuzz/**`	Adds fuzz corpora for regression and coverage.
`vms/saevm/cchain/tx/BUILD.bazel`	Bazel build/test targets for the new package and its tests.
`vms/secp256k1fx/credential.go`	Adds `Self()` to support restricting credential types in the new codec-facing interface.
`ids/short.go`	Adds `ShortFromStringOrPanic` helper.
`graft/coreth/plugin/evm/atomic/codec.go`	Exports the “missing atomic txs” error as `ErrMissingAtomicTxs`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

alarso16

Having seen the final form of the atomic txs, I think this is fine. I don't love the test format, only because it would be really difficult to maintain. However, I'm approving with the hope that we will delete it all after the H upgrade.

StephenButtolph · 2026-04-27T19:23:11Z

I don't love the test format, only because it would be really difficult to maintain. However, I'm approving with the hope that we will delete it all after the H upgrade.

I felt like using the prior code made these tests stronger... Once we remove coreth, the compatibility fuzzing will go away, but the test vectors will still exist.

JonathanOppenheimer

I believe the bazel metadata has to be re-generated

StephenButtolph · 2026-04-28T15:58:48Z

        "//tests/reexecute/c",
        "//tests/reexecute/chaos",
        "//vms/evm/emulate",
+        "//vms/saevm/cchain/...",


@ARR4N is this an anti-pattern? I could just expose this to the packages as needed rather than for the whole sub-tree.

Not an anti-pattern, just a question of what we believe the most appropriate scope to be. The others are only fine-grained because I had my agent (Maru) do it (with his agent). I think we could reasonably allow anything in //tests/... too but there's no need right now.

To me the more important thing is that the other //vms/saevm packages never import the //vms/saevm/cchain ones.

I updated the visibility logic to match graft/coreth.

…labs/avalanchego into StephenButtolph/atomic-tx-rewrite.1

ARR4N · 2026-04-30T06:45:44Z

        "//tests/reexecute/c",
        "//tests/reexecute/chaos",
        "//vms/evm/emulate",
+        "//vms/saevm/cchain/...",


Not an anti-pattern, just a question of what we believe the most appropriate scope to be. The others are only fine-grained because I had my agent (Maru) do it (with his agent). I think we could reasonably allow anything in //tests/... too but there's no need right now.

To me the more important thing is that the other //vms/saevm packages never import the //vms/saevm/cchain ones.

github-project-automation Bot added this to avalanchego Apr 25, 2026

StephenButtolph commented Apr 25, 2026

View reviewed changes

StephenButtolph changed the title ~~Re-write C-chain atomic txs for SAE~~ sae: Add C-chain atomic tx serialization Apr 25, 2026

StephenButtolph changed the title ~~sae: Add C-chain atomic tx serialization~~ sae: Add C-chain custom tx serialization Apr 25, 2026

StephenButtolph marked this pull request as ready for review April 25, 2026 18:04

StephenButtolph requested a review from ARR4N as a code owner April 25, 2026 18:04

Copilot AI review requested due to automatic review settings April 25, 2026 18:04

StephenButtolph requested review from a team as code owners April 25, 2026 18:04

Copilot started reviewing on behalf of StephenButtolph April 25, 2026 18:05 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread vms/saevm/cchain/tx/import.go

Comment thread vms/saevm/cchain/tx/export.go

Comment thread vms/saevm/cchain/tx/tx_test.go Outdated

Comment thread graft/coreth/plugin/evm/atomic/codec.go

Comment thread ids/short.go Outdated

StephenButtolph force-pushed the StephenButtolph/atomic-tx-rewrite.1 branch from 635de64 to f58a0aa Compare April 25, 2026 21:51

StephenButtolph requested a review from Copilot April 25, 2026 21:55

Copilot started reviewing on behalf of StephenButtolph April 25, 2026 21:55 View session

Copilot AI reviewed Apr 25, 2026

View reviewed changes

Comment thread vms/saevm/cchain/tx/tx_test.go

Comment thread vms/saevm/cchain/tx/tx_test.go

Comment thread vms/saevm/cchain/tx/tx_test.go

Comment thread vms/saevm/cchain/tx/tx.go

Comment thread vms/saevm/cchain/tx/tx_test.go

sae: Add C-chain custom tx serialization

eb8c0a0

StephenButtolph force-pushed the StephenButtolph/atomic-tx-rewrite.1 branch from d3e8110 to eb8c0a0 Compare April 25, 2026 22:42

JonathanOppenheimer reviewed Apr 27, 2026

View reviewed changes

Comment thread vms/saevm/cchain/tx/tx.go Outdated

Comment thread vms/saevm/cchain/tx/tx_test.go Outdated

Comment thread vms/saevm/cchain/tx/tx_test.go Outdated

Comment thread ids/short.go

alarso16 approved these changes Apr 27, 2026

View reviewed changes

Comment thread vms/saevm/cchain/tx/codec.go

Comment thread vms/saevm/cchain/tx/tx.go Outdated

nits

8c3b6f0

JonathanOppenheimer approved these changes Apr 27, 2026

View reviewed changes

StephenButtolph and others added 3 commits April 28, 2026 09:06

fix bazel

0369966

ci: free runner disk before publishing Antithesis images (#5310)

85ae854

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

88a234a

StephenButtolph force-pushed the master branch from 85ae854 to 659b7c3 Compare April 28, 2026 15:41

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

4e261c6

StephenButtolph commented Apr 28, 2026

View reviewed changes

StephenButtolph added 2 commits April 28, 2026 12:28

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

7073c57

nit

9b4c83f

StephenButtolph added 5 commits April 29, 2026 12:53

Unblock myself for sae progress

2f01ee8

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

fa25c9a

want got

46814f4

Merge branch 'StephenButtolph/atomic-tx-rewrite.1' of github.com:ava-…

58306f5

…labs/avalanchego into StephenButtolph/atomic-tx-rewrite.1

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

1ede276

StephenButtolph self-assigned this Apr 30, 2026

StephenButtolph moved this to In Progress 🏗️ in avalanchego Apr 30, 2026

ARR4N reviewed Apr 30, 2026

View reviewed changes

StephenButtolph added 4 commits April 30, 2026 09:13

Use the identifier rather than where the identifier is used

6e9545c

Update visability

47fe30d

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

adeaa57

Merge branch 'master' into StephenButtolph/atomic-tx-rewrite.1

149b1cb

StephenButtolph merged commit e413347 into master Apr 30, 2026
60 checks passed

github-project-automation Bot moved this from In Progress 🏗️ to Done 🎉 in avalanchego Apr 30, 2026

StephenButtolph deleted the StephenButtolph/atomic-tx-rewrite.1 branch April 30, 2026 17:49

Conversation

StephenButtolph commented Apr 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

Uh oh!

StephenButtolph Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

StephenButtolph Apr 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alarso16 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

StephenButtolph commented Apr 27, 2026

Uh oh!

JonathanOppenheimer left a comment

Choose a reason for hiding this comment

Uh oh!

StephenButtolph Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

ARR4N Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

StephenButtolph Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ARR4N Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

StephenButtolph commented Apr 25, 2026 •

edited

Loading