Repository navigation
refactor(felt): phantom-typed Typed[K] as the single felt implementation - #4205
Conversation
|
Claude finished @EgeCaner's task in 6m 39s —— View job Review:
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4205 +/- ##
==========================================
- Coverage 80.85% 80.73% -0.12%
==========================================
Files 512 509 -3
Lines 39124 39062 -62
==========================================
- Hits 31634 31537 -97
- Misses 7481 7516 +35
Partials 9 9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
GitHub Actions
Failure
Delegating to fp.Element through a value conversion compiled to two 32-byte stack copies per call; the array comparison reads the limbs through the pointers and is the same predicate. A test pins both helpers to the field element's own IsZero and Equal.
c810171 to
b6103a2
Compare
|
Claude finished @EgeCaner's task in 3m 25s —— View job Review:
|
Title(Describe updated until commit 3c075ad)refactor(felt): phantom-typed Typed[K] as the single felt implementation PR TypeEnhancement Description
|
| Relevant files | |||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Enhancement | 8 files
| ||||||||||||||||
| Documentation | 1 files
| ||||||||||||||||
| Tests |
…t the inlining rule
|
Claude finished @EgeCaner's task in 6m 17s —— View job Review:
|
|
Claude finished @EgeCaner's task in 3m 13s —— View job Review:
|
User description
What
core/feltnow has one field element type,Typed[K any] fp.Element, told apart at compile time by a phantom kind.Felt = Typed[feltKind];Address,Hash,ClassHash,SierraClassHash,CasmClassHash,TransactionHashandStateRootHashare aliases over unexported kinds.hash.go,address.goandunsafe.goare deleted. Ten files, all incore/felt; nothing outside the package changes and all short tests pass.Rationale
We want dedicated felt types everywhere, and as they spread they must carry the full method set, especially the methods libraries call indirectly:
MarshalCBOR/UnmarshalCBORand the text/JSON hooks. A type declared astype Hash felt.Feltinherits none of them, and the CBOR codec does not complain: it silently falls back to reflection over the four-word array. Today that yields the same bytes asFelt's fast path by coincidence of layout, which is howHash,Addressand friends have been written to disk so far. The planned on-disk encoding breaks that coincidence: a derived type without its ownMarshalCBORwould keep writing the legacy limb array and fail, or be misread, on the new format.With
Typed[K]every alias inherits every method including the codec hooks, adding a kind is one struct and one alias line, mixing kinds is a compile error, and converting between kinds is a free cast.typed_test.gopins each alias toFelt's CBOR, text and raw bytes across zero, small, single-word and full-width values, so the equivalence is tested rather than accidental. The retype lands before the codec change so every felt type goes through one codec entry point before that entry point changes format.Design choices
[]felt.Felt. So eachTyped[K]method converts tofp.Elementand calls a concrete function (setHex,setString,appendHex,unmarshalCBOR, …); the per-kind symbol inlines it and calls concrete code directly. Verified in the assembler output: no dictionary, no dynamic dispatch, identical allocations.Slice[F FeltLike]keeps the spellingfelt.Slice[felt.Felt]. A generic type cannot infer its argument, so a kind-parameterised slice would need a public kind name at 118 call sites, and a self-referential constraint would keep the spelling at the cost of a dictionary call per element. Both rejected; the decoder parses into anfp.Elementand relabels withF(element), which replaces the oldunsafepointer cast at no cost.IsZero/EqualkeepFeltLikesignatures and delegate to gnark through a value conversion that compiles to loads through the pointer, no copy. Every felt type now also has them as methods, so the twelve production call sites can move to the method during call-site migration.%Tprintsfelt.Typed[felt.hashKind]instead offelt.Hash; compiler errors still use the alias names.Benchmarks versus
origin/mainBoth test binaries run alternately six times, benchstat over twelve samples (sequential runs drift up to 5% here).
MarshalCBOR/UnmarshalCBOR, non-zeroJSONUnmarshal, random feltMarshalText, full widthJSONMarshal, random feltfelt.Slice, 1000 / 5000felt.Slice, 1000[]felt.Felt, 1000[]felt.Felt, 1000The one visible cost is JSON marshal of a plain
[]felt.Felt: json/v2 reachesAppendTextthrough the type's method table, which on main pointed at the body and now points at the per-kind symbol that makes one direct call toappendHex, 2–3 ns per felt. It is a static call, inherent to methods on an instantiated generic type, andfelt.Slice, the type hot lists should use anyway, does not pay it.PR Type
Enhancement
Description
Replace dedicated felt types with a phantom-typed
Typed[K]to share a single implementation.Delete boilerplate alias files (
address.go,hash.go) and remove the use ofunsafe.go.Update
IsZeroandEqualutils to compare array limbs directly instead of value conversions.Ensure all derived felt types inherit CBOR and JSON/text codec hooks automatically.
File Walkthrough
3 files
Delete redundant Address type and methodsDelete redundant Hash aliases and methodsDelete unsafe pointer casting utility5 files
Update CBOR codec to use Typed[K] and fp.ElementIntroduce Typed[K] and migrate Felt methods to itUse fp.Element for appending hex strings and unmarshalingDefine phantom kinds and aliases for felt typesRefactor IsZero and Equal to compare limbs directly1 files
Update FeltLike interface comments3 files
Refactor test type to use Typed kindAdd tests to ensure Typed formats identically to FeltAdd test for helper functions matching fp.Element behavior