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

chanfunding: create new package to abstract over funding workflows #3659

Merged
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
39 changes: 30 additions & 9 deletions channeldb/channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,11 @@ const (
// type, but it omits the tweak for one's key in the commitment
// transaction of the remote party.
SingleFunderTweaklessBit ChannelType = 1 << 1

// NoFundingTxBit denotes if we have the funding transaction locally on
// disk. This bit may be on if the funding transaction was crafted by a
// wallet external to the primary daemon.
NoFundingTxBit ChannelType = 1 << 2
)

// IsSingleFunder returns true if the channel type if one of the known single
Expand All @@ -166,6 +171,12 @@ func (c ChannelType) IsTweakless() bool {
return c&SingleFunderTweaklessBit == SingleFunderTweaklessBit
}

// HasFundingTx returns true if this channel type is one that has a funding
// transaction stored locally.
func (c ChannelType) HasFundingTx() bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

This might actually be a bit dangerous, as old channels which we are not initiator will not have the funding tx on disk, but also the bit won't be set.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the bit isn't set, then that means we have the funding transaction. If it's set, then it means we don't. As written, if we're about to attempt to handle the funding transaction, we also check if we're the initiator or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the confusion arises from the method name. HasFundingTx makes it sound like it's the only check needed to retrieve the transaction, perhaps something like CraftedExternally is better?

Copy link
Contributor

Choose a reason for hiding this comment

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

... we also check if we're the initiator or not.

Yeah, this needs to be done which is why I think HasFundingTx is a dangerous name, as there is no guarantee we have it even though it returns true.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it makes sense since it is the channel type that has the funding tx, not the channel. I'm okay with keeping the name, but adding to the godoc that it is only the initiator that has the tx strored locally, but I agree with @wpaulino that a more descriptive name could lessen the confusion.

return c&NoFundingTxBit == 0
halseth marked this conversation as resolved.
Show resolved Hide resolved
}

// ChannelConstraints represents a set of constraints meant to allow a node to
// limit their exposure, enact flow control and ensure that all HTLCs are
// economically relevant. This struct will be mirrored for both sides of the
Expand Down Expand Up @@ -535,7 +546,9 @@ type OpenChannel struct {
// is found to be pending.
//
// NOTE: This value will only be populated for single-funder channels
// for which we are the initiator.
// for which we are the initiator, and that we also have the funding
// transaction for. One can check this by using the HasFundingTx()
// method on the ChanType field.
FundingTxn *wire.MsgTx

// TODO(roasbeef): eww
Expand Down Expand Up @@ -2522,6 +2535,16 @@ func writeChanConfig(b io.Writer, c *ChannelConfig) error {
)
}

// fundingTxPresent returns true if expect the funding transcation to be found
// on disk or already populated within the passed oen chanel struct.
func fundingTxPresent(channel *OpenChannel) bool {
chanType := channel.ChanType

return chanType.IsSingleFunder() && chanType.HasFundingTx() &&
channel.IsInitiator &&
!channel.hasChanStatus(ChanStatusRestored)
}

func putChanInfo(chanBucket *bbolt.Bucket, channel *OpenChannel) error {
var w bytes.Buffer
if err := WriteElements(&w,
Expand All @@ -2535,10 +2558,9 @@ func putChanInfo(chanBucket *bbolt.Bucket, channel *OpenChannel) error {
return err
}

// For single funder channels that we initiated, write the funding txn.
if channel.ChanType.IsSingleFunder() && channel.IsInitiator &&
!channel.hasChanStatus(ChanStatusRestored) {

// For single funder channels that we initiated, and we have the
// funding transaction, then write the funding txn.
if fundingTxPresent(channel) {
if err := WriteElement(&w, channel.FundingTxn); err != nil {
return err
}
Expand Down Expand Up @@ -2657,10 +2679,9 @@ func fetchChanInfo(chanBucket *bbolt.Bucket, channel *OpenChannel) error {
return err
}

// For single funder channels that we initiated, read the funding txn.
if channel.ChanType.IsSingleFunder() && channel.IsInitiator &&
!channel.hasChanStatus(ChanStatusRestored) {

// For single funder channels that we initiated and have the funding
// transaction to, read the funding txn.
if fundingTxPresent(channel) {
if err := ReadElement(r, &channel.FundingTxn); err != nil {
return err
}
Expand Down
52 changes: 31 additions & 21 deletions fundingmanager.go
Original file line number Diff line number Diff line change
Expand Up @@ -521,8 +521,9 @@ func (f *fundingManager) start() error {

// Rebroadcast the funding transaction for any pending
// channel that we initiated. No error will be returned
// if the transaction already has been broadcasted.
if channel.ChanType.IsSingleFunder() &&
// if the transaction already has been broadcast.
chanType := channel.ChanType
if chanType.IsSingleFunder() && chanType.HasFundingTx() &&
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved
halseth marked this conversation as resolved.
Show resolved Hide resolved
channel.IsInitiator {

err := f.cfg.PublishTransaction(
Expand Down Expand Up @@ -1215,6 +1216,7 @@ func (f *fundingManager) handleFundingOpen(fmsg *fundingOpenMsg) {
chainHash := chainhash.Hash(msg.ChainHash)
req := &lnwallet.InitFundingReserveMsg{
ChainHash: &chainHash,
PendingChanID: msg.PendingChannelID,
NodeID: fmsg.peer.IdentityKey(),
NodeAddr: fmsg.peer.Address(),
LocalFundingAmt: 0,
Expand Down Expand Up @@ -1739,21 +1741,28 @@ func (f *fundingManager) handleFundingSigned(fmsg *fundingSignedMsg) {
// delete it from our set of active reservations.
f.deleteReservationCtx(peerKey, pendingChanID)

// Broadcast the finalized funding transaction to the network.
fundingTx := completeChan.FundingTxn
fndgLog.Infof("Broadcasting funding tx for ChannelPoint(%v): %v",
completeChan.FundingOutpoint, spew.Sdump(fundingTx))
// Broadcast the finalized funding transaction to the network, but only
// if we actually have the funding transaction.
if completeChan.ChanType.HasFundingTx() {
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved
fundingTx := completeChan.FundingTxn

err = f.cfg.PublishTransaction(fundingTx)
if err != nil {
fndgLog.Errorf("Unable to broadcast funding tx for "+
"ChannelPoint(%v): %v", completeChan.FundingOutpoint,
err)
// We failed to broadcast the funding transaction, but watch
// the channel regardless, in case the transaction made it to
// the network. We will retry broadcast at startup.
// TODO(halseth): retry more often? Handle with CPFP? Just
// delete from the DB?
fndgLog.Infof("Broadcasting funding tx for ChannelPoint(%v): %v",
completeChan.FundingOutpoint, spew.Sdump(fundingTx))

err = f.cfg.PublishTransaction(fundingTx)
if err != nil {
fndgLog.Errorf("Unable to broadcast funding tx for "+
"ChannelPoint(%v): %v",
completeChan.FundingOutpoint, err)

// We failed to broadcast the funding transaction, but
// watch the channel regardless, in case the
// transaction made it to the network. We will retry
// broadcast at startup.
//
// TODO(halseth): retry more often? Handle with CPFP?
// Just delete from the DB?
}
}

// Now that we have a finalized reservation for this funding flow,
Expand Down Expand Up @@ -2773,6 +2782,10 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
channelFlags = lnwire.FFAnnounceChannel
}

// Obtain a new pending channel ID which is used to track this
// reservation throughout its lifetime.
chanID := f.nextPendingChanID()

// Initialize a funding reservation with the local wallet. If the
// wallet doesn't have enough funds to commit to this channel, then the
// request will fail, and be aborted.
Expand All @@ -2790,6 +2803,7 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
tweaklessCommitment := localTweakless && remoteTweakless
req := &lnwallet.InitFundingReserveMsg{
ChainHash: &msg.chainHash,
PendingChanID: chanID,
NodeID: peerKey,
NodeAddr: msg.peer.Address(),
SubtractFees: msg.subtractFees,
Expand All @@ -2815,11 +2829,7 @@ func (f *fundingManager) handleInitFundingMsg(msg *initFundingMsg) {
// SubtractFees=true.
capacity := reservation.Capacity()

// Obtain a new pending channel ID which is used to track this
// reservation throughout its lifetime.
chanID := f.nextPendingChanID()

fndgLog.Infof("Target commit tx sat/kw for pending_id(%x): %v", chanID,
fndgLog.Infof("Target commit tx sat/kw for pendingID(%x): %v", chanID,
int64(commitFeePerKw))

// If the remote CSV delay was not set in the open channel request,
Expand Down
4 changes: 2 additions & 2 deletions fundingmanager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2864,7 +2864,7 @@ func TestFundingManagerFundAll(t *testing.T) {
Value: btcutil.Amount(
0.05 * btcutil.SatoshiPerBitcoin,
),
PkScript: make([]byte, 22),
PkScript: coinPkScript,
OutPoint: wire.OutPoint{
Hash: chainhash.Hash{},
Index: 0,
Expand All @@ -2875,7 +2875,7 @@ func TestFundingManagerFundAll(t *testing.T) {
Value: btcutil.Amount(
0.06 * btcutil.SatoshiPerBitcoin,
),
PkScript: make([]byte, 22),
PkScript: coinPkScript,
OutPoint: wire.OutPoint{
Hash: chainhash.Hash{},
Index: 1,
Expand Down
137 changes: 137 additions & 0 deletions lnwallet/chanfunding/assembler.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package chanfunding

import (
"github.com/btcsuite/btcd/wire"
"github.com/btcsuite/btcutil"
"github.com/lightningnetwork/lnd/lnwallet/chainfee"
)

// CoinSource is an interface that allows a caller to access a source of UTXOs
// to use when attempting to fund a new channel.
type CoinSource interface {
// ListCoins returns all UTXOs from the source that have between
// minConfs and maxConfs number of confirmations.
ListCoins(minConfs, maxConfs int32) ([]Coin, error)
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved

// CoinFromOutPoint attempts to locate details pertaining to a coin
// based on its outpoint. If the coin isn't under the control of the
// backing CoinSource, then an error should be returned.
CoinFromOutPoint(wire.OutPoint) (*Coin, error)
}

// CoinSelectionLocker is an interface that allows the caller to perform an
// operation, which is synchronized with all coin selection attempts. This can
// be used when an operation requires that all coin selection operations cease
// forward progress. Think of this as an exclusive lock on coin selection
// operations.
type CoinSelectionLocker interface {
// WithCoinSelectLock will execute the passed function closure in a
// synchronized manner preventing any coin selection operations from
// proceeding while the closure if executing. This can be seen as the
// ability to execute a function closure under an exclusive coin
// selection lock.
WithCoinSelectLock(func() error) error
halseth marked this conversation as resolved.
Show resolved Hide resolved
}

// OutpointLocker allows a caller to lock/unlock an outpoint. When locked, the
// outpoints shouldn't be used for any sort of channel funding of coin
// selection. Locked outpoints are not expected to be persisted between
// restarts.
type OutpointLocker interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a blocker for this PR, but some of these interfaces are already defined within sweep/walletsweep.go. Do we plan to import these there?

Copy link
Member Author

Choose a reason for hiding this comment

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

They were purposefully copied over as they're pretty small interfaces, so would rather not create additional dependancies. It's also the case that if the interfaces in the sweeper are used, then a circular dependency cycle is created.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no circular dependency here since this is the chanfunding package?

Copy link
Contributor

Choose a reason for hiding this comment

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

I also agree that it is nice for a package to define its own set of interfaces, since it has it own set of requirements separate from the other package that might diverge in the future.

// LockOutpoint locks a target outpoint, rendering it unusable for coin
// selection.
LockOutpoint(o wire.OutPoint)
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved

// UnlockOutpoint unlocks a target outpoint, allowing it to be used for
// coin selection once again.
UnlockOutpoint(o wire.OutPoint)
}

// Request is a new request for funding a channel. The items in the struct
// governs how the final channel point will be provisioned by the target
// Assembler.
type Request struct {
// LocalAmt is the amount of coins we're placing into the funding
// output.
LocalAmt btcutil.Amount

// RemoteAmt is the amount of coins the remote party is contributing to
// the funding output.
RemoteAmt btcutil.Amount

// MinConfs controls how many confirmations a coin need to be eligible
// to be used as an input to the funding transaction. If this value is
// set to zero, then zero conf outputs may be spent.
MinConfs int32

// SubtractFees should be set if we intend to spend exactly LocalAmt
// when opening the channel, subtracting the fees from the funding
// output. This can be used for instance to use all our remaining funds
// to open the channel, since it will take fees into
// account.
SubtractFees bool

// FeeRate is the fee rate in sat/kw that the funding transaction
// should carry.
FeeRate chainfee.SatPerKWeight

// ChangeAddr is a closure that will provide the Assembler with a
// change address for the funding transaction if needed.
ChangeAddr func() (btcutil.Address, error)
}

// Intent is returned by an Assembler and represents the base functionality the
// caller needs to proceed with channel funding on a higher level. If the
// Cancel method is called, then all resources assembled to fund the channel
// will be released back to the eligible pool.
type Intent interface {
// FundingOutput returns the witness script, and the output that
// creates the funding output.
FundingOutput() ([]byte, *wire.TxOut, error)

// ChanPoint returns the final outpoint that will create the funding
// output described above.
ChanPoint() (*wire.OutPoint, error)
Roasbeef marked this conversation as resolved.
Show resolved Hide resolved

// RemoteFundingAmt is the amount the remote party put into the
// channel.
RemoteFundingAmt() btcutil.Amount

// LocalFundingAmt is the amount we put into the channel. This may
// differ from the local amount requested, as depending on coin
// selection, we may bleed from of that LocalAmt into fees to minimize
// change.
LocalFundingAmt() btcutil.Amount

// Cancel allows the caller to cancel a funding Intent at any time.
// This will return any resources such as coins back to the eligible
// pool to be used in order channel fundings.
Cancel()
}

// Assembler is an abstract object that is capable of assembling everything
// needed to create a new funding output. As an example, this assembler may be
// our core backing wallet, an interactive PSBT based assembler, an assembler
// than can aggregate multiple intents into a single funding transaction, or an
// external protocol that creates a funding output out-of-band such as channel
// factories.
type Assembler interface {
// ProvisionChannel returns a populated Intent that can be used to
// further the channel funding workflow. Depending on the
// implementation of Assembler, additional state machine (Intent)
// actions may be required before the FundingOutput and ChanPoint are
// made available to the caller.
ProvisionChannel(*Request) (Intent, error)
}

// FundingTxAssembler is a super-set of the regular Assembler interface that's
// also able to provide a fully populated funding transaction via the intents
// that it produuces.
type FundingTxAssembler interface {
Assembler

// FundingTxAvailable is an empty method that an assembler can
// implement to signal to callers that its able to provide the funding
// transaction for the channel via the intent it returns.
FundingTxAvailable()
}
Loading