perf(core): back transaction and event []felt.Felt fields with felt.Slice

[EgeCaner]

What

Switches the []felt.Felt fields on core transactions, events and L2→L1 messages — plus
their v8/v9/v10 wire counterparts and vm.Transaction — to felt.Slice, so they decode
through the fixed-shape CBOR path instead of the generic reflect decoder.

Encoding is byte-identical (verified across every array-header width, nil and empty
included, under the canonical enc mode), so stored data needs no migration and stays
readable by the previous binary. JSON is unchanged too, including null vs [] vs omitted,
in both default and GOEXPERIMENT=jsonv2 builds — so the blockifier wire format is
untouched.

Two notes:

• core.InvokeTransaction.ProofFacts stays []felt.Felt: it carries cbor:",omitempty",
  and felt.Slice's value-receiver MarshalCBOR defeats that, writing a null key for
  every invoke transaction.
• The RPC adapters now use &t.TransactionSignature instead of new(t.Signature()), which
  drops an allocation per transaction but means the pointer aliases core state. Every use is
  a read-only deref, matching what CallData: &t.CallData already did.

Benchmarks

method	main med (ms)	felt.Slice med	Δ med	Δ avg	Δ p90	Δ p99	Δ req/s
starknet_getBlockWithTxs	0.572	0.536	−6.2%	−6.3%	−6.6%	−8.6%	+4.3%
starknet_getBlockWithReceipts	0.975	0.915	−6.1%	−7.5%	−7.7%	−8.8%	+4.2%
starknet_getTransactionReceipt	0.264	0.257	−2.7%	−2.6%	−3.1%	−2.0%	+2.2%
starknet_getTransactionByHash	0.214	0.210	−1.9%	−2.2%	−2.6%	−2.1%	+2.2%
starknet_getEvents	10.233	10.215	−0.2%	−0.8%	−0.9%	+0.1%	+0.7%

Methodology. k6 replays a fixed list of recorded requests, one at a time, against a
read-only mainnet snapshot. Each round runs main first and then this branch, so if the
machine has a slow moment it hits both sides instead of just one. Three rounds per method,
50k requests each (20k for getEvents). The node is restarted between sides, so pebble's
in-process cache starts empty for both — but the OS page cache survives, which is why round 1
is discarded: main ran first and paid for the initial disk reads while this branch found the
same data already in memory. The figures are averages of the remaining rounds — repeat runs of
the same binary vary by about 3%, so anything smaller than that is noise.

[github-actions]

Reviewing PR #3955

• Gather context (diff, felt.Slice semantics, CLAUDE.md)
• Review core/transaction.go + hashing paths
• Review rpc v8/v9/v10 adapters & JSON marshalling
• Review vm/, sequencer/, tests
• Post findings

View job run • branch perf/felt-slice

[codecov]

Codecov Report

❌ Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.59%. Comparing base (36c289c) to head (5e9e28d).

Files with missing lines	Patch %	Lines
rpc/v8/estimate_fee.go	0.00%	1 Missing ⚠️
rpc/v9/estimate_fee.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3955      +/-   ##
==========================================
- Coverage   74.77%   74.59%   -0.19%     
==========================================
  Files         461      461              
  Lines       40928    40928              
==========================================
- Hits        30605    30530      -75     
- Misses       8223     8295      +72     
- Partials     2100     2103       +3     

Flag	Coverage Δ
jsonv2	77.36% <92.59%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[github-actions]

Claude finished @EgeCaner's task in 9m 44s —— View job

---

Review: PR #3955

• Gather context (diff, felt.Slice semantics, CLAUDE.md)
• Review core/transaction.go + hashing paths
• Review rpc v8/v9/v10 adapters & JSON marshalling
• Review vm/, sequencer/, tests
• Post findings

No correctness regressions found. 5 inline comments, all perf/consistency.

Blocker

None.

Important

1. vm/transaction.go:147,170,179,188 — the casts back to *[]felt.Felt opt out of the fast path in the hottest place. vm.Transaction is JSON-marshaled per transaction in marshalTxn for every estimateFee/simulate/trace/block execution. Converting vm.Transaction's *[]felt.Felt fields to *felt.Slice[felt.Felt] gets the win and deletes all four casts. Fix this →
2. core/transaction.go:91 converts L1ToL2Message.Payload but not L2ToL1Message.Payload (line 101). L1ToL2Message is populated only by sn2core and never surfaces in an RPC response; L2ToL1Message.Payload is emitted in every receipt via MsgToL1.Payload (rpc/v9/transaction.go:308, rpc/v10/transaction_types.go:265). The hot one was missed.

Nit

3. core/transaction.go:302 — InvokeTransaction.ProofFacts is the only slice field left unconverted in that struct, and it is on the v10 response path.
4. rpc/v10/transaction_types.go:427 — same field on the RPC side; also rpc/v10/estimate_fee.go:71 still uses make([]felt.Felt, …) where v8/v9 were switched in this PR (compiles fine, cosmetic).
5. rpc/v8/events.go:30-31 — v8 Event.Keys/Data stay []felt.Felt, so v8 event responses get no fast path. v8 adapts events by copying fields (rpc/v8/transaction.go:918-923) rather than the unsafe reinterpret v9/v10 use, so it compiles and behaves correctly — but it also has no var _ = Event(core.Event{}) guard, so v8 can silently drift from core.Event the way v9/v10 can't.
6. rpc/v8/transaction.go:1008 (+ v9/v10 equivalents) — new(t.Signature()) → &t.TransactionSignature changes the RPC Signature pointer from pointer-to-copied-header to pointer into the core transaction. Read-only everywhere today and CallData: &t.CallData already did this, so it's safe and consistent; flagging only because a future write through *tx.Signature would now mutate shared/cached core state.

Things I verified as safe (so you don't have to re-derive them)

• CBOR/DB compatibility: byte-identical, no migration needed. Slice.MarshalCBOR emits a shortest-form array header followed by encodeLimbs per element — exactly what the generic encoder already produced, since felt.Felt has had its own MarshalCBOR (core/felt/cbor.go:12). Nil handling matches too: Slice.MarshalCBOR emits 0xf6, and encoder uses cbor.CanonicalEncOptions(), whose NilContainers default is NilContainerAsNull. Existing records also still decode via the unmarshalGeneric fallback.
• omitempty / validate semantics unchanged. jsonv2 omitempty inspects the encoded output ([] → omitted, both before and after); go-playground required/required_if on *felt.Slice[felt.Felt] still resolves to a nil-pointer check via reflect.Ptr, independent of the named type.
• No write-through aliasing. adaptBroadcasted*ToCore (rpc/v10/adapt_transaction.go:433-434,461,498) only derefs to read; rpccore.TransactionCache stores hashes only; LimitSlice doesn't touch these fields.
• The felt.Slice↔core.Event zero-copy guard holds: var _ = Event(core.Event{}) in v9/v10 events.go still compiles, so the unsafe.Pointer reinterpret at rpc/v9/transaction.go:1080 / rpc/v10/adapt_transaction.go:177 remains sound.

Verification

GOEXPERIMENT=jsonv2 go vet ./... passes cleanly across the whole repo (this compiles test files too, so all 20 changed files build in both configurations exercised by CI).

I could not run the test suite — go test requires an approval this job doesn't have, so the rpc/v8|v9|v10, core, sequencer and rpc/rpccore tests were not executed here. Given the change is a mechanical type swap with felt.Slice as a defined type over []felt.Felt, the compile-time check covers most of the risk, but CI's test job is the real gate.
• branch perf/felt-slice

[github-actions]

Claude finished @EgeCaner's task in 12m 12s —— View job

---

Re-review: PR #3955 (5e9e28d)

• Diff the new commits vs the previous review
• Verify @EgeCaner's fxamacker omitempty claim
• Review vm/transaction.go conversion + L2ToL1Message.Payload
• Build / vet both GOEXPERIMENT configurations
• Post findings

Both previous important items are addressed and correct. No blockers, no correctness regressions. 2 inline comments, both follow-up/perf.

You're right about omitempty — my reasoning was wrong

I confirmed it against the pinned fxamacker/[email protected], and it's not an OmitEmptyMode subtlety — the tag is unconditionally dead for cbor.Marshaler types:

// encode.go:2004
if reflect.PointerTo(t).Implements(typeMarshaler) {
	return encodeMarshalerType, alwaysNotEmpty, getIsZeroFunc(t)
}

encodeStruct (encode.go:1561) gates only on that isEmptyFunc, so alwaysNotEmpty means the field is always written. Your hex output is exactly what the code does. My "evaluated on the Go value before the marshaler runs" applies to encoding/json v1, not this encoder — leaving ProofFacts as []felt.Felt is the right call.

Scope of the trap, for the record: cbor:",omitempty" appears on exactly three fields repo-wide — InvokeTransaction.ProofFacts and two []int fields in core/block_transaction.go — so nothing converted in this PR is affected. Decoding would also have survived it (absent key → nil, 0xf6 → nil via unmarshalGeneric); the damage would have been silent record growth, not data loss. Inline comment suggests a reflection guard in the core/partial_cbor.go style so it can't be undone by accident.

Previous findings, now resolved

1. vm/transaction.go — all four casts gone, struct fields converted, Signature: &t.TransactionSignature. I verified the win is real rather than nominal: vm marshals via encoding/json (v1 API), and under GOEXPERIMENT=jsonv2 the v1 shim still dispatches MarshalJSONTo (encoding/json/v2/arshal_methods.go:205 — the legacy-semantics fallback only triggers on a forced-address receiver or an object-name position, neither of which applies to felt.Slice's value receiver). nilToZero still returns a non-nil empty slice, so blockifier keeps getting [] and not null.
2. L2ToL1Message.Payload + all three MsgToL1.Payload — converted; every receipt on the response path now takes the fast path.

Inline comments

• core/transaction.go:296 — guard so the omitempty trap can't be reintroduced silently (see above).
• rpc/v10/adapt_transaction.go:126 — the six new (*[]felt.Felt)(...) casts point at starknet.Transaction, which clients/feeder also decodes on every synced block. Converting it deletes the casts and is the only thing that would exercise Slice.UnmarshalJSONFrom, which this PR never hits. Fine as a separate PR.

Other fast-path gaps left (all outside this PR's stated scope)

Ranked by response size, in case they're useful for the follow-up:

Field	Path
SierraProgram []felt.Felt (rpc/v10/transaction_types.go:433)	getClass* — thousands of felts per response
Calldata / Result / Keys / Data / Payload (rpc/v10/trace_invocation.go:11-30, rpc/v9/trace.go:50-75)	traceBlockTransactions / simulate — deeply nested, largest responses the node emits
TxnHashes []felt.Felt (rpc/v10/block.go:75, rpc/v9/block.go:239)	every getBlockWithTxHashes
EventFilter.Keys [][]felt.Felt (v8/v9/v10)	getEvents request side; needs []felt.Slice[felt.Felt]
rpc/v8/events.go:30-31	v8 Event.Keys/Data still unconverted, so v8 event responses get nothing

ProofFacts on the RPC side (rpc/v10/transaction_types.go:427) has to stay *[]felt.Felt now that core does — withdrawing that nit.

Re-verified as still safe

• CBOR/DB: byte-identical, no migration. Slice.MarshalCBOR emits the same shortest-form array header + encodeLimbs the generic encoder produced (felt.Felt has had MarshalCBOR since core/felt/cbor.go:12), nil → 0xf6 matches CanonicalEncOptions' NilContainerAsNull, and existing records still decode via the unmarshalGeneric fallback. Newly relevant for L2ToL1Message.Payload: no omitempty, so unaffected by the above.
• JSON output unchanged. jsonrpc and clients/feeder both use the v1 API, where nil slices marshal to null — matching MarshalJSONTo's explicit jsontext.Null. (Worth remembering that json/v2 default semantics would emit [] for a nil slice, so this only holds while the v1 API is the entry point.)
• felt.Slice ↔ core.Event zero-copy guard holds. var _ = Event(core.Event{}) still compiles in v9 and v10, so the unsafe.Pointer reinterprets at rpc/v9/transaction.go:1080 and rpc/v10/adapt_transaction.go:177 remain sound.
• No write-through aliasing. The new &t.TransactionSignature in vm/transaction.go (126, 145, 185) aliases the core transaction like the RPC adapters do; vm.Transaction is marshal-only, marshalTxn mutates just its own Version copy, and nilToZero returns a pointer to a local header. Read-only everywhere today — same standing caveat that a future write through *tx.Signature would touch cached core state.
• Request-side decoding of untrusted input is safe. Slice.UnmarshalJSONFrom runs on dec.ReadValue() output, so the array is already syntactically valid; non-string elements and unparseable hex return errors, and every index is bounds-guarded. No panic path.

Verification

go vet ./... and GOEXPERIMENT=jsonv2 go vet ./... both pass clean across the repo. That compiles test files too, and there are no build-tagged Go files outside core/felt/slice_jsonv2.go, so every file is covered by at least one of the two runs.

I still could not run the test suite — go test needs an approval this job doesn't have, so core, rpc/v8|v9|v10, rpc/rpccore, adapters/sn2core and sequencer tests were not executed here. CI's test job remains the real gate; the risk it covers that vet doesn't is behavioural (golden JSON/CBOR fixtures), not structural.
• branch perf/felt-slice

[github-actions]

important (guard against a silent storage-format change): you're right about omitempty, and I confirmed it in the pinned dependency — so keeping ProofFacts as []felt.Felt (line 302) is the correct call, but nothing stops the next person from undoing it.

fxamacker/[email protected]/encode.go:2004:

if reflect.PointerTo(t).Implements(typeMarshaler) {
	return encodeMarshalerType, alwaysNotEmpty, getIsZeroFunc(t)
}

alwaysNotEmpty becomes the field's isEmptyFunc, and encodeStruct (encode.go:1561) gates purely on f.ief, so for any cbor.Marshaler the omitempty tag is a no-op regardless of OmitEmptyMode. My earlier "evaluated on the Go value before the marshaler runs" was wrong — that's encoding/json v1, not this encoder.

Consequence is milder than a migration (decoding is unaffected: absent key → nil, 0xf6 → nil via unmarshalGeneric), but every version-3 invoke record without proof facts would grow a proof_facts: null entry, silently, with no test failing. Since core/partial_cbor.go already has reflection tests asserting field-set invariants, the cheapest guard is one more in that style — walk the Transaction/TransactionReceipt struct trees and fail if any field whose type implements cbor.Marshaler carries cbor:",omitempty". That documents why ProofFacts is the odd one out and makes the trap a compile-time-ish failure instead of a silent format change. Add the guard →

[github-actions]

nit (perf, follow-up): these six (*[]felt.Felt)(...) casts are now the only fast-path opt-outs left in the PR, and the type on the other side of them is the one that would pay best.

starknet.Transaction (starknet/transaction.go:172,176,177,185,186) isn't just the outbound addTransaction body — starknet.Block.Transactions is the same type, so it's also what clients/feeder decodes on every block during sync, along with starknet.Event.Keys/Data (241-243) and L1ToL2Message/L2ToL1Message.Payload (248,256). That's the heaviest felt-slice deserialization path in the node, and Slice.UnmarshalJSONFrom is the half of felt.Slice this PR doesn't exercise anywhere yet.

I checked that it actually dispatches: clients/feeder uses encoding/json (v1 API), and under GOEXPERIMENT=jsonv2 the v1 shim still routes to MarshalJSONTo/UnmarshalJSONFrom — encoding/json/v2/arshal_methods.go:205 and :300 only fall back to the reflect path under CallMethodsWithLegacySemantics when the receiver needs a forced address (not the case for felt.Slice's value receiver) or when the value sits in an object-name position. Same reason the vm.Transaction change in this PR pays off through json.Marshal.

Fine as a separate PR — flagging so it doesn't get lost, since converting it also deletes these casts.