fix: precise fee estimation and oscillation protection (Supersedes #195) by jgmcalpine · Pull Request #264 · arkade-os/ts-sdk

jgmcalpine · 2026-01-19T20:49:58Z

Description

This PR implements precise fee estimation using address decoding and resolves the "dust oscillation" loop reported by CodeRabbit to close #63.

Reason for new PR:
This supersedes PR #195 due to inactivity. I have built upon the original work by @shubertm and added fixes requested by @louisinger and CodeRabbit which were blocking the merge.

Changes

Precise Script Sizes: Replaced string heuristics with @scure/btc-signer address decoding. This ensures correct fee calculation for Legacy, Segwit, and Taproot inputs/outputs (Fixes @louisinger feedback).
Oscillation Fix: Refactored estimateFeesAndSelectCoins to handle the "dust paradox" where removing a change output drops the fee below the budget. This prevents infinite loops/timeouts.
Tests: Added unit tests ensuring:
- Segwit outputs are calculated as smaller than Taproot.
- The fee estimator converges successfully in dust-boundary edge cases.

Co-authored-by: @shubertm

Summary by CodeRabbit

New Features
- Enhanced transaction-size estimator with more accurate address-based output sizing and varint sizing helper.
Behavior Changes
- Send, bump and unroll flows use convergence-based fee/coin selection; wallet avoids creating change outputs below the dust threshold.
- Dust threshold exposed for consistent handling.
Bug Fixes / Refinements
- Minor on-chain validation/refactor to ensure explicit boolean returns in block-tip checks.
Tests
- Expanded wallet tests for insufficient funds, dust validation, change-edge cases, and fee/size scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

…x outputs weight

Calculate precise script values Ensure dust oscillation converges and doesn't infinite loop

coderabbitai · 2026-01-19T20:50:17Z

Walkthrough

Implements iterative fee estimation and coin-selection in OnchainWallet, adds address-based output sizing to the tx size estimator, centralizes DUST_AMOUNT, updates send/unroll/bump flows to be dust-aware, minor provider refactor, and expands wallet tests for fee/change/dust scenarios.

Changes

Cohort / File(s)	Summary
Provider Logic `src/providers/onchain.ts`	Minor refactor: `isValidBlocksTip` inner arrow now explicitly returns a boolean (control-flow-only change).
Tx size estimator `src/utils/txSizeEstimator.ts`	Added top-level `getVarIntSize`; renamed `P2WKH_OUTPUT_SIZE` → `P2WPKH_OUTPUT_SIZE`; introduced `addOutputScript` / `addOutputAddress`; refactored tapscript witness sizing and vsize to use shared varint helper.
Onchain wallet `src/wallet/onchain.ts`	Added `estimateFeesAndSelectCoins(...)` (iterative fee estimation + coin selection); removed class-level `DUST_AMOUNT`; `send` and `bumpP2A` updated to use convergent fee estimation and address-based output sizing.
Unroll & utils `src/wallet/unroll.ts`, `src/wallet/utils.ts`	Exported `DUST_AMOUNT = 546`; unroll now uses `addOutputAddress` for sizing and enforces dust threshold for output creation.
Tests `test/wallet.test.ts`	Expanded send-related tests: insufficient funds, dust-limit validation, change-dust scenarios, fee correctness across UTXO mixes and tx types, additional mocks and data.

Sequence Diagram(s)

sequenceDiagram
    participant Caller as Caller (send / bump)
    participant FeeEstimator as estimateFeesAndSelectCoins
    participant CoinSelector as selectCoins
    participant TxEstimator as TxWeightEstimator
    participant Validator as Dust/Fee Validator
    participant Broadcaster as broadcast()

    Caller->>FeeEstimator: coins, amount, feeRate, recipientAddress
    loop converge
        FeeEstimator->>CoinSelector: request selection for target (amount + feeGuess)
        CoinSelector-->>FeeEstimator: selectedCoins
        FeeEstimator->>TxEstimator: addInput(...) for each selected coin
        FeeEstimator->>TxEstimator: addOutputAddress(recipient)
        FeeEstimator->>TxEstimator: addOutputAddress(change) if changeGuess ≥ 0
        FeeEstimator->>TxEstimator: vsize()
        TxEstimator-->>FeeEstimator: estimatedVSize
        FeeEstimator->>Validator: compute fee = vsize * feeRate, check dust/convergence
        Validator-->>FeeEstimator: converge / adjust feeGuess or fail
    end
    FeeEstimator-->>Caller: inputs, changeAmount, fee
    Caller->>Broadcaster: submit raw tx
    Broadcaster-->>Caller: txid

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Remove onchain wallet from Wallet, add OnchainWallet class #102: Overlaps on src/wallet/onchain.ts changes and OnchainWallet fee/selection logic.
chore: remove hardcoded dust amount check #111: Related modifications around dust-limit handling and removal of hardcoded dust logic.

Suggested reviewers

louisinger
tiero

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the primary change: implementing precise fee estimation and adding oscillation protection, which aligns with the main refactoring of fee calculation logic across the codebase.
Linked Issues check	✅ Passed	The PR successfully replaces hardcoded transaction size with dynamic computation using TxWeightEstimator, accurately calculating sizes for Legacy/Segwit/Taproot via address decoding, directly addressing issue `#63` requirements.
Out of Scope Changes check	✅ Passed	All changes relate to precise fee estimation and dust handling: refactored TxWeightEstimator, new output methods, convergence-based fee selection, and comprehensive tests—all in scope with linked issue `#63`.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/wallet/unroll.ts`:
- Line 302: The call to TxWeightEstimator.addTxOutput is invalid; replace it
with the address-based API by calling TxWeightEstimator.addOutputAddress and
pass the wallet network as the second argument so sizing uses the correct
network context (e.g., replace usages of addTxOutput(outputAddress) with
addOutputAddress(outputAddress, wallet.network) — adjust the wallet network
accessor name if your wallet object exposes it under a different property).

In `@test/wallet.test.ts`:
- Around line 266-471: The tests mock Esplora fee responses as a plain number
but EsploraProvider.getFeeRate expects an object with fees["1"], so update all
fee-rate mocks in these tests (calls that currently resolve to mockFeeRate) to
return an object with the "1" key (e.g. { "1": mockFeeRate }) so
EsploraProvider.getFeeRate reads the correct value; search for
mockFetch.mockResolvedValueOnce(...) returning mockFeeRate in this file and
replace each with the map-shaped response to validate variable fee-rate
behavior.

src/wallet/unroll.ts

test/wallet.test.ts

coderabbitai

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@src/utils/txSizeEstimator.ts`:
- Around line 16-21: The getVarIntSize function has off-by-one bugs at the
boundary checks; update its comparisons so boundary values are handled
inclusively (e.g., change checks to n <= 0xffff and n <= 0xffffffff) or use
upper-exclusive thresholds like n < 0x10000 and n < 0x100000000 to correctly
return 3 bytes for 0xffff and 5 bytes for 0xffffffff; modify the getVarIntSize
implementation accordingly to ensure each VarInt range maps to the correct byte
size.

src/utils/txSizeEstimator.ts

louisinger

thanks! LGTM except some nit comments

src/utils/txSizeEstimator.ts

pietro909 · 2026-01-20T23:57:52Z

src/providers/onchain.ts

        Array.isArray(tip) &&
        tip.every((t) => {
-            t &&
+            return (


This is quite an important fix: isValidBlocksTip has been returning false all the time.

shubertm

LGTM. Thanks @jgmcalpine

shubertm · 2026-01-21T03:11:14Z

src/utils/txSizeEstimator.ts

+ * @param n - The integer value to check
+ * @returns The size in bytes (1, 3, 5, or 9)
+ */
+const getVarIntSize = (n: number): number => {


shubertm · 2026-01-21T03:12:12Z

src/utils/txSizeEstimator.ts

+     */
+    addOutputScript(script: Uint8Array): TxWeightEstimator {
+        this.outputCount++;
+        this.outputSize += 8 + getVarIntSize(script.length) + script.length;


shubertm and others added 23 commits January 19, 2026 14:23

Computing onchain sending fees from transanction size

bd5164c

Apply TxWeightEstimator fee

3ee8e0c

Remove wallet creation from loop

7621526

Fix fee estimation to use P2TR key spend path inputs and outputs

cf552cc

Remove unused imports from wallet/onchain.ts

66d20c2

Compute change amount fee only if change is available

8a1fb73

Format code

38c7945

Improve coin selection

15748cf

Refine estimated fee

0da108a

Fix final coin selection

29fa37b

Account for any type of output in TxWeightEstimator

36e9675

Fix typo in address regex and add test networks (testnet + regtest) t…

c3feca1

…x outputs weight

Rework fee estimation by looping

9d5485c

Fix tapscript input sizing

b68a439

Fix totalAmountNeeded computation

bb22bbe

Fix duplicate txid in mockUTXOs

295b594

Some cleaning + dust unit test

cbeaf25

Add dust unit test

0dfe45e

Cap fee estimation

f887b88

Check fee stability

c7a1d38

Implement exact script size calculation

2b623bc

Pull fee and coin selection into helper and handle oscillating dust

98c18f0

Add test coverage for fee estimation edge cases

912c447

Calculate precise script values Ensure dust oscillation converges and doesn't infinite loop

coderabbitai bot reviewed Jan 19, 2026

View reviewed changes

src/wallet/unroll.ts Outdated Show resolved Hide resolved

test/wallet.test.ts Show resolved Hide resolved

jgmcalpine added 4 commits January 19, 2026 15:11

TxWeightEstimator method update

97a2544

Add docstring for estimateFeesAndSelectCoins

3f92b08

Return a map in test fee rate mocks

7c30e3a

Add docstring for getVarIntSize

4430ad4

coderabbitai bot reviewed Jan 19, 2026

View reviewed changes

src/utils/txSizeEstimator.ts Show resolved Hide resolved

Correct off-by-one error in VarInt size calculation

b5a9f7f

louisinger requested review from bordalix, louisinger and pietro909 January 20, 2026 09:10

louisinger reviewed Jan 20, 2026

View reviewed changes

src/utils/txSizeEstimator.ts Outdated Show resolved Hide resolved

src/utils/txSizeEstimator.ts Show resolved Hide resolved

Expose output methods and add addOutputScript

dbf9e04

jgmcalpine requested a review from louisinger January 20, 2026 15:49

louisinger approved these changes Jan 20, 2026

View reviewed changes

pietro909 reviewed Jan 20, 2026

View reviewed changes

shubertm approved these changes Jan 21, 2026

View reviewed changes

shubertm mentioned this pull request Jan 21, 2026

Computing onchain sending fees from transanction size #195

Closed

pietro909 approved these changes Jan 21, 2026

View reviewed changes

louisinger merged commit 633344b into arkade-os:master Jan 21, 2026
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: precise fee estimation and oscillation protection (Supersedes #195)#264

fix: precise fee estimation and oscillation protection (Supersedes #195)#264
louisinger merged 29 commits intoarkade-os:masterfrom
jgmcalpine:fix/fee-estimation-logic

jgmcalpine commented Jan 19, 2026 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jan 19, 2026 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

louisinger left a comment

Uh oh!

Uh oh!

Uh oh!

pietro909 Jan 20, 2026

Uh oh!

shubertm left a comment

Uh oh!

shubertm Jan 21, 2026

Uh oh!

shubertm Jan 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

jgmcalpine commented Jan 19, 2026 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jan 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

louisinger left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

pietro909 Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

shubertm left a comment

Choose a reason for hiding this comment

Uh oh!

shubertm Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

shubertm Jan 21, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jgmcalpine commented Jan 19, 2026 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jan 19, 2026 •

edited

Loading