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.

Sign up for GitHub

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

Jump to bottom

sweep: prepare for sweeper #1978

Merged
merged 6 commits into from
Oct 17, 2018
Merged

sweep: prepare for sweeper #1978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9fcc7ee
utxonursery: move spendable output structs to sweep package
joostjager Sep 21, 2018
4dab405
sweep: move sweep tx generation into sweep package
joostjager Sep 26, 2018
7d69df7
sweep: create new Input interface
joostjager Sep 26, 2018
c1d845a
cnct: reuse sweep tx logic for commit resolver
joostjager Sep 26, 2018
6977d59
cnct: reuse sweep tx logic for success resolver
joostjager Sep 26, 2018
95bf858
sweep: refactor functions and unify tx info logging
joostjager Oct 17, 2018
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
51 changes: 16 additions & 35 deletions breacharbiter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/lightningnetwork/lnd/channeldb"
"github.com/lightningnetwork/lnd/htlcswitch"
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/sweep"
)

var (
Expand Down Expand Up @@ -748,33 +749,6 @@ func (b *breachArbiter) handleBreachHandoff(breachEvent *ContractBreachEvent) {
go b.exactRetribution(cfChan, retInfo)
}

// SpendableOutput an interface which can be used by the breach arbiter to
// construct a transaction spending from outputs we control.
type SpendableOutput interface {
// Amount returns the number of satoshis contained within the output.
Amount() btcutil.Amount

// Outpoint returns the reference to the output being spent, used to
// construct the corresponding transaction input.
OutPoint() *wire.OutPoint

// WitnessType returns an enum specifying the type of witness that must
// be generated in order to spend this output.
WitnessType() lnwallet.WitnessType

// SignDesc returns a reference to a spendable output's sign descriptor,
// which is used during signing to compute a valid witness that spends
// this output.
SignDesc() *lnwallet.SignDescriptor

// BuildWitness returns a valid witness allowing this output to be
// spent, the witness should be attached to the transaction at the
// location determined by the given `txinIdx`.
BuildWitness(signer lnwallet.Signer, txn *wire.MsgTx,
hashCache *txscript.TxSigHashes,
txinIdx int) ([][]byte, error)
}

// breachedOutput contains all the information needed to sweep a breached
// output. A breached output is an output that we are now entitled to due to a
// revoked commitment transaction being broadcast.
Expand Down Expand Up @@ -850,9 +824,16 @@ func (bo *breachedOutput) BuildWitness(signer lnwallet.Signer, txn *wire.MsgTx,
return bo.witnessFunc(txn, hashCache, txinIdx)
}

// Add compile-time constraint ensuring breachedOutput implements
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the rationale behind this change? Seems better from the PoV of unit tests, that we can feed in this interface into various methods in his file. After all, we may eventually extract this into the contractcourt package.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed the interface because it was not used anywhere. But indeed, I can also see the breacharbiter use an (its own) instance of the sweeper. Converted to Input types.

// SpendableOutput.
var _ SpendableOutput = (*breachedOutput)(nil)
// BlocksToMaturity returns the relative timelock, as a number of blocks, that
// must be built on top of the confirmation height before the output can be
// spent.
func (bo *breachedOutput) BlocksToMaturity() uint32 {
joostjager marked this conversation as resolved.
Show resolved Hide resolved
return 0
}

// Add compile-time constraint ensuring breachedOutput implements the Input
// interface.
var _ sweep.Input = (*breachedOutput)(nil)
joostjager marked this conversation as resolved.
Show resolved Hide resolved

// retributionInfo encapsulates all the data needed to sweep all the contested
// funds within a channel whose contract has been breached by the prior
Expand Down Expand Up @@ -963,13 +944,13 @@ func (b *breachArbiter) createJusticeTx(
// outputs, while simultaneously computing the estimated weight of the
// transaction.
var (
spendableOutputs []SpendableOutput
spendableOutputs []sweep.Input
weightEstimate lnwallet.TxWeightEstimator
)

// Allocate enough space to potentially hold each of the breached
// outputs in the retribution info.
spendableOutputs = make([]SpendableOutput, 0, len(r.breachedOutputs))
spendableOutputs = make([]sweep.Input, 0, len(r.breachedOutputs))

// The justice transaction we construct will be a segwit transaction
// that pays to a p2wkh output. Components such as the version,
Expand Down Expand Up @@ -1023,7 +1004,7 @@ func (b *breachArbiter) createJusticeTx(
// sweepSpendableOutputsTxn creates a signed transaction from a sequence of
// spendable outputs by sweeping the funds into a single p2wkh output.
func (b *breachArbiter) sweepSpendableOutputsTxn(txWeight int64,
inputs ...SpendableOutput) (*wire.MsgTx, error) {
inputs ...sweep.Input) (*wire.MsgTx, error) {

// First, we obtain a new public key script from the wallet which we'll
// sweep the funds to.
Expand All @@ -1037,7 +1018,7 @@ func (b *breachArbiter) sweepSpendableOutputsTxn(txWeight int64,
// Compute the total amount contained in the inputs.
var totalAmt btcutil.Amount
for _, input := range inputs {
totalAmt += input.Amount()
totalAmt += btcutil.Amount(input.SignDesc().Output.Value)
}

// We'll actually attempt to target inclusion within the next two
Expand Down Expand Up @@ -1085,7 +1066,7 @@ func (b *breachArbiter) sweepSpendableOutputsTxn(txWeight int64,
// witness, and attaching it to the transaction. This function accepts
// an integer index representing the intended txin index, and the
// breached output from which it will spend.
addWitness := func(idx int, so SpendableOutput) error {
addWitness := func(idx int, so sweep.Input) error {
// First, we construct a valid witness for this outpoint and
// transaction using the SpendableOutput's witness generation
// function.
Expand Down
4 changes: 4 additions & 0 deletions contractcourt/chain_arbitrator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package contractcourt
import (
"errors"
"fmt"
"github.com/lightningnetwork/lnd/sweep"
"sync"
"sync/atomic"

Expand Down Expand Up @@ -130,6 +131,9 @@ type ChainArbitratorConfig struct {
// DisableChannel disables a channel, resulting in it not being able to
// forward payments.
DisableChannel func(wire.OutPoint) error

// Sweeper allows resolvers to sweep their final outputs.
Sweeper *sweep.UtxoSweeper
}

// ChainArbitrator is a sub-system that oversees the on-chain resolution of all
Expand Down
114 changes: 22 additions & 92 deletions contractcourt/contract_resolvers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ import (
"crypto/sha256"
"encoding/binary"
"fmt"
"github.com/lightningnetwork/lnd/sweep"
"io"
"io/ioutil"

"github.com/btcsuite/btcd/txscript"
"github.com/btcsuite/btcd/wire"
"github.com/davecgh/go-spew/spew"
"github.com/lightningnetwork/lnd/chainntnfs"
Expand Down Expand Up @@ -446,59 +446,19 @@ func (h *htlcSuccessResolver) Resolve() (ContractResolver, error) {
"incoming+remote htlc confirmed", h,
h.payHash[:])

// In this case, we can sweep it directly from the
// commitment output. We'll first grab a fresh address
// from the wallet to sweep the output.
addr, err := h.NewSweepAddr()
if err != nil {
return nil, err
}

// With our address obtained, we'll query for an
// estimate to be confirmed at ease.
//
// TODO(roasbeef): signal up if fee would be too large
// to sweep singly, need to batch
feePerKw, err := h.FeeEstimator.EstimateFeePerKW(6)
if err != nil {
return nil, err
}

log.Debugf("%T(%x): using %v sat/kw to sweep htlc"+
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar comment here re losing the debug logging.

"incoming+remote htlc confirmed", h,
h.payHash[:], int64(feePerKw))

// Using a weight estimator, we'll compute the total
// fee required, and from that the value we'll end up
// with.
totalWeight := (&lnwallet.TxWeightEstimator{}).
AddWitnessInput(lnwallet.OfferedHtlcSuccessWitnessSize).
AddP2WKHOutput().Weight()
totalFees := feePerKw.FeeForWeight(int64(totalWeight))
sweepAmt := h.htlcResolution.SweepSignDesc.Output.Value -
int64(totalFees)

// With the fee computation finished, we'll now
// construct the sweep transaction.
htlcPoint := h.htlcResolution.ClaimOutpoint
h.sweepTx = wire.NewMsgTx(2)
h.sweepTx.AddTxIn(&wire.TxIn{
PreviousOutPoint: htlcPoint,
})
h.sweepTx.AddTxOut(&wire.TxOut{
PkScript: addr,
Value: sweepAmt,
})

// With the transaction fully assembled, we can now
// generate a valid witness for the transaction.
h.htlcResolution.SweepSignDesc.SigHashes = txscript.NewTxSigHashes(
h.sweepTx,
)
h.sweepTx.TxIn[0].Witness, err = lnwallet.SenderHtlcSpendRedeem(
h.Signer, &h.htlcResolution.SweepSignDesc, h.sweepTx,
input := sweep.MakeHtlcSucceedInput(
&h.htlcResolution.ClaimOutpoint,
&h.htlcResolution.SweepSignDesc,
h.htlcResolution.Preimage[:],
)

var err error

// TODO: Set tx lock time to current block height instead of
// zero. Will be taken care of once sweeper implementation is
// complete.
h.sweepTx, err = h.Sweeper.CreateSweepTx(
[]sweep.Input{&input}, 0)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1256,46 +1216,16 @@ func (c *commitSweepResolver) Resolve() (ContractResolver, error) {
// If the sweep transaction isn't already generated, and the remote
// party broadcast the commitment transaction then we'll create it now.
case c.sweepTx == nil && !isLocalCommitTx:
// Now that the commitment transaction has confirmed, we'll
// craft a transaction to sweep this output into the wallet.
signDesc := c.commitResolution.SelfOutputSignDesc

// First, we'll estimate the total weight so we can compute
// fees properly. We'll use a lax estimate, as this output is
// in no immediate danger.
feePerKw, err := c.FeeEstimator.EstimateFeePerKW(6)
if err != nil {
return nil, err
}

log.Debugf("%T(%v): using %v sat/kw for sweep tx", c,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change, we've lost the debug logging here. Also just realized that we don't have a way currently to signal to the sweeper what conf timing, or fee rate that we'd like when sweeping.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved logging to sweeper.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added TODO for configurable fee rate.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before we merge we should be able to specify the fee rate. In distinct areas in the codebase, we use 3 or 6 depending on how quickly we want the transaction to enter the chain. Some cases (CLTV timeouts) are more time sensitive than others.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

c.chanPoint, int64(feePerKw))

totalWeight := (&lnwallet.TxWeightEstimator{}).
AddP2WKHInput().
AddP2WKHOutput().Weight()
totalFees := feePerKw.FeeForWeight(int64(totalWeight))
sweepAmt := signDesc.Output.Value - int64(totalFees)

c.sweepTx = wire.NewMsgTx(2)
c.sweepTx.AddTxIn(&wire.TxIn{
PreviousOutPoint: c.commitResolution.SelfOutPoint,
})
sweepAddr, err := c.NewSweepAddr()
if err != nil {
return nil, err
}
c.sweepTx.AddTxOut(&wire.TxOut{
PkScript: sweepAddr,
Value: sweepAmt,
})

// With the transaction fully assembled, we can now generate a
// valid witness for the transaction.
signDesc.SigHashes = txscript.NewTxSigHashes(c.sweepTx)
c.sweepTx.TxIn[0].Witness, err = lnwallet.CommitSpendNoDelay(
c.Signer, &signDesc, c.sweepTx,
)
input := sweep.MakeBaseInput(
&c.commitResolution.SelfOutPoint,
lnwallet.CommitmentNoDelay,
&c.commitResolution.SelfOutputSignDesc)

// TODO: Set tx lock time to current block height instead of
// zero. Will be taken care of once sweeper implementation is
// complete.
c.sweepTx, err = c.Sweeper.CreateSweepTx(
[]sweep.Input{&input}, 0)
if err != nil {
return nil, err
}
Expand Down
10 changes: 6 additions & 4 deletions log.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
package main

import (
"os"

"io"

"fmt"
"io"
"os"
"path/filepath"

"github.com/btcsuite/btcd/connmgr"
Expand All @@ -24,6 +22,7 @@ import (
"github.com/lightningnetwork/lnd/lnwallet"
"github.com/lightningnetwork/lnd/routing"
"github.com/lightningnetwork/lnd/signal"
"github.com/lightningnetwork/lnd/sweep"
)

// Loggers per subsystem. A single backend logger is created and all subsystem
Expand Down Expand Up @@ -65,6 +64,7 @@ var (
atplLog = build.NewSubLogger("ATPL", backendLog.Logger)
cnctLog = build.NewSubLogger("CNCT", backendLog.Logger)
sphxLog = build.NewSubLogger("SPHX", backendLog.Logger)
swprLog = build.NewSubLogger("SWPR", backendLog.Logger)
)

// Initialize package-global logger variables.
Expand All @@ -81,6 +81,7 @@ func init() {
contractcourt.UseLogger(cnctLog)
sphinx.UseLogger(sphxLog)
signal.UseLogger(ltndLog)
sweep.UseLogger(swprLog)
}

// subsystemLoggers maps each subsystem identifier to its associated logger.
Expand All @@ -103,6 +104,7 @@ var subsystemLoggers = map[string]btclog.Logger{
"ATPL": atplLog,
"CNCT": cnctLog,
"SPHX": sphxLog,
"SWPR": swprLog,
}

// initLogRotator initializes the logging rotator to write logs to logFile and
Expand Down
27 changes: 19 additions & 8 deletions server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"crypto/rand"
"encoding/hex"
"fmt"
"github.com/lightningnetwork/lnd/sweep"
joostjager marked this conversation as resolved.
Show resolved Hide resolved
"image/color"
"math/big"
"net"
Expand Down Expand Up @@ -58,6 +59,10 @@ const (
// durations exceeding this value will be eligible to have their
// backoffs reduced.
defaultStableConnDuration = 10 * time.Minute

// sweepTxConfirmationTarget assigns a confirmation target for sweep
// txes on which the fee calculation will be based.
sweepTxConfirmationTarget = 6
)

var (
Expand Down Expand Up @@ -581,19 +586,24 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, cc *chainControl,
return nil, err
}

sweeper := sweep.New(&sweep.UtxoSweeperConfig{
Estimator: cc.feeEstimator,
GenSweepScript: func() ([]byte, error) {
return newSweepPkScript(cc.wallet)
},
Signer: cc.wallet.Cfg.Signer,
ConfTarget: sweepTxConfirmationTarget,
})

s.utxoNursery = newUtxoNursery(&NurseryConfig{
ChainIO: cc.chainIO,
ConfDepth: 1,
FetchClosedChannels: chanDB.FetchClosedChannels,
FetchClosedChannel: chanDB.FetchClosedChannel,
Estimator: cc.feeEstimator,
GenSweepScript: func() ([]byte, error) {
return newSweepPkScript(cc.wallet)
},
Notifier: cc.chainNotifier,
PublishTransaction: cc.wallet.PublishTransaction,
Signer: cc.wallet.Cfg.Signer,
Store: utxnStore,
Notifier: cc.chainNotifier,
PublishTransaction: cc.wallet.PublishTransaction,
Store: utxnStore,
Sweeper: sweeper,
})

// Construct a closure that wraps the htlcswitch's CloseLink method.
Expand Down Expand Up @@ -685,6 +695,7 @@ func newServer(listenAddrs []net.Addr, chanDB *channeldb.DB, cc *chainControl,
DisableChannel: func(op wire.OutPoint) error {
return s.announceChanStatus(op, true)
},
Sweeper: sweeper,
}, chanDB)

s.breachArbiter = newBreachArbiter(&BreachConfig{
Expand Down
Loading