diff --git a/README.md b/README.md index 26b022451..b87fd5938 100644 --- a/README.md +++ b/README.md @@ -195,7 +195,7 @@ export ARKD_WALLET_ADDR=localhost:6060 export ARKD_WALLET_FALLBACK_ADDRS=localhost:6061,localhost:6062 ``` -Every wallet, primary and fallback, must be initialized and unlocked out of band (see [Setup arkd](#setup-arkd)) and must be on the same network as the primary; `arkd` validates this at startup and refuses to start otherwise. The primary wallet remains the sole source of the forfeit address, connector address, scanning and signing. The additional wallets are used only as sweep fallbacks; that wiring lands in a later change. +Every wallet, primary and fallback, must be initialized and unlocked out of band (see [Setup arkd](#setup-arkd)) and must be on the same network as the primary; `arkd` validates this at startup and refuses to start otherwise. The primary wallet remains the sole source of the forfeit address, connector address, scanning and signing. The additional wallets are used only as sweep fallbacks: when a batch cannot be signed by the primary wallet (for example a batch created before this wallet became the primary), `arkd` tries each fallback wallet in turn until one can sign it. The swept funds always go to the primary wallet's address, whichever wallet signs. ### Connect to signer diff --git a/docker-compose.regtest.yml b/docker-compose.regtest.yml index 478b59226..69a02913b 100644 --- a/docker-compose.regtest.yml +++ b/docker-compose.regtest.yml @@ -66,9 +66,9 @@ services: interval: 2s timeout: 2s retries: 60 - # A second arkd-wallet acting as an additional LP wallet, registered with arkd - # via ARKD_WALLET_FALLBACK_ADDRS. arkd currently only dials it and verifies it - # is initialized and unlocked at startup; it is not yet used to sign sweeps. + # A second arkd-wallet acting as an additional LP wallet, wired into arkd as a + # sweep fallback via ARKD_WALLET_FALLBACK_ADDRS: sweep signing is attempted with + # the primary first, then each fallback. arkd-wallet-2: restart: unless-stopped build: diff --git a/internal/config/config.go b/internal/config/config.go index a349d3317..845c40845 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -688,6 +688,17 @@ func (c *Config) FallbackWallets() []FallbackWallet { return c.walletFallbacks } +// fallbackWalletServices returns just the dialed fallback wallet clients, for +// consumers (the app and admin services) that sign with them but don't need the +// dial address. +func (c *Config) fallbackWalletServices() []ports.WalletService { + svcs := make([]ports.WalletService, 0, len(c.walletFallbacks)) + for _, fb := range c.walletFallbacks { + svcs = append(svcs, fb.Service) + } + return svcs +} + func (c *Config) UnlockerService() ports.Unlocker { return c.unlocker } @@ -841,13 +852,9 @@ func (c *Config) walletService() error { return nil } -// dialFallbackWallets dials the configured fallback arkd-wallets and validates -// that each one is reachable and on the same network as the primary. Fallback -// wallets belong to additional liquidity providers and are intended as sweep -// fallbacks, though arkd does not yet use them to sign sweeps; for now they are -// only dialed and readiness-checked. The primary remains the sole source of the -// forfeit pubkey, addresses and signing. Any failure is fatal so a misconfigured -// wallet is surfaced at startup. +// dialFallbackWallets dials the configured fallback arkd-wallets, used as sweep +// signers, validating each is reachable and on the primary's network. Any +// failure is fatal so misconfiguration surfaces at startup. func (c *Config) dialFallbackWallets() ([]FallbackWallet, error) { fallbacks := make([]FallbackWallet, 0, len(c.WalletFallbackAddrs)) seen := make(map[string]struct{}, len(c.WalletFallbackAddrs)) @@ -1001,7 +1008,7 @@ func (c *Config) appService() error { } svc, err := application.NewService( - c.wallet, c.signer, c.repo, c.txBuilder, c.scanner, + c.wallet, c.fallbackWalletServices(), c.signer, c.repo, c.txBuilder, c.scanner, c.scheduler, c.liveStore, c.alerts, c.fee, ) if err != nil { @@ -1019,7 +1026,7 @@ func (c *Config) adminService() error { } c.adminSvc = application.NewAdminService( - c.wallet, c.repo, c.txBuilder, c.liveStore, unit, c.fee, + c.wallet, c.fallbackWalletServices(), c.repo, c.txBuilder, c.liveStore, unit, c.fee, ) return nil } diff --git a/internal/core/application/admin.go b/internal/core/application/admin.go index bb40c03fc..92578ace0 100644 --- a/internal/core/application/admin.go +++ b/internal/core/application/admin.go @@ -69,7 +69,10 @@ type AdminService interface { } type adminService struct { - walletSvc ports.WalletService + walletSvc ports.WalletService + // walletFallbacks are additional arkd-wallets whose batches admin.Sweep may also + // sign; signing is attempted with the primary wallet first, then each fallback. + walletFallbacks []ports.WalletService repoManager ports.RepoManager txBuilder ports.TxBuilder sweeperTimeUnit ports.TimeUnit @@ -82,11 +85,13 @@ type adminService struct { } func NewAdminService( - walletSvc ports.WalletService, repoManager ports.RepoManager, txBuilder ports.TxBuilder, + walletSvc ports.WalletService, walletFallbacks []ports.WalletService, + repoManager ports.RepoManager, txBuilder ports.TxBuilder, liveStoreSvc ports.LiveStore, timeUnit ports.TimeUnit, feeManager ports.FeeManager, ) AdminService { return &adminService{ walletSvc: walletSvc, + walletFallbacks: walletFallbacks, repoManager: repoManager, txBuilder: txBuilder, sweeperTimeUnit: timeUnit, @@ -95,6 +100,12 @@ func NewAdminService( } } +// signingWallets returns the wallets to try when signing a sweep, in order: the +// primary wallet first, then any configured fallbacks. +func (a *adminService) signingWallets() []ports.WalletService { + return append([]ports.WalletService{a.walletSvc}, a.walletFallbacks...) +} + func (a *adminService) Wallet() ports.WalletService { return a.walletSvc } @@ -682,7 +693,7 @@ func (a *adminService) Sweep( ) } - txid, txhex, err = a.txBuilder.BuildSweepTx(inputs) + txid, txhex, err = buildAndSignSweepTx(a.txBuilder, a.signingWallets(), inputs) if err != nil { return } diff --git a/internal/core/application/admin_test.go b/internal/core/application/admin_test.go index f36f36c69..e83b2faaa 100644 --- a/internal/core/application/admin_test.go +++ b/internal/core/application/admin_test.go @@ -56,7 +56,7 @@ func TestAdminService_Settings(t *testing.T) { if seed != nil { require.NoError(t, repo.settingsRepo.Upsert(ctx, *seed, nil)) } - return application.NewAdminService(nil, repo, nil, nil, ports.UnixTime, nil) + return application.NewAdminService(nil, nil, repo, nil, nil, ports.UnixTime, nil) } t.Run("settings", func(t *testing.T) { @@ -251,7 +251,7 @@ func TestAdminService_SettingsSerialization(t *testing.T) { seed := validSettings() probe := &serializeProbeRepo{settings: &seed, delay: 10 * time.Millisecond} svc := application.NewAdminService( - nil, &mockRepoManager{settingsRepo: probe}, nil, nil, ports.UnixTime, nil, + nil, nil, &mockRepoManager{settingsRepo: probe}, nil, nil, ports.UnixTime, nil, ) const workers = 8 @@ -328,7 +328,7 @@ func (r *serializeProbeRepo) Close() {} func TestAdminService_GetRounds(t *testing.T) { ctx := context.Background() repo := &mockRepoManager{roundsRepo: &mockRoundRepository{}} - svc := application.NewAdminService(nil, repo, nil, nil, ports.UnixTime, nil) + svc := application.NewAdminService(nil, nil, repo, nil, nil, ports.UnixTime, nil) want := []domain.RoundSummary{{RoundId: "r1", FailReason: "boom", Failed: true}} rounds := repo.roundsRepo.(*mockRoundRepository) diff --git a/internal/core/application/service.go b/internal/core/application/service.go index c896dc113..d2466a9e5 100644 --- a/internal/core/application/service.go +++ b/internal/core/application/service.go @@ -73,6 +73,7 @@ type service struct { func NewService( wallet ports.WalletService, + walletFallbacks []ports.WalletService, signer ports.SignerService, repoManager ports.RepoManager, builder ports.TxBuilder, @@ -141,13 +142,19 @@ func NewService( ctx, cancel := context.WithCancel(ctx) svc := &service{ - wallet: wallet, - signer: signer, - repoManager: repoManager, - builder: builder, - cache: cache, - scanner: scanner, - sweeper: newSweeper(wallet, repoManager, builder, scheduler), + wallet: wallet, + signer: signer, + repoManager: repoManager, + builder: builder, + cache: cache, + scanner: scanner, + sweeper: newSweeper( + wallet, + walletFallbacks, + repoManager, + builder, + scheduler, + ), operatorPrvkey: operatorSigningKey, operatorPubkey: operatorSigningKey.PubKey(), forfeitsBoardingSigsChan: make(chan struct{}, 1), diff --git a/internal/core/application/sweeper.go b/internal/core/application/sweeper.go index 28f741926..1561472be 100644 --- a/internal/core/application/sweeper.go +++ b/internal/core/application/sweeper.go @@ -31,10 +31,13 @@ type sweeperTask struct { // it is responsible for sweeping batch outputs that reached the expiration date. // it also handles delaying the sweep events in case some parts of the tree are broadcasted type sweeper struct { - wallet ports.WalletService - repoManager ports.RepoManager - builder ports.TxBuilder - scheduler ports.SchedulerService + wallet ports.WalletService + // walletFallbacks are additional arkd-wallets whose batches this arkd may also + // sweep; signing is attempted with the primary wallet first, then each fallback. + walletFallbacks []ports.WalletService + repoManager ports.RepoManager + builder ports.TxBuilder + scheduler ports.SchedulerService // cache of scheduled tasks, avoid scheduling the same sweep event multiple times locker *sync.Mutex @@ -46,14 +49,64 @@ type sweeper struct { } func newSweeper( - wallet ports.WalletService, repoManager ports.RepoManager, builder ports.TxBuilder, + wallet ports.WalletService, walletFallbacks []ports.WalletService, + repoManager ports.RepoManager, builder ports.TxBuilder, scheduler ports.SchedulerService, ) *sweeper { return &sweeper{ - wallet, repoManager, builder, scheduler, &sync.Mutex{}, make(map[string]struct{}), nil, nil, + wallet: wallet, + walletFallbacks: walletFallbacks, + repoManager: repoManager, + builder: builder, + scheduler: scheduler, + locker: &sync.Mutex{}, + scheduledTasks: make(map[string]struct{}), } } +// signingWallets returns the wallets to try when signing a sweep, in order: the +// primary wallet first, then any configured fallbacks. +func (s *sweeper) signingWallets() []ports.WalletService { + return append([]ports.WalletService{s.wallet}, s.walletFallbacks...) +} + +// buildAndSignSweepTx builds the sweep transaction once (its destination and fees +// come from the primary wallet) and then attempts to sign it with each wallet in +// order, returning (txid, signedTx, nil) as soon as one succeeds. This lets a +// single arkd sweep batches signed by any of its primary/fallback arkd-wallets. +// Broadcasting is left to the caller. +// +// On signing failure it still returns the txid (with a non-nil error and an empty +// signed tx) so a reconcile-only caller can use the txid without rebuilding the +// tx. Callers that broadcast must treat a non-nil error as fatal. A build failure +// returns an empty txid. +func buildAndSignSweepTx( + builder ports.TxBuilder, wallets []ports.WalletService, inputs []ports.TxInput, +) (string, string, error) { + txid, unsignedTx, err := builder.BuildSweepTx(inputs) + if err != nil { + return "", "", err + } + + if len(wallets) == 0 { + return txid, "", fmt.Errorf("no signing wallets configured for sweep tx %s", txid) + } + + signErrs := make([]error, 0, len(wallets)) + for i, wallet := range wallets { + signed, signErr := builder.SignSweepTx(wallet, unsignedTx) + if signErr == nil { + return txid, signed, nil + } + // name the failing wallet so a multi-wallet operator can tell which rejected it + signErrs = append(signErrs, fmt.Errorf("wallet[%d]: %w", i, signErr)) + } + + return txid, "", fmt.Errorf( + "no wallet could sign sweep tx %s: %w", txid, errors.Join(signErrs...), + ) +} + func (s *sweeper) start(ctx context.Context) error { s.scheduledTasks = make(map[string]struct{}) s.scheduler.Start() @@ -636,7 +689,9 @@ func (s *sweeper) createBatchSweepTask(commitmentTxid, vtxoTreeRootTxid string) ) // build the sweep transaction with all the expired non-swept batch outputs - sweepTxId, sweepTx, err = s.builder.BuildSweepTx(unspentOutputsToSweep) + sweepTxId, sweepTx, err = buildAndSignSweepTx( + s.builder, s.signingWallets(), unspentOutputsToSweep, + ) if err != nil { return err } @@ -670,11 +725,26 @@ func (s *sweeper) createBatchSweepTask(commitmentTxid, vtxoTreeRootTxid string) } log.Debugf("sweeper: batch %s swept by: %s", commitmentTxid, txid) } else { - // if all outputs are spent, it means we missed to mark the batch as swept, - // build a sweep transaction without broadcasting it. we'll use it rebuild sweepEvent. - sweepTxId, sweepTx, err = s.builder.BuildSweepTx(outputsToSweep) + // All outputs are already spent on-chain — we just missed marking the + // batch swept. We rebuild the sweepEvent from the txid; the tx is never + // broadcast here, so a signing failure (e.g. the wallet that swept them + // is no longer primary or fallback) must not block reconciliation. + // buildAndSignSweepTx still returns the txid on signing failure, so we + // reconcile with the txid and store no tx bytes (an unsigned PSBT here + // would masquerade as a signed sweep tx in SweepTxs / the swept event). + sweepTxId, sweepTx, err = buildAndSignSweepTx( + s.builder, s.signingWallets(), outputsToSweep, + ) if err != nil { - return err + if sweepTxId == "" { + // the build itself failed; nothing to reconcile with + return err + } + log.WithError(err).Warnf( + "sweeper: could not sign sweep tx for already-spent batch %s, "+ + "reconciling with txid only", commitmentTxid, + ) + sweepTx = "" } } @@ -758,7 +828,9 @@ func (s *sweeper) createCheckpointSweepTask( checkpointTxid := toSweep.Txid log.Debugf("sweeper: start sweeping checkpoint %s", checkpointTxid) - sweepTxid, sweepTx, err := s.builder.BuildSweepTx([]ports.TxInput{toSweep}) + sweepTxid, sweepTx, err := buildAndSignSweepTx( + s.builder, s.signingWallets(), []ports.TxInput{toSweep}, + ) if err != nil { return err } diff --git a/internal/core/application/sweeper_test.go b/internal/core/application/sweeper_test.go index 5e6e1c86d..4b0e6c6dd 100644 --- a/internal/core/application/sweeper_test.go +++ b/internal/core/application/sweeper_test.go @@ -17,6 +17,99 @@ import ( "github.com/stretchr/testify/require" ) +// fakeWallet is a distinct, comparable ports.WalletService identity. Only its +// pointer identity matters for the fallback-iteration tests. +type fakeWallet struct { + ports.WalletService + name string +} + +// fakeSweepBuilder implements just the two sweep methods of ports.TxBuilder that +// buildAndSignSweepTx uses. SignSweepTx succeeds only for goodWallet, recording the +// order in which wallets are tried. +type fakeSweepBuilder struct { + ports.TxBuilder + buildErr error + unsignedTx string + txid string + goodWallet ports.WalletService + signCalls []ports.WalletService +} + +func (b *fakeSweepBuilder) BuildSweepTx(inputs []ports.TxInput) (string, string, error) { + if b.buildErr != nil { + return "", "", b.buildErr + } + return b.txid, b.unsignedTx, nil +} + +func (b *fakeSweepBuilder) SignSweepTx( + wallet ports.WalletService, unsignedTx string, +) (string, error) { + b.signCalls = append(b.signCalls, wallet) + if b.goodWallet != nil && wallet == b.goodWallet { + return "signed:" + unsignedTx, nil + } + return "", fmt.Errorf("wallet %v cannot sign", wallet) +} + +func TestBuildAndSignSweepTx(t *testing.T) { + inputs := []ports.TxInput{{Txid: "aa", Index: 0}} + primary := &fakeWallet{name: "primary"} + fb1 := &fakeWallet{name: "fb1"} + fb2 := &fakeWallet{name: "fb2"} + wallets := []ports.WalletService{primary, fb1, fb2} + + t.Run("primary signs, fallbacks not tried", func(t *testing.T) { + b := &fakeSweepBuilder{unsignedTx: "unsigned", txid: "txid123", goodWallet: primary} + + txid, signed, err := buildAndSignSweepTx(b, wallets, inputs) + require.NoError(t, err) + require.Equal(t, "txid123", txid) + require.Equal(t, "signed:unsigned", signed) + require.Equal(t, []ports.WalletService{primary}, b.signCalls) + }) + + t.Run("falls back to a later wallet in order", func(t *testing.T) { + b := &fakeSweepBuilder{unsignedTx: "unsigned", txid: "txid123", goodWallet: fb2} + + txid, signed, err := buildAndSignSweepTx(b, wallets, inputs) + require.NoError(t, err) + require.Equal(t, "txid123", txid) + require.Equal(t, "signed:unsigned", signed) + require.Equal(t, []ports.WalletService{primary, fb1, fb2}, b.signCalls) + }) + + t.Run("no wallet can sign returns aggregated error naming each wallet", func(t *testing.T) { + b := &fakeSweepBuilder{unsignedTx: "unsigned", txid: "txid123"} + + txid, signed, err := buildAndSignSweepTx(b, wallets, inputs) + require.Error(t, err) + require.Equal(t, "txid123", txid) + require.Empty(t, signed) + require.Contains(t, err.Error(), "no wallet could sign sweep tx txid123") + require.Contains(t, err.Error(), "wallet[0]") + require.Contains(t, err.Error(), "wallet[2]") + require.Len(t, b.signCalls, 3) + }) + + t.Run("no signing wallets configured", func(t *testing.T) { + b := &fakeSweepBuilder{unsignedTx: "unsigned", txid: "txid123"} + + _, _, err := buildAndSignSweepTx(b, nil, inputs) + require.ErrorContains(t, err, "no signing wallets configured for sweep tx txid123") + require.Empty(t, b.signCalls) + }) + + t.Run("build error short-circuits before signing", func(t *testing.T) { + b := &fakeSweepBuilder{buildErr: fmt.Errorf("boom")} + + _, _, err := buildAndSignSweepTx(b, wallets, inputs) + require.ErrorContains(t, err, "boom") + require.Empty(t, b.signCalls) + }) +} + func TestCreateCheckpointSweepTask(t *testing.T) { // checkpoint sweeps use per-outpoint sweeping (SweepVtxoOutpoints) instead of // marker-based sweeping. This prevents over-reach when markers are shared @@ -591,6 +684,23 @@ func (m *mockTxBuilder) BuildSweepTx(inputs []ports.TxInput) (string, string, er return args.String(0), args.String(1), args.Error(2) } +// SignSweepTx came in with the build/sign split. Tests that only prime +// BuildSweepTx are exercising paths where the unsigned tx is enough, so this +// hands the tx back unchanged unless a case primes it. +func (m *mockTxBuilder) SignSweepTx( + wallet ports.WalletService, unsignedTx string, +) (string, error) { + if len(m.ExpectedCalls) > 0 { + for _, c := range m.ExpectedCalls { + if c.Method == "SignSweepTx" { + args := m.Called(wallet, unsignedTx) + return args.String(0), args.Error(1) + } + } + } + return unsignedTx, nil +} + // Stub implementations for unused TxBuilder methods func (m *mockTxBuilder) BuildCommitmentTx( signerPubkey *btcec.PublicKey, intents domain.Intents, @@ -682,6 +792,6 @@ func newTestSweeper(t *testing.T) ( repoManager := &mockRepoManager{vtxos: vtxoRepo, markers: markerRepo} builder := &mockTxBuilder{} scheduler := &mockScheduler{} - s := newSweeper(wallet, repoManager, builder, scheduler) + s := newSweeper(wallet, nil, repoManager, builder, scheduler) return wallet, vtxoRepo, markerRepo, builder, s } diff --git a/internal/core/ports/tx_builder.go b/internal/core/ports/tx_builder.go index ccbe61199..9fad06672 100644 --- a/internal/core/ports/tx_builder.go +++ b/internal/core/ports/tx_builder.go @@ -60,7 +60,12 @@ type TxBuilder interface { VerifyForfeitTxs( vtxos []domain.Vtxo, connectors tree.FlatTxTree, txs []string, ) (valid map[domain.Outpoint]ValidForfeitTx, err error) - BuildSweepTx(inputs []TxInput) (txid string, signedSweepTx string, err error) + // BuildSweepTx builds the unsigned sweep tx, using the primary wallet for the + // destination address and fees. + BuildSweepTx(inputs []TxInput) (txid string, unsignedTx string, err error) + // SignSweepTx signs an unsigned sweep tx with the given wallet, so it can be + // signed by any primary/fallback wallet without rebuilding it. + SignSweepTx(wallet WalletService, unsignedTx string) (signedTx string, err error) GetSweepableBatchOutputs(vtxoTree *tree.TxTree) ( vtxoTreeExpiry *arklib.RelativeLocktime, batchOutputs *TxInput, err error, ) diff --git a/internal/infrastructure/live-store/live_store_test.go b/internal/infrastructure/live-store/live_store_test.go index aa62cab3a..ff725d5a0 100644 --- a/internal/infrastructure/live-store/live_store_test.go +++ b/internal/infrastructure/live-store/live_store_test.go @@ -1064,13 +1064,20 @@ func (m *mockedTxBuilder) BuildCommitmentTx( func (m *mockedTxBuilder) BuildSweepTx( inputs []ports.TxInput, -) (txid string, signedSweepTx string, err error) { +) (txid string, unsignedTx string, err error) { args := m.Called(inputs) res0 := args.Get(0).(string) res1 := args.Get(1).(string) return res0, res1, args.Error(2) } +func (m *mockedTxBuilder) SignSweepTx( + wallet ports.WalletService, unsignedTx string, +) (signedTx string, err error) { + args := m.Called(wallet, unsignedTx) + return args.Get(0).(string), args.Error(1) +} + func (m *mockedTxBuilder) GetSweepableBatchOutputs( vtxoTree *tree.TxTree, ) (vtxoTreeExpiry *arklib.RelativeLocktime, sweepInput *ports.TxInput, err error) { diff --git a/internal/infrastructure/tx-builder/covenantless/builder.go b/internal/infrastructure/tx-builder/covenantless/builder.go index 39c1c2dba..e5201c3c5 100644 --- a/internal/infrastructure/tx-builder/covenantless/builder.go +++ b/internal/infrastructure/tx-builder/covenantless/builder.go @@ -276,10 +276,17 @@ func (b *txBuilder) FinalizeAndExtract(tx string) (string, error) { } func (b *txBuilder) BuildSweepTx(inputs []ports.TxInput) ( - txid, signedSweepTx string, err error, + txid, unsignedTx string, err error, ) { ctx := context.Background() - return sweepTransaction(ctx, b.wallet, inputs) + return buildSweepTransaction(ctx, b.wallet, inputs) +} + +func (b *txBuilder) SignSweepTx( + wallet ports.WalletService, unsignedTx string, +) (signedTx string, err error) { + ctx := context.Background() + return signSweepTransaction(ctx, wallet, unsignedTx) } func (b *txBuilder) VerifyForfeitTxs( diff --git a/internal/infrastructure/tx-builder/covenantless/sweep.go b/internal/infrastructure/tx-builder/covenantless/sweep.go index dc34cd290..4ae0d4c7d 100644 --- a/internal/infrastructure/tx-builder/covenantless/sweep.go +++ b/internal/infrastructure/tx-builder/covenantless/sweep.go @@ -4,6 +4,7 @@ import ( "context" "encoding/hex" "fmt" + "strings" "github.com/arkade-os/arkd/internal/core/ports" arklib "github.com/arkade-os/arkd/pkg/ark-lib" @@ -17,9 +18,13 @@ import ( "github.com/btcsuite/btcd/wire" ) -func sweepTransaction( +// buildSweepTransaction builds the unsigned sweep transaction. The destination +// address, fee estimate and dust limit are all taken from the given wallet (the +// primary wallet) so every signing candidate sweeps to the same output and the +// returned txid is stable regardless of which wallet ends up signing it. +func buildSweepTransaction( ctx context.Context, wallet ports.WalletService, inputs []ports.TxInput, -) (txid string, txhex string, err error) { +) (txid string, unsignedTx string, err error) { ins := make([]*wire.OutPoint, 0) sequences := make([]uint32, 0) @@ -73,8 +78,6 @@ func sweepTransaction( amount := int64(0) - tapscriptInputIndexes := make([]int, 0) - for i, input := range inputs { if input.TapscriptLeaf != nil { tapscriptBytes, err := hex.DecodeString(input.TapscriptLeaf.Tapscript) @@ -107,8 +110,6 @@ func sweepTransaction( ptx.Inputs[i].TaprootInternalKey = schnorr.SerializePubKey( internalKey, ) - - tapscriptInputIndexes = append(tapscriptInputIndexes, i) } inputAmount := int64(input.Value) @@ -139,14 +140,14 @@ func sweepTransaction( return "", "", err } - script, err := txscript.PayToAddrScript(addr) + pkScript, err := txscript.PayToAddrScript(addr) if err != nil { return "", "", err } ptx.UnsignedTx.AddTxOut(&wire.TxOut{ Value: amount, - PkScript: script, + PkScript: pkScript, }) ptx.Outputs = append(ptx.Outputs, psbt.POutput{}) @@ -176,26 +177,39 @@ func sweepTransaction( ptx.UnsignedTx.TxOut[0].Value = amount - int64(fees) - sweepPsbtBase64, err := ptx.B64Encode() + unsignedTx, err = ptx.B64Encode() if err != nil { return "", "", err } - if len(tapscriptInputIndexes) > 0 { - sweepPsbtBase64, err = wallet.SignTransactionTapscript( - ctx, - sweepPsbtBase64, - tapscriptInputIndexes, - ) - if err != nil { - return "", "", err + return ptx.UnsignedTx.TxID(), unsignedTx, nil +} + +// signSweepTransaction signs the unsigned sweep transaction with the given wallet +// The tapscript inputs that need signing are re-derived from the psbt, so the caller +// only has to pass the unsigned tx. +func signSweepTransaction( + ctx context.Context, wallet ports.WalletService, unsignedTx string, +) (string, error) { + ptx, err := psbt.NewFromRawBytes(strings.NewReader(unsignedTx), true) + if err != nil { + return "", err + } + + tapscriptInputIndexes := make([]int, 0) + for i, in := range ptx.Inputs { + if len(in.TaprootLeafScript) > 0 { + tapscriptInputIndexes = append(tapscriptInputIndexes, i) } } - signedTxHex, err := wallet.SignTransaction(ctx, sweepPsbtBase64, true) - if err != nil { - return "", "", err + signedTx := unsignedTx + if len(tapscriptInputIndexes) > 0 { + signedTx, err = wallet.SignTransactionTapscript(ctx, signedTx, tapscriptInputIndexes) + if err != nil { + return "", err + } } - return ptx.UnsignedTx.TxID(), signedTxHex, nil + return wallet.SignTransaction(ctx, signedTx, true) } diff --git a/internal/infrastructure/tx-builder/covenantless/sweep_test.go b/internal/infrastructure/tx-builder/covenantless/sweep_test.go new file mode 100644 index 000000000..ca57ec1c9 --- /dev/null +++ b/internal/infrastructure/tx-builder/covenantless/sweep_test.go @@ -0,0 +1,106 @@ +package txbuilder + +import ( + "context" + "encoding/hex" + "testing" + + "github.com/arkade-os/arkd/internal/core/ports" + "github.com/btcsuite/btcd/btcutil/psbt" + "github.com/btcsuite/btcd/chaincfg/chainhash" + "github.com/btcsuite/btcd/txscript" + "github.com/btcsuite/btcd/wire" + "github.com/stretchr/testify/require" +) + +// recordingWallet captures the arguments signSweepTransaction passes to the wallet. +type recordingWallet struct { + ports.WalletService + gotTapscriptIndexes []int + gotSignInput string + tapscriptSigned string + finalSigned string +} + +func (w *recordingWallet) SignTransactionTapscript( + _ context.Context, partialTx string, inputIndexes []int, +) (string, error) { + w.gotTapscriptIndexes = inputIndexes + return w.tapscriptSigned, nil +} + +func (w *recordingWallet) SignTransaction( + _ context.Context, partialTx string, _ bool, +) (string, error) { + w.gotSignInput = partialTx + return w.finalSigned, nil +} + +// TestSignSweepTransaction_DerivesTapscriptIndexes verifies that signing re-derives +// the tapscript input indexes from the psbt (the inputs carrying a TaprootLeafScript) +// rather than relying on indexes threaded through the builder API. +func TestSignSweepTransaction_DerivesTapscriptIndexes(t *testing.T) { + op0 := &wire.OutPoint{Hash: chainhash.Hash{0x01}, Index: 0} + op1 := &wire.OutPoint{Hash: chainhash.Hash{0x02}, Index: 0} + op2 := &wire.OutPoint{Hash: chainhash.Hash{0x03}, Index: 0} + + ptx, err := psbt.New( + []*wire.OutPoint{op0, op1, op2}, nil, 2, 0, + []uint32{wire.MaxTxInSequenceNum, wire.MaxTxInSequenceNum, wire.MaxTxInSequenceNum}, + ) + require.NoError(t, err) + + // a valid control block is a leaf-version/parity byte + a valid 32-byte x-only + // internal key (here secp256k1's generator G) + an optional merkle path; psbt + // deserialization parses and validates the internal key. + internalKey, err := hex.DecodeString( + "79be667ef9dcbbac55a06295ce870b07029bfcdb2dce28d959f2815b16f81798", + ) + require.NoError(t, err) + controlBlock := append([]byte{byte(txscript.BaseLeafVersion)}, internalKey...) + + // only inputs 1 and 2 carry a taproot leaf script + for _, i := range []int{1, 2} { + ptx.Inputs[i].TaprootLeafScript = []*psbt.TaprootTapLeafScript{{ + ControlBlock: controlBlock, + Script: []byte{0x04, 0x05}, + LeafVersion: txscript.BaseLeafVersion, + }} + } + + unsignedTx, err := ptx.B64Encode() + require.NoError(t, err) + + w := &recordingWallet{tapscriptSigned: "tapscript-signed-psbt", finalSigned: "final-raw-tx"} + + out, err := signSweepTransaction(context.Background(), w, unsignedTx) + require.NoError(t, err) + + require.Equal(t, "final-raw-tx", out) + require.Equal(t, []int{1, 2}, w.gotTapscriptIndexes) + // the tapscript-signed psbt must be the one forwarded to SignTransaction + require.Equal(t, "tapscript-signed-psbt", w.gotSignInput) +} + +// TestSignSweepTransaction_NoTapscriptInputs verifies that with no tapscript inputs, +// SignTransactionTapscript is skipped and the unsigned psbt goes straight to signing. +func TestSignSweepTransaction_NoTapscriptInputs(t *testing.T) { + op0 := &wire.OutPoint{Hash: chainhash.Hash{0x01}, Index: 0} + ptx, err := psbt.New( + []*wire.OutPoint{op0}, nil, 2, 0, []uint32{wire.MaxTxInSequenceNum}, + ) + require.NoError(t, err) + + unsignedTx, err := ptx.B64Encode() + require.NoError(t, err) + + w := &recordingWallet{tapscriptSigned: "should-not-be-used", finalSigned: "final-raw-tx"} + + out, err := signSweepTransaction(context.Background(), w, unsignedTx) + require.NoError(t, err) + + require.Equal(t, "final-raw-tx", out) + require.Nil(t, w.gotTapscriptIndexes) + // with no tapscript inputs, the unsigned psbt is forwarded unchanged + require.Equal(t, unsignedTx, w.gotSignInput) +}