From 4b316d97c8688a2754f1435d21e90a9ccfa94e48 Mon Sep 17 00:00:00 2001 From: Olaoluwa Osuntokun Date: Sat, 17 Nov 2018 21:06:59 -0800 Subject: [PATCH] lnwallet: add new WithCoinSelectLock method to fix coin select race conditions In this commit, we add a new method WithCoinSelectLock. This method will allow us to fix bugs in the project atm that can arise if a channel funding is attempted (either manually or by autopilot) while a users is attempting to send an on-chain transaction. If this happens concurrently, then both contexts will grab the set of UTXOs and attempt to lock them one by one. However, since they didn't obtain an exclusive snapshot of the UTXO set of the wallet, they may both attempt to lock the same input. We also ensure that calls to SendMany cannot run into this issue by using the WithCoinSelectLock synchronization when attempting to instruct the internal wallet to send payments. --- lnwallet/wallet.go | 11 +++++++++++ rpcserver.go | 19 ++++++++++++++++++- 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index 6f7863dc5..2dabf056d 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -1252,6 +1252,17 @@ func (l *LightningWallet) handleSingleFunderSigs(req *addSingleFunderSigsMsg) { l.limboMtx.Unlock() } +// WithCoinSelectLock will execute the passed function closure in a +// synchronized manner preventing any coin selection operations from proceeding +// while the closure if executing. This can be seen as the ability to execute a +// function closure under an exclusive coin selection lock. +func (l *LightningWallet) WithCoinSelectLock(f func() error) error { + l.coinSelectMtx.Lock() + defer l.coinSelectMtx.Unlock() + + return f() +} + // selectCoinsAndChange performs coin selection in order to obtain witness // outputs which sum to at least 'numCoins' amount of satoshis. If coin // selection is successful/possible, then the selected coins are available diff --git a/rpcserver.go b/rpcserver.go index dfd3e8905..a49fea0a4 100644 --- a/rpcserver.go +++ b/rpcserver.go @@ -804,7 +804,24 @@ func (r *rpcServer) SendMany(ctx context.Context, rpcsLog.Infof("[sendmany] outputs=%v, sat/kw=%v", spew.Sdump(in.AddrToAmount), int64(feePerKw)) - txid, err := r.sendCoinsOnChain(in.AddrToAmount, feePerKw) + var txid *chainhash.Hash + + // We'll attempt to send to the target set of outputs, ensuring that we + // synchronize with any other ongoing coin selection attempts which + // happen to also be concurrently executing. + wallet := r.server.cc.wallet + err = wallet.WithCoinSelectLock(func() error { + sendManyTXID, err := r.sendCoinsOnChain( + in.AddrToAmount, feePerKw, + ) + if err != nil { + return err + } + + txid = sendManyTXID + + return nil + }) if err != nil { return nil, err }