Revert "feat(wallet): HD receive rotation via contract repository (#473)"

[Kukks]

Reverts the squash-merge of #473 (4eec1120).

Merged too early — needs more bake time before HD receive rotation lands. The original PR branch (feat/hd-wallet-via-contracts) has been restored at its pre-merge tip (afb6def2) and PR #473 will be reopened to continue review.

No code changes other than the inverse of the merge commit. pnpm exec tsc --noEmit clean, pnpm test:unit 1137/1138 pass (back to pre-#473 count).

Summary by CodeRabbit

• Bug Fixes

  • Fixed fee rate calculation in transaction preparation for more accurate fee computation.

• Refactoring

  • Removed wallet receive address rotation feature and its associated walletMode configuration option.
  • Simplified contract refresh behavior by removing support for filtering inactive contracts.
  • Streamlined HD descriptor provider implementation by removing receive rotation factory capabilities.
  • Updated descriptor provider documentation for clarity.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ef6f052-bd64-4df7-912a-5d6da157a314

📥 Commits

Reviewing files that changed from the base of the PR and between 4eec112 and 88519ab.

📒 Files selected for processing (12)

• src/contracts/contractManager.ts
• src/identity/descriptorProvider.ts
• src/identity/hdCapableIdentity.ts
• src/identity/index.ts
• src/index.ts
• src/wallet/hdDescriptorProvider.ts
• src/wallet/index.ts
• src/wallet/unroll.ts
• src/wallet/wallet.ts
• src/wallet/walletReceiveRotator.ts
• test/contracts/manager.test.ts
• test/walletHdRotation.test.ts

💤 Files with no reviewable changes (8)

• test/walletHdRotation.test.ts
• src/wallet/walletReceiveRotator.ts
• src/identity/index.ts
• src/identity/hdCapableIdentity.ts
• test/contracts/manager.test.ts
• src/identity/descriptorProvider.ts
• src/wallet/index.ts
• src/index.ts

---

Walkthrough

This PR removes HD receive-rotation functionality and its supporting infrastructure across the wallet, identity, and contract-sync layers. Specifically, it eliminates WalletMode configuration, the WalletReceiveRotator class, receive-rotation methods from HDDescriptorProvider, and the includeInactive VTXO-refresh option, while simplifying contract-sync cursor semantics and updating related public exports and tests.

Changes

Wallet Receive-Rotation Feature Removal

Layer / File(s)	Summary
Wallet receive rotation wiring src/wallet/wallet.ts, src/wallet/walletReceiveRotator.ts	Wallet no longer accepts, stores, or subscribes to receiveRotator; removed _receiveRotator field and rotation subscription install/dispose logic. Removed entire walletReceiveRotator.ts module (624 lines), which contained WalletReceiveRotator, boot-resolution types, and rotation lifecycle interfaces.
HD Descriptor Provider rotation interface removal src/wallet/hdDescriptorProvider.ts	HDDescriptorProvider no longer implements ReceiveRotatorFactory; removed getCurrentSigningDescriptor() and createReceiveRotator() methods and their boot-related imports. Class remains a descriptor allocator/signer wrapper with getNextSigningDescriptor(), isOurs, and signing delegation.
Wallet offchainTapscript immutability src/wallet/wallet.ts	Made offchainTapscript readonly on ReadonlyWallet; script initialization now derives baseline/delegate scripts directly from this.offchainTapscript.options.pubKey instead of identity-derived values, and inlines script hex encoding at contract creation.
Wallet.create() bootstrap simplification src/wallet/wallet.ts	Wallet.create() no longer resolves boot-rotator via WalletReceiveRotator.resolveBoot() and directly passes setup.offchainTapscript into the Wallet constructor. Removed boot-rotation wiring from constructor parameters.

Contract Refresh/Sync Semantics Simplification

Layer / File(s)	Summary
RefreshVtxosOptions and refreshVtxos() cleanup src/contracts/contractManager.ts	Removed includeInactive option from RefreshVtxosOptions type. Updated refreshVtxos() documentation and implementation to no longer forward includeInactive to syncContracts(). Cursor advancement is now gated only on full-scope, cursor-derived syncs; targeted/supplied-window syncs do not advance cursor.
syncContracts() scope simplification src/contracts/contractManager.ts	Removed includeInactive parameter from syncContracts() internal options. When no explicit contract subset is provided, syncing now uses this.watcher.getWatchedContracts() directly (no widening). Updated cursor-advance gate comments to clarify distinction between full-scope delta syncs and targeted syncs.

Identity and Public API Surface Cleanup

Layer / File(s)	Summary
Type-guard and descriptor provider cleanup src/identity/hdCapableIdentity.ts, src/identity/descriptorProvider.ts, src/identity/index.ts	Removed exported isHDCapableIdentity structural type-guard function. Updated DescriptorProvider JSDoc to clarify it is a pure allocator with no "current" accessor. Updated identity barrel to no longer re-export isHDCapableIdentity.
Wallet module exports simplification src/wallet/index.ts, src/index.ts	Removed WalletMode type definition and walletMode?: WalletMode config option from wallet configuration interface. Removed DescriptorProvider import from wallet index. Removed WalletMode from root-level type re-exports in src/index.ts.

Test and Minor Fix Cleanup

Layer / File(s)	Summary
Contract manager test updates test/contracts/manager.test.ts	Removed test helper collectRequestedScripts and deleted entire describe("refreshVtxos includeInactive", ...) test block that validated inactive-contract coverage, flag precedence, and cursor-advance behavior.
Wallet HD rotation test removal test/walletHdRotation.test.ts	Removed entire test suite (800 lines) covering HD rotation lifecycle, persistence, walletMode resolution, and disposal.
Fee calculation simplification src/wallet/unroll.ts	Changed feeAmount computation to apply BigInt(feeRate) directly without prior Math.ceil rounding, simplifying fractional fee-rate handling.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

• arkade-os/ts-sdk#473: Modifies the same RefreshVtxosOptions/refreshVtxos/syncContracts cursor and option-forwarding logic in contractManager.ts.
• arkade-os/ts-sdk#360: Changes refreshVtxos/syncContracts semantics and how scope/options are applied to full vs targeted VTXO syncs.
• arkade-os/ts-sdk#440: Affects the same descriptor/HD receive-rotation surfaces (hdDescriptorProvider.ts, descriptorProvider.ts, identity/index.ts, src/index.ts) and related provider API behavior.

Suggested reviewers

• louisinger
• pietro909
• arkanaai

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary change: reverting a specific feature commit (PR #473) related to HD receive rotation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch revert/473-hd-wallet

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[arkanaai]

Review: Revert of #473 (HD receive rotation)

Verdict: APPROVE — clean revert, no cross-repo breakage, returns to stable pre-#473 baseline.

What I checked

• Full diff across all 12 changed files
• Cross-repo grep of all downstream consumers (banco, btcpay-arkade, wallet, demos, arkade-explorer, boltz-swap, go-sdk, rust-sdk, dotnet-sdk) for removed exports (WalletMode, isHDCapableIdentity, walletReceiveRotator, ReceiveRotatorFactory, includeInactive)
• Git history on feat/hd-wallet-via-contracts branch vs master
• Protocol-critical surface analysis

Findings

No cross-repo breakage. WalletMode was exported from src/index.ts but zero downstream repos import it. All other removed symbols (isHDCapableIdentity, WalletReceiveRotator, ReceiveRotatorFactory) were internal — never exported from the package entrypoint.

No protocol-critical regressions. The reverted code was purely additive (HD receive rotation lifecycle). Removing it returns to the shipping baseline. VTXO handling, transaction signing, forfeit paths, round lifecycle, and exit paths are untouched by this diff — the only wallet.ts changes are removing the rotator wiring and restoring offchainTapscript to readonly (correct — no concurrent mutation without the rotator).

The finalizeAsyncPayment snapshot removal is safe. The pre-#473 code reads this.offchainTapscript directly. The snapshot pattern was only needed because the rotator could mutate offchainTapscript concurrently. No rotator → no race → snapshot unnecessary.

One issue to track separately

src/wallet/unroll.ts — BigInt(feeRate) without Math.ceil will throw RangeError if the onchain provider returns a fractional sat/vB (bitcoind regtest returns e.g. 1.006). PR #473 bundled a fix (Math.ceil(feeRate)) that this revert correctly removes (it wasn't in master before #473). This is a pre-existing bug on master, not introduced by this revert. Recommend a separate 1-line fix PR:

// src/wallet/unroll.ts ~line 335
const feeAmount = txWeightEstimator.vsize().fee(BigInt(Math.ceil(feeRate)));

Summary

Clean revert. No orphaned references, no broken imports, no protocol-level concerns. Test count returns to pre-#473 baseline (1137/1138 as noted in PR description). Ship it.

🤖 Reviewed by Arkana