From 558e55059dbc5c99fa211b3ac6e6a4d4d6b034ac Mon Sep 17 00:00:00 2001 From: bhuvan-somisetty Date: Sat, 31 Oct 2026 15:48:16 +0530 Subject: [PATCH] fix(fees): avoid floating point error when converting sats/vbyte to sats/kvbyte `Math.ceil(value * 1_000)` rounds up floating point noise, e.g. 4.03 * 1_000 === 4030.0000000000005, so 4.03 sat/vB became 4031 sat/kvB. This hit 2936 of the valid values (1.001-350 sat/vB), both for the fee sent with a transaction and for `tx_fees` saved from the fee settings, which then reopened showing 4.031 instead of 4.03. Round the product to 3 decimals before rounding up, in one shared helper. Signed-off-by: bhuvan-somisetty --- src/components/send/collaborativeSend.test.ts | 12 +++++++ src/components/send/collaborativeSend.ts | 4 +-- .../settings/fees/FeeConfigDialog.test.tsx | 31 ++++++++++++++++++ .../settings/fees/FeeConfigDialog.tsx | 4 +-- src/lib/feeConfig.test.ts | 32 ++++++++++++++++++- src/lib/feeConfig.ts | 8 +++++ 6 files changed, 86 insertions(+), 5 deletions(-) diff --git a/src/components/send/collaborativeSend.test.ts b/src/components/send/collaborativeSend.test.ts index 2f425cfaa..993334738 100644 --- a/src/components/send/collaborativeSend.test.ts +++ b/src/components/send/collaborativeSend.test.ts @@ -81,6 +81,18 @@ describe('buildCollaborativeSendRequest', () => { expect(request.txfee).toBe(1_334) }) + it('passes txfee in sats/kvbyte without floating point rounding errors', () => { + const request = buildCollaborativeSendRequest({ + ...baseValues(), + txFee: { + txFeeUnit: TX_FEE_UNITS.SATS_PER_VBYTE, + txFeeInSatsPerVbyte: 4.03, + }, + }) + + expect(request.txfee).toBe(4_030) + }) + it('throws for invalid destination address', () => { expect(() => buildCollaborativeSendRequest({ diff --git a/src/components/send/collaborativeSend.ts b/src/components/send/collaborativeSend.ts index f48cada0d..9be48c47c 100644 --- a/src/components/send/collaborativeSend.ts +++ b/src/components/send/collaborativeSend.ts @@ -1,5 +1,5 @@ import type { DirectSendRequest, DoCoinjoinRequest } from '@joinmarket-webui/joinmarket-api-ts/jm' -import { TX_FEE_UNITS } from '@/lib/feeConfig' +import { satsPerVbyteToSatsPerKvbyte, TX_FEE_UNITS } from '@/lib/feeConfig' import { isValidAddress, normalizeBitcoinAddress } from '@/lib/formValidation' import { isValidInteger, isValidNumber } from '@/lib/utils' import type { SendFormValues } from './types' @@ -15,7 +15,7 @@ const txFeeValueForRequest = (data: SendFormValues): number | undefined => { if (data.txFee.txFeeUnit === TX_FEE_UNITS.BLOCKS && data.txFee.txFeeInBlocks !== undefined) { return data.txFee.txFeeInBlocks } else if (data.txFee.txFeeUnit === TX_FEE_UNITS.SATS_PER_VBYTE && data.txFee.txFeeInSatsPerVbyte !== undefined) { - return Math.ceil(data.txFee.txFeeInSatsPerVbyte * 1_000) + return satsPerVbyteToSatsPerKvbyte(data.txFee.txFeeInSatsPerVbyte) } return undefined } diff --git a/src/components/settings/fees/FeeConfigDialog.test.tsx b/src/components/settings/fees/FeeConfigDialog.test.tsx index d15966b6f..6ff257968 100644 --- a/src/components/settings/fees/FeeConfigDialog.test.tsx +++ b/src/components/settings/fees/FeeConfigDialog.test.tsx @@ -245,6 +245,37 @@ describe('FeeConfigDialog', () => { }) }) + it('saves the sats per vbyte value unchanged', async () => { + h.mutateAsync = vi.fn().mockResolvedValue({}) + const refetchAll = vi.fn().mockResolvedValue(undefined) + renderDialog( + makeValidValidation({ + refetchAll, + feeConfigValues: { + txFeeFactor: 0.1, + maxSweepFeeChangeFactor: 0.8, + maxCjAbsoluteFee: 100, + maxCjRelativeFee: 0.001, + txFee: { + txFeeUnit: 'sats/vbyte', + txFeeInBlocks: undefined, + txFeeInSatsPerVbyte: 4.03, + }, + }, + }), + ) + + fireEvent.click(screen.getByText('settings.fees.text_button_submit')) + + await waitFor(() => { + expect(h.toastSuccess).toHaveBeenCalled() + }) + expect(h.mutateAsync).toHaveBeenCalledWith({ + path: { walletname: 'test.jmdat' }, + body: { section: 'POLICY', field: 'tx_fees', value: '4030' }, + }) + }) + it('skips validation when form validation is disabled in developer mode', async () => { h.developerModeEnabled = true h.mutateAsync = vi.fn().mockResolvedValue({}) diff --git a/src/components/settings/fees/FeeConfigDialog.tsx b/src/components/settings/fees/FeeConfigDialog.tsx index 84d8cb930..bfd4b37a9 100644 --- a/src/components/settings/fees/FeeConfigDialog.tsx +++ b/src/components/settings/fees/FeeConfigDialog.tsx @@ -25,7 +25,7 @@ import { FEE_CONFIG_KEYS, type FeeConfigName } from '@/constants/jm' import { useApiClient } from '@/hooks/useApiClient' import { useFeeConfigValidation } from '@/hooks/useFeeConfigValidation' import { getErrorReason } from '@/lib/errorReason' -import { TX_FEE_UNITS } from '@/lib/feeConfig' +import { satsPerVbyteToSatsPerKvbyte, TX_FEE_UNITS } from '@/lib/feeConfig' import { cn, factorToPercentage, isValidInteger, isValidNumber, percentageToFactorString } from '@/lib/utils' import type { WalletFileName } from '@/lib/utils' import { useDeveloperMode } from '@/store/jamSettingsStore' @@ -136,7 +136,7 @@ export const FeeConfigDialog = ({ const txFeesSatsPerKvByteValue = miningData.txFee.txFeeInSatsPerVbyte !== undefined && isValidNumber(miningData.txFee.txFeeInSatsPerVbyte) - ? String(Math.ceil(miningData.txFee.txFeeInSatsPerVbyte * 1_000)) + ? String(satsPerVbyteToSatsPerKvbyte(miningData.txFee.txFeeInSatsPerVbyte)) : '' const txFeesValue = miningData.txFee.txFeeUnit === TX_FEE_UNITS.BLOCKS ? txFeesBlocksValue : txFeesSatsPerKvByteValue diff --git a/src/lib/feeConfig.test.ts b/src/lib/feeConfig.test.ts index 8d5e0d2fd..dbaefb431 100644 --- a/src/lib/feeConfig.test.ts +++ b/src/lib/feeConfig.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest' import { MAX_TX_FEE_IN_BLOCKS, MIN_TX_FEE_IN_SATS_PER_VBYTE } from '@/components/send/TxFeeForm.schema' -import { isMaxFeesConfigMissing, toJamFeeConfigValues, TX_FEE_UNITS } from './feeConfig' +import { isMaxFeesConfigMissing, satsPerVbyteToSatsPerKvbyte, toJamFeeConfigValues, TX_FEE_UNITS } from './feeConfig' describe('toJamFeeConfigValues', () => { it('should parse raw values successfully', () => { @@ -202,3 +202,33 @@ describe('isMaxFeesConfigMissing', () => { expect(isMaxFeesConfigMissing(values)).toBe(false) }) }) + +describe('satsPerVbyteToSatsPerKvbyte', () => { + it('should convert sats/vbyte to sats/kvbyte', () => { + expect(satsPerVbyteToSatsPerKvbyte(1.001)).toBe(1_001) + expect(satsPerVbyteToSatsPerKvbyte(5)).toBe(5_000) + expect(satsPerVbyteToSatsPerKvbyte(350)).toBe(350_000) + }) + + it('should not round up because of floating point noise', () => { + // e.g. 4.03 * 1_000 === 4030.0000000000005 + expect(satsPerVbyteToSatsPerKvbyte(2.007)).toBe(2_007) + expect(satsPerVbyteToSatsPerKvbyte(4.03)).toBe(4_030) + expect(satsPerVbyteToSatsPerKvbyte(4.001)).toBe(4_001) + }) + + it('should round up fractions of a sat/kvbyte', () => { + expect(satsPerVbyteToSatsPerKvbyte(1 + 1 / 3)).toBe(1_334) + expect(satsPerVbyteToSatsPerKvbyte(4.0301)).toBe(4_031) + }) + + it('should return the value of "tx_fees" it was parsed from', () => { + const mismatches: string[] = [] + for (let txFees = 1_001; txFees <= 350_000; txFees++) { + const { txFee } = toJamFeeConfigValues({ tx_fees: String(txFees) }) + const result = satsPerVbyteToSatsPerKvbyte(txFee.txFeeInSatsPerVbyte!) + if (result !== txFees) mismatches.push(`${txFees} -> ${result}`) + } + expect(mismatches).toStrictEqual([]) + }) +}) diff --git a/src/lib/feeConfig.ts b/src/lib/feeConfig.ts index ea15f80eb..df3375c3a 100644 --- a/src/lib/feeConfig.ts +++ b/src/lib/feeConfig.ts @@ -28,6 +28,14 @@ export interface JamFeeConfigValues { } } +/** + * Converts a fee rate in sats/vbyte to the integer sats/kvbyte value used for `tx_fees`. + * + * The product is rounded to 3 decimals before rounding up, so floating point noise + * (e.g. `4.03 * 1_000 === 4030.0000000000005`) doesn't bump the result by 1 sat/kvbyte. + */ +export const satsPerVbyteToSatsPerKvbyte = (value: number): number => Math.ceil(Number((value * 1_000).toFixed(3))) + export const isMaxFeesConfigMissing = (values: JamFeeConfigValues | undefined): boolean => { return !!values && (values.maxCjAbsoluteFee === undefined || values.maxCjRelativeFee === undefined) }