Skip to content

Commit

Permalink
multi: fix and test v1 output spend ntfns
Browse files Browse the repository at this point in the history
Because Taproot key spend only spends don't allow us to re-construct the
spent pkScript from the witness alone, we cannot support registering
spend notifications for v1 pkScripts only. We instead require the
outpoint to be specified. This commit makes it possible to only match by
outpoint and also adds an itest for it.
  • Loading branch information
guggero committed Mar 24, 2022
1 parent 7697013 commit 840f5a4
Show file tree
Hide file tree
Showing 6 changed files with 137 additions and 7 deletions.
44 changes: 44 additions & 0 deletions chainntnfs/txnotifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ var (
// notifier to match _and_ dispatch upon detecting the spend of the
// script on-chain, rather than the outpoint.
ZeroOutPoint wire.OutPoint

// zeroV1KeyPush is a pkScript that pushes an all-zero 32-byte Taproot
// SegWit v1 key to the stack.
zeroV1KeyPush = [34]byte{
txscript.OP_1, txscript.OP_DATA_32, // 32 byte of zeroes here
}

// ZeroTaprootPkScript is the parsed txscript.PkScript of an empty
// Taproot SegWit v1 key being pushed to the stack. This allows the
// notifier to match _and_ dispatch upon detecting the spend of the
// outpoint on-chain, rather than the pkScript (which cannot be derived
// from the witness alone in the SegWit v1 case).
ZeroTaprootPkScript, _ = txscript.ParsePkScript(zeroV1KeyPush[:])
)

var (
Expand Down Expand Up @@ -322,6 +335,24 @@ func NewSpendRequest(op *wire.OutPoint, pkScript []byte) (SpendRequest, error) {
}
r.PkScript = outputScript

// For Taproot spends we have the main problem that for the key spend
// path we cannot derive the pkScript from only looking at the input's
// witness. So we need to rely on the outpoint information alone.
//
// TODO(guggero): For script path spends we can derive the pkScript from
// the witness, since we have the full control block and the spent
// script available.
if outputScript.Class() == txscript.WitnessV1TaprootTy {
if op == nil {
return r, fmt.Errorf("cannot register witness v1 " +
"spend request without outpoint")
}

// We have an outpoint, so we can set the pkScript to an all
// zero Taproot key that we'll compare this spend request to.
r.PkScript = ZeroTaprootPkScript
}

return r, nil
}

Expand Down Expand Up @@ -1488,6 +1519,19 @@ func (n *TxNotifier) filterTx(tx *btcutil.Tx, blockHash *chainhash.Hash,
if _, ok := n.spendNotifications[spendRequest]; ok {
notifyDetails(spendRequest, prevOut, uint32(i))
}

// Now try with an empty taproot key pkScript, since we
// cannot derive the spent pkScript directly from the
// witness. But we have the outpoint, which should be
// enough.
spendRequest.PkScript = ZeroTaprootPkScript
if _, ok := n.spendNotifications[spendRequest]; ok {
notifyDetails(spendRequest, prevOut, uint32(i))
}

// Restore the pkScript but try with a zero outpoint
// instead (won't be possible for Taproot).
spendRequest.PkScript = pkScript
spendRequest.OutPoint = ZeroOutPoint
if _, ok := n.spendNotifications[spendRequest]; ok {
notifyDetails(spendRequest, prevOut, uint32(i))
Expand Down
6 changes: 5 additions & 1 deletion lnrpc/chainrpc/chainnotifier.pb.go

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

6 changes: 5 additions & 1 deletion lnrpc/chainrpc/chainnotifier.proto
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,11 @@ message SpendRequest {
/*
The outpoint for which we should request a spend notification for. If set to
a zero outpoint, then the spend notification will be requested for the
script instead.
script instead. A zero or nil outpoint is not supported for Taproot spends
because the output script cannot reliably be computed from the witness alone
and the spent output script is not always available in the rescan context.
So an outpoint must _always_ be specified when registering a spend
notification for a Taproot output.
*/
Outpoint outpoint = 1;

Expand Down
2 changes: 1 addition & 1 deletion lnrpc/chainrpc/chainnotifier.swagger.json
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@
"properties": {
"outpoint": {
"$ref": "#/definitions/chainrpcOutpoint",
"description": "The outpoint for which we should request a spend notification for. If set to\na zero outpoint, then the spend notification will be requested for the\nscript instead."
"description": "The outpoint for which we should request a spend notification for. If set to\na zero outpoint, then the spend notification will be requested for the\nscript instead. A zero or nil outpoint is not supported for Taproot spends\nbecause the output script cannot reliably be computed from the witness alone\nand the spent output script is not always available in the rescan context.\nSo an outpoint must _always_ be specified when registering a spend\nnotification for a Taproot output."
},
"script": {
"type": "string",
Expand Down
5 changes: 5 additions & 0 deletions lntest/harness_node.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"github.com/jackc/pgx/v4/pgxpool"
"github.com/lightningnetwork/lnd/chanbackup"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnrpc/chainrpc"
"github.com/lightningnetwork/lnd/lnrpc/invoicesrpc"
"github.com/lightningnetwork/lnd/lnrpc/routerrpc"
"github.com/lightningnetwork/lnd/lnrpc/signrpc"
Expand Down Expand Up @@ -361,6 +362,7 @@ type HarnessNode struct {
Watchtower watchtowerrpc.WatchtowerClient
WatchtowerClient wtclientrpc.WatchtowerClientClient
StateClient lnrpc.StateClient
ChainClient chainrpc.ChainNotifierClient
}

// RPCClients wraps a list of RPC clients into a single struct for easier
Expand All @@ -378,6 +380,7 @@ type RPCClients struct {
Watchtower watchtowerrpc.WatchtowerClient
WatchtowerClient wtclientrpc.WatchtowerClientClient
State lnrpc.StateClient
ChainClient chainrpc.ChainNotifierClient
}

// Assert *HarnessNode implements the lnrpc.LightningClient interface.
Expand Down Expand Up @@ -929,6 +932,7 @@ func (hn *HarnessNode) InitRPCClients(c *grpc.ClientConn) {
WatchtowerClient: wtclientrpc.NewWatchtowerClientClient(c),
Signer: signrpc.NewSignerClient(c),
State: lnrpc.NewStateClient(c),
ChainClient: chainrpc.NewChainNotifierClient(c),
}
}

Expand All @@ -949,6 +953,7 @@ func (hn *HarnessNode) initLightningClient() error {
hn.WatchtowerClient = wtclientrpc.NewWatchtowerClientClient(conn)
hn.SignerClient = signrpc.NewSignerClient(conn)
hn.StateClient = lnrpc.NewStateClient(conn)
hn.ChainClient = chainrpc.NewChainNotifierClient(conn)

// Wait until the server is fully started.
if err := hn.WaitUntilServerActive(); err != nil {
Expand Down
81 changes: 77 additions & 4 deletions lntest/itest/lnd_taproot_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
"github.com/btcsuite/btcd/wire"
"github.com/lightningnetwork/lnd/input"
"github.com/lightningnetwork/lnd/lnrpc"
"github.com/lightningnetwork/lnd/lnrpc/chainrpc"
"github.com/lightningnetwork/lnd/lnrpc/signrpc"
"github.com/lightningnetwork/lnd/lnrpc/walletrpc"
"github.com/lightningnetwork/lnd/lntest"
Expand Down Expand Up @@ -101,8 +102,36 @@ func testTaprootKeySpend(ctxt context.Context, t *harnessTest,
}
assertWalletUnspent(t, net.Alice, op)

// Before we confirm the transaction, let's register a confirmation
// listener for it, which we expect to fire after mining a block.
p2trAddr, err := btcutil.DecodeAddress(
p2trResp.Address, harnessNetParams,
)
require.NoError(t.t, err)
p2trPkScript, err := txscript.PayToAddrScript(p2trAddr)
require.NoError(t.t, err)

_, currentHeight, err := net.Miner.Client.GetBestBlock()
require.NoError(t.t, err)
confClient, err := net.Alice.ChainClient.RegisterConfirmationsNtfn(
ctxt, &chainrpc.ConfRequest{
Script: p2trPkScript,
Txid: txid[:],
HeightHint: uint32(currentHeight),
NumConfs: 1,
},
)
require.NoError(t.t, err)

// Mine another block to clean up the mempool.
mineBlocks(t, net, 1, 1)

// We now expect our confirmation to go through.
confMsg, err := confClient.Recv()
require.NoError(t.t, err)
conf := confMsg.GetConf()
require.NotNil(t.t, conf)
require.Equal(t.t, conf.BlockHeight, uint32(currentHeight+1))
}

// testTaprootScriptSpend tests sending to and spending from p2tr script
Expand Down Expand Up @@ -161,6 +190,10 @@ func testTaprootScriptSpend(ctxt context.Context, t *harnessTest,
require.NoError(t.t, err)

p2trOutputIndex := getOutputIndex(t, net, txid, tapScriptAddr.String())
p2trOutpoint := wire.OutPoint{
Hash: *txid,
Index: uint32(p2trOutputIndex),
}

// Clear the mempool.
mineBlocks(t, net, 1, 1)
Expand All @@ -182,10 +215,7 @@ func testTaprootScriptSpend(ctxt context.Context, t *harnessTest,

tx := wire.NewMsgTx(2)
tx.TxIn = []*wire.TxIn{{
PreviousOutPoint: wire.OutPoint{
Hash: *txid,
Index: uint32(p2trOutputIndex),
},
PreviousOutPoint: p2trOutpoint,
}}
value := int64(800_000 - requiredFee)
tx.TxOut = []*wire.TxOut{{
Expand Down Expand Up @@ -233,6 +263,42 @@ func testTaprootScriptSpend(ctxt context.Context, t *harnessTest,
txWeight := blockchain.GetTransactionWeight(btcutil.NewTx(tx))
require.Equal(t.t, txWeight, estimatedWeight)

// Before we publish the tx that spends the p2tr transaction, we want to
// register a spend listener that we expect to fire after mining the
// block.
_, currentHeight, err := net.Miner.Client.GetBestBlock()
require.NoError(t.t, err)

// For a Taproot output we cannot leave the outpoint empty. Let's make
// sure the API returns the correct error here.
spendClient, err := net.Alice.ChainClient.RegisterSpendNtfn(
ctxt, &chainrpc.SpendRequest{
Script: p2trPkScript,
HeightHint: uint32(currentHeight),
},
)
require.NoError(t.t, err)

// The error is only thrown when trying to read a message.
_, err = spendClient.Recv()
require.Contains(
t.t, err.Error(),
"cannot register witness v1 spend request without outpoint",
)

// Now try again, this time with the outpoint set.
spendClient, err = net.Alice.ChainClient.RegisterSpendNtfn(
ctxt, &chainrpc.SpendRequest{
Outpoint: &chainrpc.Outpoint{
Hash: p2trOutpoint.Hash[:],
Index: p2trOutpoint.Index,
},
Script: p2trPkScript,
HeightHint: uint32(currentHeight),
},
)
require.NoError(t.t, err)

_, err = net.Alice.WalletKitClient.PublishTransaction(
ctxt, &walletrpc.Transaction{
TxHex: buf.Bytes(),
Expand All @@ -253,6 +319,13 @@ func testTaprootScriptSpend(ctxt context.Context, t *harnessTest,
// Mine another block to clean up the mempool and to make sure the spend
// tx is actually included in a block.
mineBlocks(t, net, 1, 1)

// We now expect our spend event to go through.
spendMsg, err := spendClient.Recv()
require.NoError(t.t, err)
spend := spendMsg.GetSpend()
require.NotNil(t.t, spend)
require.Equal(t.t, spend.SpendingHeight, uint32(currentHeight+1))
}

// testTaprootKeySpendRPC tests that a tapscript address can also be spent using
Expand Down

0 comments on commit 840f5a4

Please sign in to comment.