Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

neutrino remove sweeptx #7800

Merged
merged 6 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 61 additions & 1 deletion cmd/lncli/walletrpc_active.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ func walletCommands() []cli.Command {
labelTxCommand,
publishTxCommand,
getTxCommand,
removeTxCommand,
releaseOutputCommand,
leaseOutputCommand,
listLeasesCommand,
Expand Down Expand Up @@ -545,7 +546,6 @@ func publishTransaction(ctx *cli.Context) error {

req := &walletrpc.Transaction{
TxHex: tx,
Label: ctx.String("label"),
}

_, err = walletClient.PublishTransaction(ctxc, req)
Expand Down Expand Up @@ -599,6 +599,66 @@ func getTransaction(ctx *cli.Context) error {
return nil
}

var removeTxCommand = cli.Command{
Name: "removetx",
Usage: "Attempts to remove the unconfirmed transaction with the " +
"specified txid and all its children from the underlying " +
"internal wallet.",
ArgsUsage: "txid",
Description: `
Removes the transaction with the specified txid from the underlying
wallet which must still be unconfirmmed (in mempool). This command is
useful when a transaction is RBFed by another transaction. The wallet
will only resolve this conflict when the other transaction is mined
(which can take time). If a transaction was removed erronously a simple
rebroadcast of the former transaction with the "publishtx" cmd will
register the relevant outputs of the raw tx again with the wallet
(if there are no errors broadcasting this transaction due to an RBF
replacement sitting in the mempool). As soon as a removed transaction
is confirmed funds will be registered with the wallet again.`,
Flags: []cli.Flag{},
Action: actionDecorator(removeTransaction),
}

func removeTransaction(ctx *cli.Context) error {
ctxc := getContext()

// Display the command's help message if we do not have the expected
// number of arguments/flags.
if ctx.NArg() != 1 {
return cli.ShowCommandHelp(ctx, "removetx")
}

// Fetch the only cmd argument which must be a valid txid.
txid := ctx.Args().First()
txHash, err := chainhash.NewHashFromStr(txid)
if err != nil {
return err
}

walletClient, cleanUp := getWalletClient(ctx)
defer cleanUp()

req := &walletrpc.GetTransactionRequest{
Txid: txHash.String(),
}

resp, err := walletClient.RemoveTransaction(ctxc, req)
if err != nil {
return err
}

printJSON(&struct {
guggero marked this conversation as resolved.
Show resolved Hide resolved
Status string `json:"status"`
TxID string `json:"txid"`
}{
Status: resp.GetStatus(),
TxID: txHash.String(),
})

return nil
}

// utxoLease contains JSON annotations for a lease on an unspent output.
type utxoLease struct {
ID string `json:"id"`
Expand Down
3 changes: 3 additions & 0 deletions docs/code_formatting_rules.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,9 @@ func foo(a, b,

func baz(a, b, c) (d,
error) {

func longFunctionName(
ziggie1984 marked this conversation as resolved.
Show resolved Hide resolved
a, b, c) (d, error) {
```

If a function declaration spans multiple lines the body should start with an
Expand Down
8 changes: 8 additions & 0 deletions docs/release-notes/release-notes-0.18.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,14 @@

* [Ensure that a valid SCID](https://github.com/lightningnetwork/lnd/pull/8171)
is used when marking a zombie edge as live.

* [Remove sweep transactions of the
same exclusive group](https://github.com/lightningnetwork/lnd/pull/7800).
When using neutrino as a backend unconfirmed transactions have to be
removed from the wallet when a conflicting tx is confirmed. For other backends
these unconfirmed transactions are already removed. In addition a new
walletrpc endpoint `RemoveTransaction` is introduced which let one easily
remove unconfirmed transaction manually.

# New Features
## Functional Enhancements
Expand Down
4 changes: 4 additions & 0 deletions itest/list_on_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -558,4 +558,8 @@ var allTestCases = []*lntest.TestCase{
Name: "query blinded route",
TestFunc: testQueryBlindedRoutes,
},
{
Name: "removetx",
TestFunc: testRemoveTx,
},
}
103 changes: 102 additions & 1 deletion itest/lnd_onchain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ func testCPFP(ht *lntest.HarnessTest) {
runCPFP(ht, ht.Alice, ht.Bob)
}

// runCPFP ensures that the daemon can bump an unconfirmed transaction's fee
// runCPFP ensures that the daemon can bump an unconfirmed transaction's fee
// rate by broadcasting a Child-Pays-For-Parent (CPFP) transaction.
func runCPFP(ht *lntest.HarnessTest, alice, bob *node.HarnessNode) {
// Skip this test for neutrino, as it's not aware of mempool
Expand Down Expand Up @@ -731,3 +731,104 @@ func genAnchorSweep(ht *lntest.HarnessTest,

return btcutil.NewTx(tx)
}

// testRemoveTx tests that we are able to remove an unconfirmed transaction
// from the internal wallet as long as the tx is still unconfirmed. This test
// also verifies that after the tx is removed (while unconfirmed) it will show
// up as confirmed as soon as the original transaction is mined.
ziggie1984 marked this conversation as resolved.
Show resolved Hide resolved
func testRemoveTx(ht *lntest.HarnessTest) {
// Create a new node so that we start with no funds on the internal
// wallet.
alice := ht.NewNode("Alice", nil)

const initialWalletAmt = btcutil.SatoshiPerBitcoin

// Funding the node with an initial balance.
ht.FundCoins(initialWalletAmt, alice)

// Create an address for Alice to send the coins to.
req := &lnrpc.NewAddressRequest{
Type: lnrpc.AddressType_WITNESS_PUBKEY_HASH,
}
resp := alice.RPC.NewAddress(req)

// We send half the amount to that address generating two unconfirmed
// outpoints in our internal wallet.
sendReq := &lnrpc.SendCoinsRequest{
Addr: resp.Address,
Amount: initialWalletAmt / 2,
}
alice.RPC.SendCoins(sendReq)
txID := ht.Miner.AssertNumTxsInMempool(1)[0]

// Make sure the unspent number of utxos is 2 and the unconfirmed
// balances add up.
unconfirmed := ht.GetUTXOsUnconfirmed(
alice, lnwallet.DefaultAccountName,
)
require.Lenf(ht, unconfirmed, 2, "number of unconfirmed tx")

// Get the raw transaction to calculate the exact fee.
tx := ht.Miner.GetNumTxsFromMempool(1)[0]
ziggie1984 marked this conversation as resolved.
Show resolved Hide resolved

// Calculate the tx fee so we can compare the end amounts. We are
// sending from the internal wallet to the internal wallet so only
// the tx fee applies when calucalting the final amount of the wallet.
txFee := ht.CalculateTxFee(tx)

// All of alice's balance is unconfirmed and equals the initial amount
// minus the tx fee.
aliceBalResp := alice.RPC.WalletBalance()
expectedAmt := btcutil.Amount(initialWalletAmt) - txFee
require.EqualValues(ht, expectedAmt, aliceBalResp.UnconfirmedBalance)

// Now remove the transaction. We should see that the wallet state
// equals the amount prior to sending the transaction. It is important
// to understand that we do not remove any transaction from the mempool
// (thats not possible in reality) we just remove it from our local
// store.
var buf bytes.Buffer
require.NoError(ht, tx.Serialize(&buf))
alice.RPC.RemoveTransaction(&walletrpc.GetTransactionRequest{
Txid: txID.String(),
})

// Verify that the balance equals the initial state.
confirmed := ht.GetUTXOsConfirmed(
alice, lnwallet.DefaultAccountName,
)
require.Lenf(ht, confirmed, 1, "number confirmed tx")

// Alice's balance should be the initial balance now because all the
// unconfirmed tx got removed.
aliceBalResp = alice.RPC.WalletBalance()
ziggie1984 marked this conversation as resolved.
Show resolved Hide resolved
expectedAmt = btcutil.Amount(initialWalletAmt)
require.EqualValues(ht, expectedAmt, aliceBalResp.ConfirmedBalance)

// Mine a block and make sure the transaction previously broadcasted
// shows up in alice's wallet although we removed the transaction from
// the wallet when it was unconfirmed.
block := ht.Miner.MineBlocks(1)[0]
ht.Miner.AssertTxInBlock(block, txID)

// Verify that alice has 2 confirmed unspent utxos in her default
// wallet.
err := wait.NoError(func() error {
confirmed = ht.GetUTXOsConfirmed(
alice, lnwallet.DefaultAccountName,
)
if len(confirmed) != 2 {
return fmt.Errorf("expected 2 confirmed tx, "+
" got %v", len(confirmed))
}

return nil
}, lntest.DefaultTimeout)
require.NoError(ht, err, "timeout checking for confirmed utxos")

// The remaining balance should equal alice's starting balance minus the
// tx fee.
aliceBalResp = alice.RPC.WalletBalance()
expectedAmt = btcutil.Amount(initialWalletAmt) - txFee
require.EqualValues(ht, expectedAmt, aliceBalResp.ConfirmedBalance)
}