From 29602e88e8114f28338c1f3bef908dab62e4fec1 Mon Sep 17 00:00:00 2001 From: Joost Jager Date: Fri, 4 Sep 2020 11:19:27 +0200 Subject: [PATCH] cnct: cpfp-sweep anchors For unconfirmed commit tx anchors, supply the sweeper with cpfp info and a confirmation target fee estimate. The sweeper will try to pay for the parent commit tx as long as the current fee estimate exceeds the pre-signed commit tx fee rate. --- contractcourt/anchor_resolver.go | 3 + contractcourt/channel_arbitrator.go | 29 +++++--- lntest/itest/lnd_channel_backup_test.go | 4 ++ ...d_multi-hop_htlc_local_chain_claim_test.go | 4 ++ .../lnd_multi-hop_htlc_local_timeout_test.go | 4 ++ ...ulti-hop_htlc_receiver_chain_claim_test.go | 4 ++ ..._multi-hop_htlc_remote_chain_claim_test.go | 4 ++ ..._force_close_on_chain_htlc_timeout_test.go | 4 ++ ..._force_close_on_chain_htlc_timeout_test.go | 4 ++ lntest/itest/lnd_multi-hop_test.go | 4 ++ lntest/itest/lnd_test.go | 67 +++++++++++++++++-- lnwallet/channel.go | 21 ++++++ 12 files changed, 136 insertions(+), 16 deletions(-) diff --git a/contractcourt/anchor_resolver.go b/contractcourt/anchor_resolver.go index ac67173f..98a4fcca 100644 --- a/contractcourt/anchor_resolver.go +++ b/contractcourt/anchor_resolver.go @@ -89,6 +89,9 @@ func (c *anchorResolver) Resolve() (ContractResolver, error) { // An exclusive group is not necessary anymore, because we know that // this is the only anchor that can be swept. // + // We also clear the parent tx information for cpfp, because the + // commitment tx is confirmed. + // // After a restart or when the remote force closes, the sweeper is not // yet aware of the anchor. In that case, it will be added as new input // to the sweeper. diff --git a/contractcourt/channel_arbitrator.go b/contractcourt/channel_arbitrator.go index 46bb61ba..2d50ca1d 100644 --- a/contractcourt/channel_arbitrator.go +++ b/contractcourt/channel_arbitrator.go @@ -30,6 +30,12 @@ var ( "process of being force closed") ) +const ( + // anchorSweepConfTarget is the conf target used when sweeping + // commitment anchors. + anchorSweepConfTarget = 6 +) + // WitnessSubscription represents an intent to be notified once new witnesses // are discovered by various active contract resolvers. A contract resolver may // use this to be notified of when it can satisfy an incoming contract after we @@ -1060,9 +1066,6 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution, // anchors from being batched together. exclusiveGroup := c.cfg.ShortChanID.ToUint64() - // Retrieve the current minimum fee rate from the sweeper. - minFeeRate := c.cfg.Sweeper.RelayFeePerKW() - for _, anchor := range anchors { log.Debugf("ChannelArbitrator(%v): pre-confirmation sweep of "+ "anchor of tx %v", c.cfg.ChanPoint, anchor.CommitAnchor) @@ -1073,19 +1076,25 @@ func (c *ChannelArbitrator) sweepAnchors(anchors []*lnwallet.AnchorResolution, input.CommitmentAnchor, &anchor.AnchorSignDescriptor, heightHint, - nil, + &input.TxInfo{ + Fee: anchor.CommitFee, + Weight: anchor.CommitWeight, + }, ) - // Sweep anchor output with the minimum fee rate. This usually - // (up to a min relay fee of 3 sat/b) means that the anchor - // sweep will be economical. Also signal that this is a force - // sweep. If the user decides to bump the fee on the anchor - // sweep, it will be swept even if it isn't economical. + // Sweep anchor output with a confirmation target fee + // preference. Because this is a cpfp-operation, the anchor will + // only be attempted to sweep when the current fee estimate for + // the confirmation target exceeds the commit fee rate. + // + // Also signal that this is a force sweep, so that the anchor + // will be swept even if it isn't economical purely based on the + // anchor value. _, err := c.cfg.Sweeper.SweepInput( &anchorInput, sweep.Params{ Fee: sweep.FeePreference{ - FeeRate: minFeeRate, + ConfTarget: anchorSweepConfTarget, }, Force: true, ExclusiveGroup: &exclusiveGroup, diff --git a/lntest/itest/lnd_channel_backup_test.go b/lntest/itest/lnd_channel_backup_test.go index 9a791463..a55228cc 100644 --- a/lntest/itest/lnd_channel_backup_test.go +++ b/lntest/itest/lnd_channel_backup_test.go @@ -368,6 +368,10 @@ func testChannelBackupRestore(net *lntest.NetworkHarness, t *harnessTest) { for _, testCase := range testCases { success := t.t.Run(testCase.name, func(t *testing.T) { h := newHarnessTest(t, net) + + // Start each test with the default static fee estimate. + net.SetFeeEstimate(12500) + testChanRestoreScenario(h, net, &testCase, password) }) if !success { diff --git a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go index 0bab9b41..256a8afe 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_chain_claim_test.go @@ -96,6 +96,10 @@ func testMultiHopHtlcLocalChainClaim(net *lntest.NetworkHarness, t *harnessTest, // hop logic. waitForInvoiceAccepted(t, carol, payHash) + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // At this point, Bob decides that he wants to exit the channel // immediately, so he force closes his commitment transaction. ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) diff --git a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go index 03320f08..67b13b56 100644 --- a/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_local_timeout_test.go @@ -105,6 +105,10 @@ func testMultiHopHtlcLocalTimeout(net *lntest.NetworkHarness, t *harnessTest, t.Fatalf("htlc mismatch: %v", predErr) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // We'll now mine enough blocks to trigger Bob's broadcast of his // commitment transaction due to the fact that the HTLC is about to // timeout. With the default outgoing broadcast delta of zero, this will diff --git a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go index b157f4f0..a19e51d1 100644 --- a/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_receiver_chain_claim_test.go @@ -117,6 +117,10 @@ func testMultiHopReceiverChainClaim(net *lntest.NetworkHarness, t *harnessTest, t.Fatalf("settle invoice: %v", err) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // Now we'll mine enough blocks to prompt carol to actually go to the // chain in order to sweep her HTLC since the value is high enough. // TODO(roasbeef): modify once go to chain policy changes diff --git a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go index 5ea1c4a1..7e3fe616 100644 --- a/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go +++ b/lntest/itest/lnd_multi-hop_htlc_remote_chain_claim_test.go @@ -96,6 +96,10 @@ func testMultiHopHtlcRemoteChainClaim(net *lntest.NetworkHarness, t *harnessTest // hop logic. waitForInvoiceAccepted(t, carol, payHash) + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // Next, Alice decides that she wants to exit the channel, so she'll // immediately force close the channel by broadcast her commitment // transaction. diff --git a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go index 576295ec..a2a97269 100644 --- a/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_local_force_close_on_chain_htlc_timeout_test.go @@ -79,6 +79,10 @@ func testMultiHopLocalForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, t.Fatalf("htlc mismatch: %v", err) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // Now that all parties have the HTLC locked in, we'll immediately // force close the Bob -> Carol channel. This should trigger contract // resolution mode for both of them. diff --git a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go index 9d30a2dc..90fc24eb 100644 --- a/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go +++ b/lntest/itest/lnd_multi-hop_remote_force_close_on_chain_htlc_timeout_test.go @@ -80,6 +80,10 @@ func testMultiHopRemoteForceCloseOnChainHtlcTimeout(net *lntest.NetworkHarness, t.Fatalf("htlc mismatch: %v", predErr) } + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // At this point, we'll now instruct Carol to force close the // transaction. This will let us exercise that Bob is able to sweep the // expired HTLC on Carol's version of the commitment transaction. If diff --git a/lntest/itest/lnd_multi-hop_test.go b/lntest/itest/lnd_multi-hop_test.go index ec73e187..100a5842 100644 --- a/lntest/itest/lnd_multi-hop_test.go +++ b/lntest/itest/lnd_multi-hop_test.go @@ -101,6 +101,10 @@ func testMultiHopHtlcClaims(net *lntest.NetworkHarness, t *harnessTest) { success := ht.t.Run(subTest.name, func(t *testing.T) { ht := newHarnessTest(t, net) + // Start each test with the default + // static fee estimate. + net.SetFeeEstimate(12500) + subTest.test(net, ht, alice, bob, commitType) }) if !success { diff --git a/lntest/itest/lnd_test.go b/lntest/itest/lnd_test.go index b5a532a8..010b7c43 100644 --- a/lntest/itest/lnd_test.go +++ b/lntest/itest/lnd_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -3460,6 +3461,9 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, numInvoices = 6 ) + const commitFeeRate = 20000 + net.SetFeeEstimate(commitFeeRate) + // TODO(roasbeef): should check default value in config here // instead, or make delay a param defaultCLTV := uint32(lnd.DefaultBitcoinTimeLockDelta) @@ -3574,6 +3578,9 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, // execute a force closure of the channel. This will also assert that // the commitment transaction was immediately broadcast in order to // fulfill the force closure request. + const actualFeeRate = 30000 + net.SetFeeEstimate(actualFeeRate) + ctxt, _ = context.WithTimeout(ctxb, channelCloseTimeout) _, closingTxID, err := net.CloseChannel(ctxt, alice, chanPoint, true) if err != nil { @@ -3635,17 +3642,35 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, // broadcast as a result of the force closure. If there are anchors, we // also expect the anchor sweep tx to be in the mempool. expectedTxes := 1 + expectedFeeRate := commitFeeRate if channelType == commitTypeAnchors { expectedTxes = 2 + expectedFeeRate = actualFeeRate } - sweepTxns, err := waitForNTxsInMempool( + sweepTxns, err := getNTxsFromMempool( net.Miner.Node, expectedTxes, minerMempoolTimeout, ) if err != nil { t.Fatalf("failed to find commitment in miner mempool: %v", err) } + // Verify fee rate of the commitment tx plus anchor if present. + var totalWeight, totalFee int64 + for _, tx := range sweepTxns { + utx := btcutil.NewTx(tx) + totalWeight += blockchain.GetTransactionWeight(utx) + + fee, err := getTxFee(net.Miner.Node, tx) + require.NoError(t.t, err) + totalFee += int64(fee) + } + feeRate := totalFee * 1000 / totalWeight + + // Allow some deviation because weight estimates during tx generation + // are estimates. + require.InEpsilon(t.t, expectedFeeRate, feeRate, 0.005) + // Find alice's commit sweep and anchor sweep (if present) in the // mempool. aliceCloseTx := waitingClose.Commitments.LocalTxid @@ -3737,7 +3762,7 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest, // Carol's sweep tx should be in the mempool already, as her output is // not timelocked. If there are anchors, we also expect Carol's anchor // sweep now. - sweepTxns, err = waitForNTxsInMempool( + sweepTxns, err = getNTxsFromMempool( net.Miner.Node, expectedTxes, minerMempoolTimeout, ) if err != nil { @@ -4416,12 +4441,13 @@ type sweptOutput struct { // we have to bring another input to add fees to the anchor. Note that the // anchor swept output may be nil if the channel did not have anchors. func findCommitAndAnchor(t *harnessTest, net *lntest.NetworkHarness, - sweepTxns []*chainhash.Hash, closeTx string) (*sweptOutput, *sweptOutput) { + sweepTxns []*wire.MsgTx, closeTx string) (*sweptOutput, *sweptOutput) { var commitSweep, anchorSweep *sweptOutput for _, tx := range sweepTxns { - sweepTx, err := net.Miner.Node.GetRawTransaction(tx) + txHash := tx.TxHash() + sweepTx, err := net.Miner.Node.GetRawTransaction(&txHash) require.NoError(t.t, err) // We expect our commitment sweep to have a single input, and, @@ -4434,7 +4460,7 @@ func findCommitAndAnchor(t *harnessTest, net *lntest.NetworkHarness, if len(inputs) == 1 { commitSweep = &sweptOutput{ OutPoint: inputs[0].PreviousOutPoint, - SweepTx: tx.String(), + SweepTx: txHash.String(), } } else { // Since we have more than one input, we run through @@ -4445,7 +4471,7 @@ func findCommitAndAnchor(t *harnessTest, net *lntest.NetworkHarness, if outpointStr == closeTx { anchorSweep = &sweptOutput{ OutPoint: txin.PreviousOutPoint, - SweepTx: tx.String(), + SweepTx: txHash.String(), } } } @@ -7574,6 +7600,28 @@ func getNTxsFromMempool(miner *rpcclient.Client, n int, return txes, nil } +// getTxFee retrieves parent transactions and reconstructs the fee paid. +func getTxFee(miner *rpcclient.Client, tx *wire.MsgTx) (btcutil.Amount, error) { + var balance btcutil.Amount + for _, in := range tx.TxIn { + parentHash := in.PreviousOutPoint.Hash + rawTx, err := miner.GetRawTransaction(&parentHash) + if err != nil { + return 0, err + } + parent := rawTx.MsgTx() + balance += btcutil.Amount( + parent.TxOut[in.PreviousOutPoint.Index].Value, + ) + } + + for _, out := range tx.TxOut { + balance -= btcutil.Amount(out.Value) + } + + return balance, nil +} + // testFailingChannel tests that we will fail the channel by force closing ii // in the case where a counterparty tries to settle an HTLC with the wrong // preimage. @@ -9423,6 +9471,10 @@ func assertDLPExecuted(net *lntest.NetworkHarness, t *harnessTest, dave *lntest.HarnessNode, daveStartingBalance int64, anchors bool) { + // Increase the fee estimate so that the following force close tx will + // be cpfp'ed. + net.SetFeeEstimate(30000) + // We disabled auto-reconnect for some tests to avoid timing issues. // To make sure the nodes are initiating DLP now, we have to manually // re-connect them. @@ -14527,6 +14579,9 @@ func TestLightningNetworkDaemon(t *testing.T) { t.Fatalf("unable to add to log: %v", err) } + // Start every test with the default static fee estimate. + lndHarness.SetFeeEstimate(12500) + success := t.Run(testCase.name, func(t1 *testing.T) { ht := newHarnessTest(t1, lndHarness) ht.RunTestCase(testCase) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index ccdf5530..fdfda736 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -5961,6 +5961,12 @@ type AnchorResolution struct { // CommitAnchor is the anchor outpoint on the commit tx. CommitAnchor wire.OutPoint + + // CommitFee is the fee of the commit tx. + CommitFee btcutil.Amount + + // CommitWeight is the weight of the commit tx. + CommitWeight int64 } // LocalForceCloseSummary describes the final commitment state before the @@ -6399,9 +6405,24 @@ func NewAnchorResolution(chanState *channeldb.OpenChannel, HashType: txscript.SigHashAll, } + // Calculate commit tx weight. This commit tx doesn't yet include the + // witness spending the funding output, so we add the (worst case) + // weight for that too. + utx := btcutil.NewTx(commitTx) + weight := blockchain.GetTransactionWeight(utx) + + input.WitnessCommitmentTxWeight + + // Calculate commit tx fee. + fee := chanState.Capacity + for _, out := range commitTx.TxOut { + fee -= btcutil.Amount(out.Value) + } + return &AnchorResolution{ CommitAnchor: *outPoint, AnchorSignDescriptor: *signDesc, + CommitWeight: weight, + CommitFee: fee, }, nil }