fix: normalize decimal values in encoders and decoder - #9
Merged
Merged
Conversation
Non-normalized decimal values (e.g. mantissa=50, exponent=0 instead of
mantissa=5, exponent=1) caused the Rust decoder to reject edits with
DecimalNotNormalized, silently dropping them from the kg-indexer pipeline.
The root cause is that neither the TypeScript nor Rust encoder normalized
decimal mantissa/exponent pairs before writing. Users passing values like
{mantissa: 50, exponent: 0} produced valid-looking but non-canonical wire
data that the strict decoder rejected.
Changes:
- Both encoders (Rust + TypeScript) now normalize decimals before encoding
by stripping trailing zeros from the mantissa and adjusting the exponent
- The Rust decoder now normalizes on read instead of rejecting, so
already-published non-normalized edits (like the "Create payout" edit
with CID bafkreicaxdkhdaw77exhib2c7niio6y3757chivyum4sswrzroggekndwa)
can be decoded successfully
- Added normalize_decimal() and supporting helpers for big mantissa
conversion (i128 <-> two's complement bytes)
- Updated tests to verify normalization roundtrips
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces canonical decimal normalization (strip trailing zeros from mantissa and adjust exponent) in the GRC-20 codecs, and adds/updates tests to validate normalization behavior.
Changes:
- Add decimal normalization during TypeScript decimal encoding (including Big mantissas).
- Update Rust decimal decoding/encoding to normalize decimals rather than rejecting non-normalized inputs.
- Add test coverage for decimal normalization scenarios in both TypeScript and Rust.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| typescript/src/codec/value.ts | Adds decimal normalization + BigInt→two’s-complement re-encoding for canonical decimal encoding. |
| typescript/src/test/basic.test.ts | Adds roundtrip tests asserting normalized decimal mantissa/exponent after encode/decode. |
| rust/crates/grc-20/src/codec/value.rs | Switches from “reject non-normalized decimals” to “normalize on read/encode”, and expands decimal tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Normalize DECIMAL values consistently on encode and decode in both runtimes. Replace Rust’s lossy i128-based big mantissa normalization with arbitrary- precision byte-level normalization, preserve borrowed canonical mantissas on decode, and fix TypeScript’s two’s-complement minimization for negative power-of-two boundary values. Add regression tests for large big mantissas, decode-time normalization, and minimal negative big-byte encodings. Update the spec and encoding docs to reflect canonical encoding plus lenient decode-time normalization for legacy payloads.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Tighten DECIMAL canonicalization so both runtimes always emit minimal two’s-complement mantissa bytes and reject redundant sign extension on decode. Update the Rust encoder to treat non-minimal big mantissas as requiring canonicalization, and rework the TypeScript big-mantissa path to normalize on bytes instead of preserving non-minimal input bytes. Add regression coverage for: - trimming non-minimal big mantissas on encode in Rust and TypeScript - rejecting non-minimal mantissa bytes on TypeScript decode - preserving the existing large-mantissa and negative boundary behavior This keeps Rust and TypeScript aligned and closes the latest PR review findings.
Treat mantissa_type = 0x01 with a zero-length payload as malformed instead of silently canonicalizing it to zero. This keeps empty byte mantissas distinct from the canonical zero representation and aligns both decoders with the minimal two’s-complement rule. Add regression tests in Rust and TypeScript to lock in rejection of empty big-mantissa payloads while preserving the existing decimal normalization behavior.
Remove the now-unused BigInt mantissa helper and drop the unused exponent parameter from bigMantissaNeedsCanonicalization. These were left behind after moving TypeScript DECIMAL normalization to the byte-based path and caused tsc to fail in CI with TS6133 unused symbol errors.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reject DECIMAL exponents outside the int32 range in both decoders and fail normalization when stripping trailing zeros would overflow the exponent. This prevents silent truncation on decode and avoids debug panics or release-mode wrapping during canonicalization. Also enforce the shared 64 MB byte-length limit for big DECIMAL mantissas on decode so oversized payloads are rejected before normalization work begins. Add regression coverage for: - out-of-range DECIMAL exponents on decode - exponent overflow during encode-time and decode-time normalization - oversized big-mantissa byte payloads - the nested TypeScript decimal normalization test block formatting cleanup
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Treat empty DECIMAL big-mantissa byte arrays as invalid input instead of silently canonicalizing them to zero. This aligns encode-time behavior with the existing decode-time minimality checks and keeps Rust and TypeScript consistent. Also tighten the zero-detection path so empty byte arrays are no longer treated as numeric zero, and avoid an extra full-array scan in the TypeScript big- mantissa canonicalization path. Add regression coverage for rejecting empty big mantissas during encoding in both runtimes, and update the Rust helper test to reflect the stricter invariant.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Enforce the DECIMAL big-mantissa byte limit on encode in both Rust and TypeScript so the library cannot emit payloads that its own decoder rejects. Also cap big-mantissa normalization to a fixed number of divide-by-10 steps to prevent pathological legacy decimals with huge trailing base-10 factors from causing excessive CPU work during encode-time or decode-time normalization. Add regression coverage for: - rejecting oversized big mantissas on encode - rejecting decimals that exceed the normalization step limit on encode/decode - preserving the existing aligned Rust/TypeScript DECIMAL behavior
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Non-normalized decimal values (e.g.
mantissa=50, exponent=0instead ofmantissa=5, exponent=1) caused the Rust decoder to reject edits withDecimalNotNormalized, silently dropping them from the kg-indexer pipeline.The root cause is that neither the TypeScript nor Rust encoder normalized decimal mantissa/exponent pairs before writing. Users passing values like
{mantissa: 50, exponent: 0}produced valid-looking but non-canonical wire data that the strict decoder rejected.Changes
normalize_decimal()and supporting helpers for big mantissa conversion (i128 ↔ two's complement bytes)Why both encoder and decoder?
bafkreicaxdkhdaw77exhib2c7niio6y3757chivyum4sswrzroggekndwa, a "Create payout" edit withmantissa=50, exponent=0) can be ingested