Render a raw signed integer as signed - #12
Open
moodysalem wants to merge 1 commit into
Open
Conversation
ERC-7730 calls `raw` "the natural representation of the underlying structured data type", and for `intN` that is a signed number. `format_raw` shared one match arm between `Uint` and `Int` and put both through `BigUint::from_bytes_be`, so every negative value was printed as its two's-complement word. The failure mode is worse than a wrong-looking number. A Uniswap-style tick of -140 rendered as 115792089237316195423570985008687907853269984665640564039457584007913129639796, which is not obviously wrong to a reader — it is a plausible-looking enormous integer on the screen whose entire job is telling someone what they are about to sign. Descriptor authors have no way to route around it either: the v2 schema has no signed-integer format, so `raw` is the only way to ask for one. `int_to_bigint` already existed for `format_number` and does the two's complement correctly; this points the `Int` arm at it. The `Uint` arm is untouched, and the test pins both sides of the boundary so the fix cannot later be "simplified" back into a shared arm. The EIP-712 renderer's `Raw` path is not affected: it formats JSON values, where a negative number already arrives as "-140" rather than as a word.
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.
format_rawshares one match arm betweenArgumentValue::UintandArgumentValue::Intand puts both throughBigUint::from_bytes_be, so everynegative value prints as its two's-complement word.
A Uniswap-style tick of
-140renders aswhich is the part worth emphasising: it is not a number a reader can recognise
as wrong. It is a plausible-looking enormous integer, on the screen whose entire
job is telling someone what they are about to sign.
Why descriptors cannot work around it
ERC-7730 defines
rawas "the natural representation of the underlyingstructured data type", and for
intNthat is signed. The v2 schema has nosigned-integer format at all, so
rawis the only thing a descriptor authorcan reach for. A descriptor that wants a signed display has nowhere else to go.
In practice authors have been reaching for
number, which renders correctlyhere because
format_numberalready callsint_to_bigint— butnumberisnot in the v2 schema, so those descriptors fail schema validation. Fixing the
engine lets them be spec-conformant and correct at the same time.
The change
Split the arm and point
Intat the existingint_to_bigint, which alreadydoes the two's-complement conversion for
format_number:The
Uintarm is unchanged. The test pins both sides of the boundary — thesame 32-byte word read as
Intand asUint, plus zero,-1, and the sign-bitedge — so the fix cannot later be simplified back into a shared arm.
format_raw_with_separatorinherits it through its per-itemformat_rawcall.The EIP-712 renderer's
Rawpath is unaffected: it formats JSON values, wherea negative number already arrives as
"-140"rather than as a word.Testing
cargo test --workspacepasses: 139 unit tests plus every integration suite,with no existing expectation changed. Nothing in the corpus was relying on the
unsigned reading.
Found while tracking down why a wallet's approval screen showed a 78-digit
number for a liquidity position's lower tick.