Skip to content

feat: add binary codec with backward-compatible migration#5419

Open
martinconic wants to merge 2 commits intomasterfrom
feat/bigint-binary-serialization
Open

feat: add binary codec with backward-compatible migration#5419
martinconic wants to merge 2 commits intomasterfrom
feat/bigint-binary-serialization

Conversation

@martinconic
Copy link
Copy Markdown
Contributor

@martinconic martinconic commented Apr 1, 2026

Checklist

  • I have read the coding guide.
  • My change requires a documentation update, and I have done it.
  • I have added tests to cover my changes.
  • I have filled out the description and linked the related issues.

Description

  • Adds MarshalBinary/UnmarshalBinary to bigint.BigInt, using compact
    Gob encoding instead of the JSON fallback that *big.Int previously
    triggered in the statestore
  • Migrates all balance store.Put/store.Get calls in accounting and
    chequebook (totalIssued) to use bigint.BigInt explicitly
  • Fixes a backward-compat gap discovered during implementation: the old
    code stored *big.Int as an unquoted JSON decimal (e.g. 123);
    UnmarshalBinary now handles all three on-disk formats —
    Gob (new), unquoted decimal (old *big.Int), quoted string (old bigint.BigInt JSON)

Why

The statestore serializes values via encoding.BinaryMarshaler if
available, otherwise falls back to json.Marshal. Since *big.Int does
not implement BinaryMarshaler, balance values were previously stored as
unquoted JSON numbers. This change introduces a proper binary codec that
is ~2x more compact for large integers and decouples balance storage from
JSON encoding, while remaining fully backward compatible with existing
stored data.

Open API Spec Version Changes (if applicable)

Motivation and Context (Optional)

Related Issue (Optional)

Screenshots (if appropriate):

AI Disclosure

  • This PR contains code that has been generated by an LLM.
  • I have reviewed the AI generated code thoroughly.
  • I possess the technical expertise to responsibly review the code generated in this PR.

@martinconic martinconic marked this pull request as ready for review April 1, 2026 11:40
t.Parallel()

val := big.NewInt(555000)
legacyData, err := val.GobEncode()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why not test full circle the MarshalBinary part too?

if len(data) == 0 {
return nil
}
if i.Int == nil {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i think this should always be true

}
if data[0] == '"' {
// quoted decimal string: e.g. "123" — from bigint.BigInt JSON marshaling
return json.Unmarshal(data, i)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

we are straight away adding technical debt with this. the state store supports migrations. why not just migrate the balance values all at once and get it over with?

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