fix(wallet): round instead of truncate in micro-denom conversion - #495
Open
memosr wants to merge 1 commit into
Open
fix(wallet): round instead of truncate in micro-denom conversion#495memosr wants to merge 1 commit into
memosr wants to merge 1 commit into
Conversation
toMicroDenom and toMicro used Math.floor on a floating-point product, so values whose decimal expansion is not exactly representable in IEEE-754 were truncated one micro-unit low. For example 2.01 CNPY converted to 2009999 uCNPY instead of 2010000. The deprecated toBaseDenom, which toMicroDenom replaced, used .toFixed(0) and rounded correctly. Switching to Math.round restores that behavior and makes the two conversion helpers agree.
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
toMicroDenom(templaterFunctions.ts) andtoMicro(useDenom.ts) convert auser-entered display amount into the chain's base unit by multiplying by the denom
factor and calling
Math.floor. The intermediate product is an IEEE-754 double, soamounts whose decimal expansion is not exactly representable land just below the true
integer and are truncated one micro-unit low.
The deprecated
toBaseDenom, whichtoMicroDenomwas introduced to replace, used.toFixed(0)and rounded correctly. This change restores that behavior.Reproduction
Math.floor)Across all 100,000 two-decimal values from 0.01 to 1000.00,
Math.flooris off by onemicro-unit for 1,196 of them (1.20%).
Math.roundis exact for all 100,000.Why it matters
toMicroDenomis referenced 25+ times inmanifest.jsonacross the send, stake, DAOdeposit and swap flows, and for transaction fees.
that the amount was adjusted.
requestedAmountfor an exact match, so an order that isone micro-unit short does not match.
minFeeinfsm/swap.go; a fee at the boundary rounds downand the transaction is rejected without a clear reason.
Changes
cmd/rpc/web/wallet/src/core/templaterFunctions.ts-toMicroDenomusesMath.round.cmd/rpc/web/wallet/src/hooks/useDenom.ts-toMicrousesMath.round.Notes
Math.roundon the double product is exact for the value range the wallet handles. Afully exact fix would use string-based decimal shifting and avoid floating point
entirely. I kept this minimal and behavior-preserving, but happy to follow up with that
if you would prefer it.
tsc --noEmitpasses on the wallet package.