Fix client server number rounding#4188
Open
kajoseph wants to merge 11 commits into
Open
Conversation
…d try-catch to before all hook for debugging ci pipeline
Contributor
There was a problem hiding this comment.
Pull request overview
This PR aims to make large-number handling consistent across crypto-wallet-core (CWC), bitcore-wallet-service (BWS), and bitcore-wallet-client (BWC), primarily by normalizing Number→hex conversion and introducing a numberFormat parameter to control how tx-building numeric fields are represented over the API (e.g., hex strings to preserve precision). It also adds round-trip tests for tx proposals and includes a few operational/idempotence fixes.
Changes:
- Update CWC
toHexto convertnumberinputs via a locale-formatted decimal string beforeBigInt()conversion, plus add tests for large-number behavior. - Add
numberFormatplumbing across BWS routes/server methods and BWC client methods, including formatting of tx proposal numeric fields and send-max info. - Add BWC round-trip tx proposal integration tests; make bitcore-client wallet deletion idempotent; prevent local CI docker runs from creating root-owned build dirs.
Reviewed changes
Copilot reviewed 26 out of 27 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/crypto-wallet-core/test/utils.test.ts | Adds regression tests around toHex behavior for large Numbers. |
| packages/crypto-wallet-core/src/utils/index.ts | Adjusts toHex to normalize number inputs before BigInt() conversion. |
| packages/crypto-wallet-core/src/utils/bigint.ts | Adds bigint helpers (isEqual, BigIntTry) for cross-format comparisons/conversion. |
| packages/crypto-wallet-core/src/transactions/index.ts | Tightens typing for transaction provider proxy methods. |
| packages/bitcore-wallet-service/src/types/server.d.ts | Introduces option typings for send-max info and number formatting. |
| packages/bitcore-wallet-service/src/lib/server.ts | Adds number formatting support to getTx and getSendMaxInfo; refactors internal helpers. |
| packages/bitcore-wallet-service/src/lib/routes/walletdata.ts | Accepts numberFormat on /v1/sendmaxinfo and aligns feeLevel typing. |
| packages/bitcore-wallet-service/src/lib/routes/transactions.ts | Passes numberFormat through on tx proposal fetch route(s). |
| packages/bitcore-wallet-service/src/lib/common/utils.ts | Adds a generic number conversion helper used by send-max formatting. |
| packages/bitcore-wallet-service/src/lib/chain/xrp/index.ts | Adds typings for send-max opts. |
| packages/bitcore-wallet-service/src/lib/chain/sol/index.ts | Adds typings and adjusts amount validation to rely on toHex throwing behavior. |
| packages/bitcore-wallet-service/src/lib/chain/index.ts | Adds typings for send-max proxy method. |
| packages/bitcore-wallet-service/src/lib/chain/eth/index.ts | Adds typings and adjusts amount validation; normalizes gasLimit numeric addition. |
| packages/bitcore-wallet-service/src/lib/chain/doge/index.ts | Adds typings for send-max opts. |
| packages/bitcore-wallet-service/src/lib/chain/btc/index.ts | Adds typings for send-max opts. |
| packages/bitcore-wallet-client/test/helpers.ts | Updates explorer mock to better support EVM scenarios. |
| packages/bitcore-wallet-client/test/api.test.ts | Updates many tests to use bigint/hex formats and adds round-trip txp tests. |
| packages/bitcore-wallet-client/src/lib/verifier.ts | Uses bigint-aware equality for output amounts and fees during proposal verification. |
| packages/bitcore-wallet-client/src/lib/common/utils.ts | Exposes CWC Utils via BWC Utils for reuse. |
| packages/bitcore-wallet-client/src/lib/api.ts | Adds numberFormat query usage and bigint parsing for send-max responses; refactors createTxProposal signature. |
| packages/bitcore-node/test/integration/models/wallet.test.ts | Minor typing/import + lifecycle adjustments in integration test. |
| packages/bitcore-client/src/wallet.ts | Makes deleteWallet idempotent when default wallet path is missing. |
| packages/bitcore-client/src/storage/textFile.ts | Improves invalid-path error message and makes delete idempotent if wallet file missing. |
| packages/bitcore-client/src/storage/level.ts | Improves invalid-path error message to include path. |
| packages/bitcore-cli/src/commands/transaction.ts | Tightens destination tag validation and switches fee values to bigint; renders nonce as BigInt. |
| docker-compose.test.local.yml | Runs docker test service as host UID/GID to avoid root-owned artifacts. |
| ci.sh | Exports DOCKER_UID/DOCKER_GID for local docker-compose runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // (12345678901234567890123) | ||
| // .toLocaleString('fullwide', { useGrouping: false }) => '12345678901234568000000' => ✔ Produces a string representation of the JavaScript rounded integer that can be directly converted to BigInt | ||
|
|
||
| input = input.toLocaleString('fullwide', { useGrouping: false }); |
Comment on lines
+244
to
+258
| export function isEqual(int1: BigIntLike, int2: BigIntLike): boolean { | ||
| if (int1 == int2) return true; | ||
| if (!isBigIntLike(int1) || !isBigIntLike(int2)) return false; | ||
| try { | ||
| return BigInt(int1) === BigInt(int2); | ||
| } catch { | ||
| // BigInt() may throw for integer floats (e.g. '1.0') and/or scientific notation (e.g. '1e0') | ||
| int1 = int1.toString().includes('e') ? Number(int1).toString() : int1.toString(); | ||
| int2 = int2.toString().includes('e') ? Number(int2).toString() : int2.toString(); | ||
| const [left1, right1 = '0'] = int1.toString().split('.'); | ||
| const [left2, right2 = '0'] = int2.toString().split('.'); | ||
| if (BigInt(left1) !== BigInt(left2) || BigInt(right1) !== BigInt(right2)) return false; | ||
| return Number(int1) === Number(int2); // Note: NaN values are captured above in isBigIntLike() | ||
| } | ||
| } |
Comment on lines
+408
to
+409
| expect(BI.isEqual('Infinity', Infinity)).to.equal(true); | ||
| expect(BI.isEqual('-Infinity', -Infinity)).to.equal(true); |
Comment on lines
+452
to
+458
| it('should return true when both values are invalid but loosely equal', function() { | ||
| expect(BI.isEqual('invalid', 'invalid')).to.equal(true); | ||
| expect(BI.isEqual(null, null)).to.equal(true); | ||
| expect(BI.isEqual(undefined, undefined)).to.equal(true); | ||
| expect(BI.isEqual(null, undefined)).to.equal(true); | ||
| expect(BI.isEqual('', '')).to.equal(true); | ||
| }); |
Comment on lines
+165
to
+167
| checkNumberFormat(req.query.numberFormat, res); | ||
| req.body.txProposalId = req.params['id']; | ||
| req.body.numberFormat = req.query.numberFormat; |
| if (query.feeLevel) opts.feeLevel = query.feeLevel; | ||
| if (query.excludeUnconfirmedUtxos == '1') opts.excludeUnconfirmedUtxos = true; | ||
| if (query.returnInputs == '1') opts.returnInputs = true; | ||
| if (query.numberFormat) opts.numberFormat = query.numberFormat as 'hex' | 'number' | 'string'; |
Comment on lines
+167
to
+169
| case 'string': | ||
| convertFn = (n) => typeof n === 'string' && n.startsWith('0x') ? BigInt(n).toString() : n.toString(); | ||
| break; |
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.
Description
CWC's
toHexutil provided inconsistent conversions for large Numbers.Changelog
Testing Notes
Add any helpful notes for reviewers to test your code here.
Checklist