Skip to content

Commit

Permalink
Merge pull request #8800 from ProofOfKeags/bugfix/8535
Browse files Browse the repository at this point in the history
contractcourt: consider delivery addresses when evaluating toSelfAmount
  • Loading branch information
yyforyongyu committed Jun 11, 2024
2 parents 931b3dc + 1fea14f commit 286ee95
Show file tree
Hide file tree
Showing 7 changed files with 161 additions and 14 deletions.
64 changes: 53 additions & 11 deletions contractcourt/chain_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package contractcourt
import (
"bytes"
"fmt"
"slices"
"sync"
"sync/atomic"
"time"
Expand All @@ -16,8 +17,10 @@ import (
"github.com/davecgh/go-spew/spew"
"github.com/lightningnetwork/lnd/chainntnfs"
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/fn"
"github.com/lightningnetwork/lnd/input"
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/lnwire"
)

const (
Expand Down Expand Up @@ -970,28 +973,67 @@ func (c *chainWatcher) handleUnknownRemoteState(
}

// toSelfAmount takes a transaction and returns the sum of all outputs that pay
// to a script that the wallet controls. If no outputs pay to us, then we
// to a script that the wallet controls or the channel defines as its delivery
// script . If no outputs pay to us (determined by these criteria), then we
// return zero. This is possible as our output may have been trimmed due to
// being dust.
func (c *chainWatcher) toSelfAmount(tx *wire.MsgTx) btcutil.Amount {
var selfAmt btcutil.Amount
for _, txOut := range tx.TxOut {
// There are two main cases we have to handle here. First, in the coop
// close case we will always have saved the delivery address we used
// whether it was from the upfront shutdown, from the delivery address
// requested at close time, or even an automatically generated one. All
// coop-close cases can be identified in the following manner:
shutdown, _ := c.cfg.chanState.ShutdownInfo()
oDeliveryAddr := fn.MapOption(
func(i channeldb.ShutdownInfo) lnwire.DeliveryAddress {
return i.DeliveryScript.Val
})(shutdown)

// Here we define a function capable of identifying whether an output
// corresponds with our local delivery script from a ShutdownInfo if we
// have a ShutdownInfo for this chainWatcher's underlying channel.
//
// isDeliveryOutput :: *TxOut -> bool
isDeliveryOutput := func(o *wire.TxOut) bool {
return fn.ElimOption(
oDeliveryAddr,
// If we don't have a delivery addr, then the output
// can't match it.
func() bool { return false },
// Otherwise if the PkScript of the TxOut matches our
// delivery script then this is a delivery output.
func(a lnwire.DeliveryAddress) bool {
return slices.Equal(a, o.PkScript)
},
)
}

// Here we define a function capable of identifying whether an output
// belongs to the LND wallet. We use this as a heuristic in the case
// where we might be looking for spendable force closure outputs.
//
// isWalletOutput :: *TxOut -> bool
isWalletOutput := func(out *wire.TxOut) bool {
_, addrs, _, err := txscript.ExtractPkScriptAddrs(
// Doesn't matter what net we actually pass in.
txOut.PkScript, &chaincfg.TestNet3Params,
out.PkScript, &chaincfg.TestNet3Params,
)
if err != nil {
continue
return false
}

for _, addr := range addrs {
if c.cfg.isOurAddr(addr) {
selfAmt += btcutil.Amount(txOut.Value)
}
}
return fn.Any(c.cfg.isOurAddr, addrs)
}

return selfAmt
// Grab all of the outputs that correspond with our delivery address
// or our wallet is aware of.
outs := fn.Filter(fn.PredOr(isDeliveryOutput, isWalletOutput), tx.TxOut)

// Grab the values for those outputs.
vals := fn.Map(func(o *wire.TxOut) int64 { return o.Value }, outs)

// Return the sum.
return btcutil.Amount(fn.Sum(vals))
}

// dispatchCooperativeClose processed a detect cooperative channel closure.
Expand Down
4 changes: 4 additions & 0 deletions docs/release-notes/release-notes-0.18.1.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@

# Bug Fixes

* `closedchannels` now [successfully reports](https://github.com/lightningnetwork/lnd/pull/8800)
settled balances even if the delivery address is set to an address that
LND does not control.

# New Features
## Functional Enhancements
## RPC Additions
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ require (
github.com/lightningnetwork/lightning-onion v1.2.1-0.20230823005744-06182b1d7d2f
github.com/lightningnetwork/lnd/cert v1.2.2
github.com/lightningnetwork/lnd/clock v1.1.1
github.com/lightningnetwork/lnd/fn v1.0.5
github.com/lightningnetwork/lnd/fn v1.0.9
github.com/lightningnetwork/lnd/healthcheck v1.2.4
github.com/lightningnetwork/lnd/kvdb v1.4.8
github.com/lightningnetwork/lnd/queue v1.1.1
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,8 @@ github.com/lightningnetwork/lnd/cert v1.2.2 h1:71YK6hogeJtxSxw2teq3eGeuy4rHGKcFf
github.com/lightningnetwork/lnd/cert v1.2.2/go.mod h1:jQmFn/Ez4zhDgq2hnYSw8r35bqGVxViXhX6Cd7HXM6U=
github.com/lightningnetwork/lnd/clock v1.1.1 h1:OfR3/zcJd2RhH0RU+zX/77c0ZiOnIMsDIBjgjWdZgA0=
github.com/lightningnetwork/lnd/clock v1.1.1/go.mod h1:mGnAhPyjYZQJmebS7aevElXKTFDuO+uNFFfMXK1W8xQ=
github.com/lightningnetwork/lnd/fn v1.0.5 h1:ffDgMSn83avw6rNzxhbt6w5/2oIrwQKTPGfyaLupZtE=
github.com/lightningnetwork/lnd/fn v1.0.5/go.mod h1:P027+0CyELd92H9gnReUkGGAqbFA1HwjHWdfaDFD51U=
github.com/lightningnetwork/lnd/fn v1.0.9 h1:VPljrzHGh0Wfs2NZe/ugUfH0hl6/L2eXW0LLXMUEy3s=
github.com/lightningnetwork/lnd/fn v1.0.9/go.mod h1:P027+0CyELd92H9gnReUkGGAqbFA1HwjHWdfaDFD51U=
github.com/lightningnetwork/lnd/healthcheck v1.2.4 h1:lLPLac+p/TllByxGSlkCwkJlkddqMP5UCoawCj3mgFQ=
github.com/lightningnetwork/lnd/healthcheck v1.2.4/go.mod h1:G7Tst2tVvWo7cx6mSBEToQC5L1XOGxzZTPB29g9Rv2I=
github.com/lightningnetwork/lnd/kvdb v1.4.8 h1:xH0a5Vi1yrcZ5BEeF2ba3vlKBRxrL9uYXlWTjOjbNTY=
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 @@ -626,4 +626,8 @@ var allTestCases = []*lntest.TestCase{
Name: "sweep commit output and anchor",
TestFunc: testSweepCommitOutputAndAnchor,
},
{
Name: "coop close with external delivery",
TestFunc: testCoopCloseWithExternalDelivery,
},
}
92 changes: 92 additions & 0 deletions itest/lnd_coop_close_external_delivery_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package itest

import (
"testing"

"github.com/btcsuite/btcd/btcutil"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lntest"
"github.com/stretchr/testify/require"
)

func testCoopCloseWithExternalDelivery(ht *lntest.HarnessTest) {
ht.Run("set delivery address at open", func(t *testing.T) {
tt := ht.Subtest(t)
testCoopCloseWithExternalDeliveryImpl(tt, true)
})
ht.Run("set delivery address at close", func(t *testing.T) {
tt := ht.Subtest(t)
testCoopCloseWithExternalDeliveryImpl(tt, false)
})
}

// testCoopCloseWithExternalDeliveryImpl ensures that we have a valid settled
// balance irrespective of whether the delivery address is in LND's wallet or
// not. Some users set this value to be an address in a different wallet and
// this should not affect our ability to accurately report the settled balance.
func testCoopCloseWithExternalDeliveryImpl(ht *lntest.HarnessTest,
upfrontShutdown bool) {

alice, bob := ht.Alice, ht.Bob
ht.ConnectNodes(alice, bob)

// Here we generate a final delivery address in bob's wallet but set by
// alice. We do this to ensure that the address is not in alice's LND
// wallet. We already correctly track settled balances when the address
// is in the LND wallet.
addr := bob.RPC.NewAddress(&lnrpc.NewAddressRequest{
Type: lnrpc.AddressType_UNUSED_WITNESS_PUBKEY_HASH,
})

// Prepare for channel open.
openParams := lntest.OpenChannelParams{
Amt: btcutil.Amount(1000000),
}

// If we are testing the case where we set it on open then we'll set the
// upfront shutdown script in the channel open parameters.
if upfrontShutdown {
openParams.CloseAddress = addr.Address
}

// Open the channel!
chanPoint := ht.OpenChannel(alice, bob, openParams)

// Prepare for channel close.
closeParams := lnrpc.CloseChannelRequest{
ChannelPoint: chanPoint,
TargetConf: 6,
}

// If we are testing the case where we set the delivery address on
// channel close then we will set it in the channel close parameters.
if !upfrontShutdown {
closeParams.DeliveryAddress = addr.Address
}

// Close the channel!
closeClient := alice.RPC.CloseChannel(&closeParams)

// Assert that we got a channel update when we get a closing txid.
_, err := closeClient.Recv()
require.NoError(ht, err)

// Mine the closing transaction.
ht.MineClosingTx(chanPoint)

// Assert that we got a channel update when the closing tx was mined.
_, err = closeClient.Recv()
require.NoError(ht, err)

// Here we query our closed channels to conduct the final test
// assertion. We want to ensure that even though alice's delivery
// address is set to an address in bob's wallet, we should still show
// the balance as settled.
closed := alice.RPC.ClosedChannels(&lnrpc.ClosedChannelsRequest{
Cooperative: true,
})

// The settled balance should never be zero at this point.
require.NotZero(ht, len(closed.Channels))
require.NotZero(ht, closed.Channels[0].SettledBalance)
}
5 changes: 5 additions & 0 deletions lntest/harness.go
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,10 @@ type OpenChannelParams struct {
// FundMax flag is specified the entirety of selected funds is
// allocated towards channel funding.
Outpoints []*lnrpc.OutPoint

// CloseAddress sets the upfront_shutdown_script parameter during
// channel open. It is expected to be encoded as a bitcoin address.
CloseAddress string
}

// prepareOpenChannel waits for both nodes to be synced to chain and returns an
Expand Down Expand Up @@ -1059,6 +1063,7 @@ func (h *HarnessTest) prepareOpenChannel(srcNode, destNode *node.HarnessNode,
FundMax: p.FundMax,
Memo: p.Memo,
Outpoints: p.Outpoints,
CloseAddress: p.CloseAddress,
}
}

Expand Down

0 comments on commit 286ee95

Please sign in to comment.