Skip to content

Comments

Add codec factory registration and enhance type mapping functionality#102

Open
freehere107 wants to merge 2 commits intomasterfrom
optimize/interface
Open

Add codec factory registration and enhance type mapping functionality#102
freehere107 wants to merge 2 commits intomasterfrom
optimize/interface

Conversation

@freehere107
Copy link
Collaborator

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the codec registry to register decoder factories (instead of shared instances), adds a codec cache for faster codec lookup/instantiation, and broadens encode support for more Go slice types while adding targeted tests/benchmarks.

Changes:

  • Replace the reflection-based registry with CodecFactory registration plus a concurrency-safe (module,spec,typeString) codec cache.
  • Expand encoding to accept more concrete slice types (e.g., []uint32, []int) via helper conversion utilities.
  • Add new unit tests/benchmarks for type mapping access, vec encoding, U256 encoding, and cache concurrency.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
utiles/tools.go Removes unused reflect-based nil helper and cleans imports.
types/v13.go Registers OriginCaller via factory to avoid shared decoder state.
types/types_test.go Adds assertions/tests for vec encoding slice types and U256 byte-slice encoding.
types/types.go Updates multiple Encode signatures to interface{} and improves some decode/encode internals.
types/type_mapping_test.go Adds test verifying Consensus exposes a TypeMapping via TypeMappingGetter.
types/registry_types.go Adds generated list of base codec type names (input to registry generator).
types/registry_gen.go Adds generated baseCodecFactories map (type name → factory).
types/registry.go Implements factory-based registry, codec cache, and GetCodec API returning (Decoder, subType, error).
types/encode_helpers.go Adds helpers to normalize many concrete slice types into []interface{} for encoders.
types/customType_test.go Adds concurrency test covering codec cache + encode/decode under goroutines.
types/customType.go Migrates custom type registration to factory functions; resets codec cache on registration.
types/benchmark_test.go Adds benchmarks for decode/encode hot paths.
types/basic_types_test.go Adds tests for Bytes/HexBytes/Option/Null/hash types encode/decode.
types/base.go Introduces Decoder/Encoder interfaces, adds fast-path decoding for common primitives, updates encode path to use GetCodec.
types/Vectors.go Refactors Vec.Encode to accept more slice types without reflection.
types/Uint.go Refactors uint decode/encode (less allocation) and extends U256 encoding inputs.
types/Struct.go Updates struct encoding to accept interface{} and type assert to map.
types/Results.go Updates result encoding to accept interface{} and type assert to map.
types/Option.go Replaces reflect-based nil detection with explicit checks for common typed-nil cases.
types/FixedArray.go Refactors fixed array encoding to support more slice types without reflection.
types/Bytes.go Updates Bytes/HexBytes/String encoders to accept interface{} + type assertion.
types/Bool.go Updates Bool encoder to accept interface{} + type assertion.
tools/gen_registry/main.go Adds generator that produces registry_gen.go from registry_types.go.
.github/workflows/ci.yml Updates Go GitHub Actions to newer major versions.

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

Comment on lines +114 to 122
if factory == nil && t != "[]" && string(t[0]) == "[" && t[len(t)-1:] == "]" {
if typePart := strings.Split(t[1:len(t)-1], ";"); len(typePart) >= 2 {
remainPart := typePart[0 : len(typePart)-1]
fixed := FixedArray{
FixedLength: utiles.StringToInt(strings.TrimSpace(typePart[len(typePart)-1])),
SubType: strings.TrimSpace(strings.Join(remainPart, ";")),
}
rt = &fixed
factory = func() Decoder { return &fixed }
}
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

In getCodecInstant's fixed-array fallback, the factory closure captures the local fixed variable and returns &fixed. Because the closure reuses the same captured instance on subsequent calls (and via codecCache), the returned decoder instance will be shared across calls/goroutines, leading to state corruption/data races when Init/Process mutates embedded ScaleDecoder fields. Build the factory to allocate a new FixedArray each time (capture only FixedLength/SubType values).

Copilot uses AI. Check for mistakes.
panic(fmt.Errorf("invalid vec input"))
values, ok := asInterfaceSlice(value)
if !ok {
panic(fmt.Errorf("invalid vec input"))
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

The panic message in U256.Encode uses "invalid vec input", which is misleading for this type (and makes debugging harder). Consider changing it to something U256-specific (e.g., "invalid U256 input") and optionally include the received type/value in the message.

Suggested change
panic(fmt.Errorf("invalid vec input"))
panic(fmt.Errorf("invalid U256 input: expected slice-like value, got %T (%v)", value, value))

Copilot uses AI. Check for mistakes.
if f.FixedLength == 1 {
return EncodeWithOpt(f.SubType, value, &ScaleDecoderOption{Spec: f.Spec, Metadata: f.Metadata})
}
panic(fmt.Errorf("invalid vec input"))
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

FixedArray.Encode panics with the message "invalid vec input" even though this is a fixed-length array encoder. Updating the message to something FixedArray-specific (and/or including the expected length/subtype) would make failures much easier to diagnose.

Suggested change
panic(fmt.Errorf("invalid vec input"))
panic(fmt.Errorf("invalid fixed array input: expected fixed length %d with subtype %q, got value of type %T", f.FixedLength, f.SubType, value))

Copilot uses AI. Check for mistakes.
Comment on lines 217 to +221
TypeRegistryLock.Lock()
TypeRegistry[key] = rt
TypeRegistry[key] = factory
TypeRegistryLock.Unlock()
}
resetCodecCache()
Copy link

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

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

resetCodecCache() is called on every regCustomKey invocation. When RegCustomTypes registers many entries (including during init via BaseType load), this repeatedly reallocates the cache and takes the cache lock for each key. Consider resetting the codec cache once per bulk registration (e.g., in RegCustomTypes after the loop) and only resetting per-key for truly incremental/single-key updates.

Copilot uses AI. Check for mistakes.
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.

1 participant