Fix STNumber serialization and add counterparty signing helper functions #3191
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| /* eslint-disable complexity -- required for various checks */ | ||
| import { BinaryParser } from '../serdes/binary-parser' | ||
| import { SerializedType } from './serialized-type' | ||
| import { writeInt32BE, writeInt64BE, readInt32BE, readInt64BE } from '../utils' | ||
|
|
@@ -6,8 +7,8 @@ import { writeInt32BE, writeInt64BE, readInt32BE, readInt64BE } from '../utils' | |
| * Constants for mantissa and exponent normalization per XRPL Number spec. | ||
| * These define allowed magnitude for mantissa and exponent after normalization. | ||
| */ | ||
| const MIN_MANTISSA = BigInt('1000000000000000000') // 10^18 | ||
| const MAX_INT64 = BigInt('9223372036854775807') // 2^63 - 1, max signed 64-bit integer | ||
| const MIN_EXPONENT = -32768 | ||
| const MAX_EXPONENT = 32768 | ||
| const DEFAULT_VALUE_EXPONENT = -2147483648 | ||
|
|
@@ -72,7 +73,7 @@ function extractNumberPartsFromString(val: string): { | |
| /** | ||
| * Normalize the mantissa and exponent to XRPL constraints. | ||
| * | ||
| * Ensures that after normalization, the mantissa is between MIN_MANTISSA and MAX_INT64. | ||
| * Adjusts the exponent as needed by shifting the mantissa left/right (multiplying/dividing by 10). | ||
| * | ||
| * @param mantissa - The unnormalized mantissa (BigInt). | ||
|
|
@@ -87,16 +88,41 @@ function normalize( | |
| let m = mantissa < BigInt(0) ? -mantissa : mantissa | ||
| const isNegative = mantissa < BigInt(0) | ||
|
|
||
| if (m > MAX_INT64) { | ||
| throw new Error('Mantissa overflow: value too large to represent') | ||
| } | ||
|
|
||
| // Handle zero | ||
| if (m === BigInt(0)) { | ||
| return { mantissa: BigInt(0), exponent: DEFAULT_VALUE_EXPONENT } | ||
| } | ||
|
|
||
| // Grow mantissa until it reaches MIN_MANTISSA | ||
| while (m < MIN_MANTISSA && exponent > MIN_EXPONENT) { | ||
| exponent -= 1 | ||
| m *= BigInt(10) | ||
| } | ||
|
|
||
| // Handle underflow: if exponent too small or mantissa too small, throw error | ||
| if (exponent < MIN_EXPONENT || m < MIN_MANTISSA) { | ||
| throw new Error('Underflow: value too small to represent') | ||
| } | ||
|
|
||
| // Handle overflow: if exponent exceeds MAX_EXPONENT after growing. | ||
| if (exponent > MAX_EXPONENT) { | ||
| throw new Error('Exponent overflow: value too large to represent') | ||
| } | ||
|
|
||
| // Handle overflow: if mantissa exceeds MAX_INT64 (2^63-1) after growing. | ||
| // Rounding is not required because last digit is always 0 after growing. | ||
| if (m > MAX_INT64) { | ||
| if (exponent >= MAX_EXPONENT) { | ||
| throw new Error('Exponent overflow: value too large to represent') | ||
| } | ||
| exponent += 1 | ||
| m /= BigInt(10) | ||
| } | ||
|
|
||
|
Comment on lines
+124
to
+131
Collaborator
There was a problem hiding this comment. just want to make sure, do we really need to divide here Take After
Collaborator
Author
There was a problem hiding this comment. Actually after bringing
Collaborator
There was a problem hiding this comment. sorry my bad, yes 9,900,000,000,000,000,000 would be m, but I believe that is still acceptable as long as it's less than MAX_MANTISSA
Collaborator
There was a problem hiding this comment. what would happen if we removed this block of code? Would any tests fail?
Collaborator
Author
There was a problem hiding this comment. Ya, the transaction fails with
Collaborator
There was a problem hiding this comment. I just tested in rippled. A number with So
Collaborator
There was a problem hiding this comment. I'm wondering what removing this if-block would do, since you just pushed a change that removes trailing zeroes from the mantissa
Collaborator
Author
There was a problem hiding this comment. If we remove the if-block (do not bring back increased manitssa <= INT64_MAX) the transaction fails with This is the transaction that essentially gets submitted to rippled and fails. However, if we truncate the manitssa to Can you help in confirming that if we need to use Also, if we keep the current logic (truncating if it goes beyond INT64_MAX) this is the bytes representation |
||
| if (isNegative) m = -m | ||
| return { mantissa: m, exponent } | ||
| } | ||
|
|
@@ -159,17 +185,9 @@ export class STNumber extends SerializedType { | |
| * @throws Error if val is not a valid number string. | ||
| */ | ||
| static fromValue(val: string): STNumber { | ||
| const { mantissa, exponent } = extractNumberPartsFromString(val) | ||
| const { mantissa: normalizedMantissa, exponent: normalizedExponent } = | ||
| normalize(mantissa, exponent) | ||
|
|
||
| const bytes = new Uint8Array(12) | ||
| writeInt64BE(bytes, normalizedMantissa, 0) | ||
|
|
@@ -193,7 +211,6 @@ export class STNumber extends SerializedType { | |
| * | ||
| * @returns String representation of the value | ||
| */ | ||
| toJSON(): string { | ||
| const b = this.bytes | ||
| if (!b || b?.length !== 12) | ||
|
|
@@ -202,30 +219,58 @@ export class STNumber extends SerializedType { | |
| // Signed 64-bit mantissa | ||
| const mantissa = readInt64BE(b, 0) | ||
| // Signed 32-bit exponent | ||
| let exponent = readInt32BE(b, 8) | ||
|
|
||
| // Special zero: XRPL encodes canonical zero as mantissa=0, exponent=DEFAULT_VALUE_EXPONENT. | ||
| if (mantissa === BigInt(0) && exponent === DEFAULT_VALUE_EXPONENT) { | ||
| return '0' | ||
| } | ||
|
|
||
| const isNegative = mantissa < BigInt(0) | ||
| let mantissaAbs = isNegative ? -mantissa : mantissa | ||
|
|
||
| // If mantissa < MIN_MANTISSA, it was shrunk for int64 serialization (mantissa > 2^63-1). | ||
| // Restore it for proper string rendering to match rippled's internal representation. | ||
| if (mantissaAbs !== BigInt(0) && mantissaAbs < MIN_MANTISSA) { | ||
| mantissaAbs *= BigInt(10) | ||
| exponent -= 1 | ||
| } | ||
|
|
||
| // For large mantissa range (default), rangeLog = 18 | ||
| const rangeLog = 18 | ||
|
|
||
| // Use scientific notation for exponents that are too small or too large | ||
| // Condition from rippled: exponent != 0 AND (exponent < -(rangeLog + 10) OR exponent > -(rangeLog - 10)) | ||
| // For rangeLog = 18: exponent != 0 AND (exponent < -28 OR exponent > -8) | ||
| if ( | ||
| exponent !== 0 && | ||
| (exponent < -(rangeLog + 10) || exponent > -(rangeLog - 10)) | ||
| ) { | ||
| // Strip trailing zeros from mantissa (matches rippled behavior) | ||
| let exp = exponent | ||
| while ( | ||
| mantissaAbs !== BigInt(0) && | ||
| mantissaAbs % BigInt(10) === BigInt(0) && | ||
| exp < MAX_EXPONENT | ||
| ) { | ||
| mantissaAbs /= BigInt(10) | ||
| exp += 1 | ||
| } | ||
| const sign = isNegative ? '-' : '' | ||
| return `${sign}${mantissaAbs}e${exp}` | ||
| } | ||
|
|
||
| // Decimal rendering for -(rangeLog + 10) <= exponent <= -(rangeLog - 10) | ||
| // i.e., -28 <= exponent <= -8, or exponent == 0 | ||
| const padPrefix = rangeLog + 12 // 30 | ||
| const padSuffix = rangeLog + 8 // 26 | ||
|
|
||
| const mantissaStr = mantissaAbs.toString() | ||
| const rawValue = '0'.repeat(padPrefix) + mantissaStr + '0'.repeat(padSuffix) | ||
| const offset = exponent + padPrefix + rangeLog + 1 // exponent + 49 | ||
|
|
||
| const integerPart = rawValue.slice(0, offset).replace(/^0+/, '') || '0' | ||
| const fractionPart = rawValue.slice(offset).replace(/0+$/, '') | ||
|
|
||
| return `${isNegative ? '-' : ''}${integerPart}${ | ||
| fractionPart ? '.' + fractionPart : '' | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just want to make sure,
mhere is already the mantissa with all trailing zeroes stripped, right?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, if the user enters the input as
990000000000000. Thenmhere would be990000000000000. But, I think it should be changed to the following to provide more room for other digits:I will make that change and add some tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed trailing zeroes from mantissa and added tests that passes (Otherwise would have failed due to
m > MAX_INT64) in 9e3c80e