From feb8e7e64e276dc3ea24b0c30a32fcfbd7caa628 Mon Sep 17 00:00:00 2001 From: eugene Date: Thu, 5 Jan 2023 17:38:38 -0500 Subject: [PATCH 1/4] aliasmgr: export StartingAlias so other packages can use it --- aliasmgr/aliasmgr.go | 8 ++++---- aliasmgr/aliasmgr_test.go | 12 ++++++------ 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/aliasmgr/aliasmgr.go b/aliasmgr/aliasmgr.go index 67d4840998..381476a738 100644 --- a/aliasmgr/aliasmgr.go +++ b/aliasmgr/aliasmgr.go @@ -53,10 +53,10 @@ var ( // endBlockHeight is the ending block height of the alias range. endBlockHeight = 16_250_000 - // startingAlias is the first alias ShortChannelID that will get + // StartingAlias is the first alias ShortChannelID that will get // assigned by RequestAlias. The starting BlockHeight is chosen so that // legitimate SCIDs in integration tests aren't mistaken for an alias. - startingAlias = lnwire.ShortChannelID{ + StartingAlias = lnwire.ShortChannelID{ BlockHeight: uint32(startingBlockHeight), TxIndex: 0, TxPosition: 0, @@ -401,8 +401,8 @@ func (m *Manager) RequestAlias() (lnwire.ShortChannelID, error) { lastBytes := bucket.Get(lastAliasKey) if lastBytes == nil { // If the key does not exist, then we can write the - // startingAlias to it. - nextAlias = startingAlias + // StartingAlias to it. + nextAlias = StartingAlias var scratch [8]byte byteOrder.PutUint64(scratch[:], nextAlias.ToUint64()) diff --git a/aliasmgr/aliasmgr_test.go b/aliasmgr/aliasmgr_test.go index 9ae7f82df9..17159ed877 100644 --- a/aliasmgr/aliasmgr_test.go +++ b/aliasmgr/aliasmgr_test.go @@ -32,12 +32,12 @@ func TestAliasStorePeerAlias(t *testing.T) { // Test that we can put the (chanID, alias) mapping in the database. // Also check that we retrieve exactly what we put in. - err = aliasStore.PutPeerAlias(chanID1, startingAlias) + err = aliasStore.PutPeerAlias(chanID1, StartingAlias) require.NoError(t, err) storedAlias, err := aliasStore.GetPeerAlias(chanID1) require.NoError(t, err) - require.Equal(t, startingAlias, storedAlias) + require.Equal(t, StartingAlias, storedAlias) } // TestAliasStoreRequest tests that the aliasStore delivers the expected SCID. @@ -55,12 +55,12 @@ func TestAliasStoreRequest(t *testing.T) { aliasStore, err := NewManager(db) require.NoError(t, err) - // We'll assert that the very first alias we receive is startingAlias. + // We'll assert that the very first alias we receive is StartingAlias. alias1, err := aliasStore.RequestAlias() require.NoError(t, err) - require.Equal(t, startingAlias, alias1) + require.Equal(t, StartingAlias, alias1) - // The next alias should be the result of passing in startingAlias to + // The next alias should be the result of passing in StartingAlias to // getNextScid. nextAlias := getNextScid(alias1) alias2, err := aliasStore.RequestAlias() @@ -78,7 +78,7 @@ func TestGetNextScid(t *testing.T) { }{ { name: "starting alias", - current: startingAlias, + current: StartingAlias, expected: lnwire.ShortChannelID{ BlockHeight: uint32(startingBlockHeight), TxIndex: 0, From d807ff23d24444d6c11894a42b31ab38a8352a53 Mon Sep 17 00:00:00 2001 From: eugene Date: Thu, 5 Jan 2023 17:38:59 -0500 Subject: [PATCH 2/4] channeldb: fix DisconnectBlockAtHeight bug for zero-conf channels When a block is disconnected due to a reorg, DisconnectBlockAtHeight is called for the block height. Prior to this patch, it would delete every SCID in the graph with a block height greater than the disconnected height. This meant that a reorg would delete every zero-conf channel edge from the graph. The fix simply iterates up until the StartingAlias and deletes every SCID between the disconnected height and the StartingAlias height. --- channeldb/graph.go | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/channeldb/graph.go b/channeldb/graph.go index 957c0b828d..1e6dbf8d62 100644 --- a/channeldb/graph.go +++ b/channeldb/graph.go @@ -20,6 +20,7 @@ import ( "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" + "github.com/lightningnetwork/lnd/aliasmgr" "github.com/lightningnetwork/lnd/batch" "github.com/lightningnetwork/lnd/kvdb" "github.com/lightningnetwork/lnd/lnwire" @@ -1525,12 +1526,10 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf BlockHeight: height, } - // Delete everything after this height from the db. - endShortChanID := lnwire.ShortChannelID{ - BlockHeight: math.MaxUint32 & 0x00ffffff, - TxIndex: math.MaxUint32 & 0x00ffffff, - TxPosition: math.MaxUint16, - } + // Delete everything after this height from the db up until the + // SCID alias range. + endShortChanID := aliasmgr.StartingAlias + // The block height will be the 3 first bytes of the channel IDs. var chanIDStart [8]byte byteOrder.PutUint64(chanIDStart[:], startShortChanID.ToUint64()) @@ -1569,11 +1568,14 @@ func (c *ChannelGraph) DisconnectBlockAtHeight(height uint32) ([]*ChannelEdgeInf // found edge. // NOTE: we must delete the edges after the cursor loop, since // modifying the bucket while traversing is not safe. + // NOTE: We use a < comparison in bytes.Compare instead of <= + // so that the StartingAlias itself isn't deleted. var keys [][]byte cursor := edgeIndex.ReadWriteCursor() - for k, v := cursor.Seek(chanIDStart[:]); k != nil && - bytes.Compare(k, chanIDEnd[:]) <= 0; k, v = cursor.Next() { + //nolint:lll + for k, v := cursor.Seek(chanIDStart[:]); k != nil && + bytes.Compare(k, chanIDEnd[:]) < 0; k, v = cursor.Next() { edgeInfoReader := bytes.NewReader(v) edgeInfo, err := deserializeChanEdgeInfo(edgeInfoReader) if err != nil { From 9d56963f5e1440601f32cd94a3fe15e278368a3e Mon Sep 17 00:00:00 2001 From: eugene Date: Thu, 5 Jan 2023 17:40:35 -0500 Subject: [PATCH 3/4] itest: add zero-conf reorg edge existence test This itest asserts that zero-conf edges exist even after a reorg. Without the prior commit, this test fails. --- lntest/itest/list_on_test.go | 4 + lntest/itest/lnd_zero_conf_test.go | 171 +++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/lntest/itest/list_on_test.go b/lntest/itest/list_on_test.go index 14d8b02d73..f2531203fd 100644 --- a/lntest/itest/list_on_test.go +++ b/lntest/itest/list_on_test.go @@ -505,4 +505,8 @@ var allTestCasesTemp = []*lntemp.TestCase{ Name: "sign verify message with addr", TestFunc: testSignVerifyMessageWithAddr, }, + { + Name: "zero conf reorg edge existence", + TestFunc: testZeroConfReorg, + }, } diff --git a/lntest/itest/lnd_zero_conf_test.go b/lntest/itest/lnd_zero_conf_test.go index f630bfdeae..bf5de369a4 100644 --- a/lntest/itest/lnd_zero_conf_test.go +++ b/lntest/itest/lnd_zero_conf_test.go @@ -1,17 +1,22 @@ package itest import ( + "context" "testing" "time" + "github.com/btcsuite/btcd/btcjson" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/integration/rpctest" "github.com/go-errors/errors" + "github.com/lightningnetwork/lnd/aliasmgr" "github.com/lightningnetwork/lnd/chainreg" "github.com/lightningnetwork/lnd/lnrpc" "github.com/lightningnetwork/lnd/lnrpc/routerrpc" "github.com/lightningnetwork/lnd/lntemp" "github.com/lightningnetwork/lnd/lntemp/node" "github.com/lightningnetwork/lnd/lntemp/rpc" + "github.com/lightningnetwork/lnd/lntest" "github.com/lightningnetwork/lnd/lntest/wait" "github.com/lightningnetwork/lnd/lnwire" "github.com/stretchr/testify/require" @@ -883,3 +888,169 @@ func acceptChannel(t *testing.T, zeroConf bool, stream rpc.AcceptorClient) { err = stream.Send(resp) require.NoError(t, err) } + +// testZeroConfReorg tests that a reorg does not cause a zero-conf channel to +// be deleted from the channel graph. This was previously the case due to logic +// in the function DisconnectBlockAtHeight. +func testZeroConfReorg(ht *lntemp.HarnessTest) { + if ht.ChainBackendName() == lntest.NeutrinoBackendName { + ht.Skipf("skipping zero-conf reorg test for neutrino backend") + } + + var ( + ctxb = context.Background() + temp = "temp" + ) + + // Since zero-conf is opt in, the harness nodes provided won't be able + // to open zero-conf channels. In that case, we just spin up new nodes. + zeroConfArgs := []string{ + "--protocol.option-scid-alias", + "--protocol.zero-conf", + "--protocol.anchors", + } + + carol := ht.NewNode("Carol", zeroConfArgs) + + // Spin-up Dave so Carol can open a zero-conf channel to him. + dave := ht.NewNode("Dave", zeroConfArgs) + + // We'll give Carol some coins in order to fund the channel. + ht.FundCoins(btcutil.SatoshiPerBitcoin, carol) + + // Ensure that both Carol and Dave are connected. + ht.EnsureConnected(carol, dave) + + // Setup a ChannelAcceptor for Dave. + acceptStream, cancel := dave.RPC.ChannelAcceptor() + go acceptChannel(ht.T, true, acceptStream) + + // Open a private zero-conf anchors channel of 1M satoshis. + params := lntemp.OpenChannelParams{ + Amt: btcutil.Amount(1_000_000), + CommitmentType: lnrpc.CommitmentType_ANCHORS, + ZeroConf: true, + } + _ = ht.OpenChannelNoAnnounce(carol, dave, params) + + // Remove the ChannelAcceptor. + cancel() + + // Attempt to send a 10K satoshi payment from Carol to Dave. This + // requires that the edge exists in the graph. + daveInvoiceParams := &lnrpc.Invoice{ + Value: int64(10_000), + } + daveInvoiceResp := dave.RPC.AddInvoice(daveInvoiceParams) + + payReqs := []string{daveInvoiceResp.PaymentRequest} + ht.CompletePaymentRequests(carol, payReqs) + + // We will now attempt to query for the alias SCID in Carol's graph. + // We will query for the starting alias, which is exported by the + // aliasmgr package. + _, err := carol.RPC.LN.GetChanInfo(ctxb, &lnrpc.ChanInfoRequest{ + ChanId: aliasmgr.StartingAlias.ToUint64(), + }) + require.NoError(ht.T, err) + + // Now we will trigger a reorg and we'll assert that the edge still + // exists in the graph. + // + // First, we'll setup a new miner that we can use to cause a reorg. + tempLogDir := ".tempminerlogs" + logFilename := "output-open_channel_reorg-temp_miner.log" + tempMiner := lntemp.NewTempMiner( + ht.Context(), ht.T, tempLogDir, logFilename, + ) + defer tempMiner.Stop() + + require.NoError( + ht.T, tempMiner.SetUp(false, 0), "unable to setup mining node", + ) + + // We start by connecting the new miner to our original miner, such + // that it will sync to our original chain. + err = ht.Miner.Client.Node( + btcjson.NConnect, tempMiner.P2PAddress(), &temp, + ) + require.NoError(ht.T, err, "unable to connect node") + + nodeSlice := []*rpctest.Harness{ht.Miner.Harness, tempMiner.Harness} + err = rpctest.JoinNodes(nodeSlice, rpctest.Blocks) + require.NoError(ht.T, err, "unable to join node on blocks") + + // The two miners should be on the same block height. + assertMinerBlockHeightDelta(ht, ht.Miner, tempMiner, 0) + + // We disconnect the two miners, such that we can mine two chains and + // cause a reorg later. + err = ht.Miner.Client.Node( + btcjson.NDisconnect, tempMiner.P2PAddress(), &temp, + ) + require.NoError(ht.T, err, "unable to remove node") + + // We now cause a fork, by letting our original miner mine 1 block and + // our new miner will mine 2. + ht.MineBlocks(1) + _, err = tempMiner.Client.Generate(2) + require.NoError(ht.T, err, "unable to generate blocks") + + // Ensure the temp miner is one block ahead. + assertMinerBlockHeightDelta(ht, ht.Miner, tempMiner, 1) + + // Wait for Carol to sync to the original miner's chain. + _, minerHeight, err := ht.Miner.Client.GetBestBlock() + require.NoError(ht.T, err, "unable to get current blockheight") + + ht.WaitForNodeBlockHeight(carol, minerHeight) + + // Now we'll disconnect Carol's chain backend from the original miner + // so that we can connect the two miners together and let the original + // miner sync to the temp miner's chain. + ht.DisconnectMiner() + + // Connecting to the temporary miner should cause the original miner to + // reorg to the longer chain. + err = ht.Miner.Client.Node( + btcjson.NConnect, tempMiner.P2PAddress(), &temp, + ) + require.NoError(ht.T, err, "unable to remove node") + + nodes := []*rpctest.Harness{tempMiner.Harness, ht.Miner.Harness} + err = rpctest.JoinNodes(nodes, rpctest.Blocks) + require.NoError(ht.T, err, "unable to join node on blocks") + + // They should now be on the same chain. + assertMinerBlockHeightDelta(ht, ht.Miner, tempMiner, 0) + + // Now we disconnect the two miners and reconnect our original chain + // backend. + err = ht.Miner.Client.Node( + btcjson.NDisconnect, tempMiner.P2PAddress(), &temp, + ) + require.NoError(ht.T, err, "unable to remove node") + + ht.ConnectMiner() + + // This should have caused a reorg and Alice should sync to the new + // chain. + _, tempMinerHeight, err := tempMiner.Client.GetBestBlock() + require.NoError(ht.T, err, "unable to get current blockheight") + + ht.WaitForNodeBlockHeight(carol, tempMinerHeight) + + err = wait.Predicate(func() bool { + ctxt, _ := context.WithTimeout(ctxb, defaultTimeout) + + _, err = carol.RPC.LN.GetChanInfo(ctxt, &lnrpc.ChanInfoRequest{ + ChanId: aliasmgr.StartingAlias.ToUint64(), + }) + + return err == nil + }, defaultTimeout) + require.NoError(ht.T, err, "carol doesn't have zero-conf edge") + + // Mine the zero-conf funding transaction so the test doesn't fail. + ht.MineBlocks(1) +} From b0028794c136cb1cb0aae16275b71e6a6b452471 Mon Sep 17 00:00:00 2001 From: eugene Date: Mon, 23 Jan 2023 16:22:56 -0500 Subject: [PATCH 4/4] release-notes: update for 0.16.0 --- docs/release-notes/release-notes-0.16.0.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 5712245486..3205de4721 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/docs/release-notes/release-notes-0.16.0.md @@ -234,6 +234,9 @@ in the lnwire package](https://github.com/lightningnetwork/lnd/pull/7303) * [Remove non-existent Cleanup calls from etcd test code in the `kvdb` package](https://github.com/lightningnetwork/lnd/pull/7352) +* [A bug has been fixed where a reorg would cause zero-conf channels to be deleted + from the graph.](https://github.com/lightningnetwork/lnd/pull/7292) + ## `lncli` * [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice