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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multi: update listsweeps to allow sweeps that are not in ListTransactions #4762

Merged
merged 1 commit into from
Nov 17, 2020
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lnrpc/walletrpc/walletkit.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion lnrpc/walletrpc/walletkit.proto
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,8 @@ message BumpFeeResponse {
message ListSweepsRequest {
/*
Retrieve the full sweep transaction details. If false, only the sweep txids
will be returned.
will be returned. Note that some sweeps that LND publishes will have been
replaced-by-fee, so will not be included in this output.
*/
bool verbose = 1;
}
Expand Down
2 changes: 1 addition & 1 deletion lnrpc/walletrpc/walletkit.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@
"parameters": [
{
"name": "verbose",
"description": "Retrieve the full sweep transaction details. If false, only the sweep txids\nwill be returned.",
"description": "Retrieve the full sweep transaction details. If false, only the sweep txids\nwill be returned. Note that some sweeps that LND publishes will have been\nreplaced-by-fee, so will not be included in this output.",
"in": "query",
"required": false,
"type": "boolean",
Expand Down
58 changes: 29 additions & 29 deletions lnrpc/walletrpc/walletkit_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -782,56 +782,56 @@ func (w *WalletKit) ListSweeps(ctx context.Context,
}

sweepTxns := make(map[string]bool)

txids := make([]string, len(sweeps))
for i, sweep := range sweeps {
for _, sweep := range sweeps {
sweepTxns[sweep.String()] = true
txids[i] = sweep.String()
}

// If the caller does not want verbose output, just return the set of
// sweep txids.
if !in.Verbose {
txidResp := &ListSweepsResponse_TransactionIDs{
TransactionIds: txids,
}

return &ListSweepsResponse{
Sweeps: &ListSweepsResponse_TransactionIds{
TransactionIds: txidResp,
},
}, nil
}

// If the caller does want full transaction lookups, query our wallet
// for all transactions, including unconfirmed transactions.
// Some of our sweeps could have been replaced by fee, or dropped out
// of the mempool. Here, we lookup our wallet transactions so that we
// can match our list of sweeps against the list of transactions that
// the wallet is still tracking.
transactions, err := w.cfg.Wallet.ListTransactionDetails(
0, btcwallet.UnconfirmedHeight,
)
if err != nil {
return nil, err
}

var sweepTxDetails []*lnwallet.TransactionDetail
var (
txids []string
txDetails []*lnwallet.TransactionDetail
)

for _, tx := range transactions {
_, ok := sweepTxns[tx.Hash.String()]
if !ok {
continue
}

sweepTxDetails = append(sweepTxDetails, tx)
// Add the txid or full tx details depending on whether we want
// verbose output or not.
if in.Verbose {
txDetails = append(txDetails, tx)
} else {
txids = append(txids, tx.Hash.String())
}
}

// Fail if we have not retrieved all of our sweep transactions from the
// wallet.
if len(sweepTxDetails) != len(txids) {
return nil, fmt.Errorf("not all sweeps found by list "+
"transactions: %v, %v", len(sweepTxDetails), len(txids))
if in.Verbose {
return &ListSweepsResponse{
Sweeps: &ListSweepsResponse_TransactionDetails{
TransactionDetails: lnrpc.RPCTransactionDetails(
txDetails,
),
},
}, nil
}

return &ListSweepsResponse{
Sweeps: &ListSweepsResponse_TransactionDetails{
TransactionDetails: lnrpc.RPCTransactionDetails(transactions),
Sweeps: &ListSweepsResponse_TransactionIds{
TransactionIds: &ListSweepsResponse_TransactionIDs{
TransactionIds: txids,
},
},
}, nil
}
Expand Down
72 changes: 47 additions & 25 deletions lntest/itest/lnd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3975,11 +3975,8 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest,
}

// Check that we can find the commitment sweep in our set of known
// sweeps.
err = findSweep(ctxb, alice, sweepingTXID)
if err != nil {
t.Fatalf("csv sweep not found: %v", err)
}
// sweeps, using the simple transaction id ListSweeps output.
assertSweepFound(ctxb, t.t, alice, sweepingTXID.String(), false)

// Restart Alice to ensure that she resumes watching the finalized
// commitment sweep txid.
Expand Down Expand Up @@ -4368,11 +4365,9 @@ func channelForceClosureTest(net *lntest.NetworkHarness, t *harnessTest,
}
}

// Check that we can find the htlc sweep in our set of sweeps.
err = findSweep(ctxb, alice, htlcSweepTx.Hash())
if err != nil {
t.Fatalf("htlc sweep not found: %v", err)
}
// Check that we can find the htlc sweep in our set of sweeps using
// the verbose output of the listsweeps output.
assertSweepFound(ctxb, t.t, alice, htlcSweepTx.Hash().String(), true)

// The following restart checks to ensure that the nursery store is
// storing the txid of the previously broadcast htlc sweep txn, and that
Expand Down Expand Up @@ -4571,33 +4566,60 @@ func assertReports(ctxb context.Context, t *harnessTest,
}
}

// findSweep looks up a sweep in a nodes list of broadcast sweeps.
func findSweep(ctx context.Context, node *lntest.HarnessNode,
sweep *chainhash.Hash) error {
// assertSweepFound looks up a sweep in a nodes list of broadcast sweeps.
func assertSweepFound(ctx context.Context, t *testing.T, node *lntest.HarnessNode,
sweep string, verbose bool) {

// List all sweeps that alice's node had broadcast.
ctx, _ = context.WithTimeout(ctx, defaultTimeout)
sweepResp, err := node.WalletKitClient.ListSweeps(
ctx, &walletrpc.ListSweepsRequest{
Verbose: false,
})
if err != nil {
return fmt.Errorf("list sweeps error: %v", err)
}
Verbose: verbose,
},
)
require.NoError(t, err)
carlaKC marked this conversation as resolved.
Show resolved Hide resolved

sweepTxIDs, ok := sweepResp.Sweeps.(*walletrpc.ListSweepsResponse_TransactionIds)
if !ok {
return errors.New("expected sweep txids in response")
var found bool
if verbose {
found = findSweepInDetails(t, sweep, sweepResp)
carlaKC marked this conversation as resolved.
Show resolved Hide resolved
} else {
found = findSweepInTxids(t, sweep, sweepResp)
}

require.True(t, found, "sweep: %v not found", sweep)
}

func findSweepInTxids(t *testing.T, sweepTxid string,
sweepResp *walletrpc.ListSweepsResponse) bool {

sweepTxIDs := sweepResp.GetTransactionIds()
require.NotNil(t, sweepTxIDs, "expected transaction ids")
require.Nil(t, sweepResp.GetTransactionDetails())

// Check that the sweep tx we have just produced is present.
for _, tx := range sweepTxIDs.TransactionIds.TransactionIds {
if tx == sweep.String() {
return nil
for _, tx := range sweepTxIDs.TransactionIds {
if tx == sweepTxid {
return true
}
}

return false
}

func findSweepInDetails(t *testing.T, sweepTxid string,
sweepResp *walletrpc.ListSweepsResponse) bool {

sweepDetails := sweepResp.GetTransactionDetails()
require.NotNil(t, sweepDetails, "expected transaction details")
require.Nil(t, sweepResp.GetTransactionIds())

for _, tx := range sweepDetails.Transactions {
if tx.TxHash == sweepTxid {
return true
}
}

return fmt.Errorf("sweep: %v not found", sweep.String())
return false
}

// assertAmountSent generates a closure which queries listchannels for sndr and
Expand Down