From 2c46640dd043a1c98306472cc7cfbae193d22b8e Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:02 +0100 Subject: [PATCH 01/17] channeldb+lnwallet: note that balance is after subtracting commit fee It was incorrectly stated that the commitment balance was before subctracting the commit fee, which led to some confusion. --- channeldb/channel.go | 4 ++++ lnwallet/channel.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/channeldb/channel.go b/channeldb/channel.go index 84e96c8ea..edbe42796 100644 --- a/channeldb/channel.go +++ b/channeldb/channel.go @@ -320,10 +320,14 @@ type ChannelCommitment struct { // LocalBalance is the current available settled balance within the // channel directly spendable by us. + // + // NOTE: This is the balance *after* subtracting any commitment fee. LocalBalance lnwire.MilliSatoshi // RemoteBalance is the current available settled balance within the // channel directly spendable by the remote node. + // + // NOTE: This is the balance *after* subtracting any commitment fee. RemoteBalance lnwire.MilliSatoshi // CommitFee is the amount calculated to be paid in fees for the diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 837f05cb5..8f7282834 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -502,7 +502,7 @@ type commitment struct { // evaluating all the add/remove/settle log entries before the listed // indexes. // - // NOTE: This is the balance *before* subtracting any commitment fee. + // NOTE: This is the balance *after* subtracting any commitment fee. ourBalance lnwire.MilliSatoshi theirBalance lnwire.MilliSatoshi From f0019006a7ae7b94e2e462be2d6fa9dceeaf6c1d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:02 +0100 Subject: [PATCH 02/17] lnwallet: remove unused initiator param to CreateCooperativeCloseTx --- lnwallet/channel.go | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 8f7282834..b4bbe7f65 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -5910,10 +5910,11 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, theirBalance = theirBalance - proposedFee + commitFee } - closeTx := CreateCooperativeCloseTx(lc.fundingTxIn(), - lc.localChanCfg.DustLimit, lc.remoteChanCfg.DustLimit, - ourBalance, theirBalance, localDeliveryScript, - remoteDeliveryScript, lc.channelState.IsInitiator) + closeTx := CreateCooperativeCloseTx( + lc.fundingTxIn(), lc.localChanCfg.DustLimit, + lc.remoteChanCfg.DustLimit, ourBalance, theirBalance, + localDeliveryScript, remoteDeliveryScript, + ) // Ensure that the transaction doesn't explicitly violate any // consensus rules such as being too big, or having any value with a @@ -5980,10 +5981,11 @@ func (lc *LightningChannel) CompleteCooperativeClose(localSig, remoteSig []byte, // Create the transaction used to return the current settled balance // on this active channel back to both parties. In this current model, // the initiator pays full fees for the cooperative close transaction. - closeTx := CreateCooperativeCloseTx(lc.fundingTxIn(), - lc.localChanCfg.DustLimit, lc.remoteChanCfg.DustLimit, - ourBalance, theirBalance, localDeliveryScript, - remoteDeliveryScript, lc.channelState.IsInitiator) + closeTx := CreateCooperativeCloseTx( + lc.fundingTxIn(), lc.localChanCfg.DustLimit, + lc.remoteChanCfg.DustLimit, ourBalance, theirBalance, + localDeliveryScript, remoteDeliveryScript, + ) // Ensure that the transaction doesn't explicitly validate any // consensus rules such as being too big, or having any value with a @@ -6272,8 +6274,7 @@ func CreateCommitTx(fundingOutput wire.TxIn, // transaction in full. func CreateCooperativeCloseTx(fundingTxIn wire.TxIn, localDust, remoteDust, ourBalance, theirBalance btcutil.Amount, - ourDeliveryScript, theirDeliveryScript []byte, - initiator bool) *wire.MsgTx { + ourDeliveryScript, theirDeliveryScript []byte) *wire.MsgTx { // Construct the transaction to perform a cooperative closure of the // channel. In the event that one side doesn't have any settled funds From e2b050aca3d57782c9b6d3145c8c2356f0e6d27f Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:02 +0100 Subject: [PATCH 03/17] lnwallet: make addHtlc non-LightningChannel method It has no dependency on LightningChannel --- lnwallet/channel.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index b4bbe7f65..b1523db67 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2467,7 +2467,7 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, continue } - err := lc.addHTLC(commitTx, c.isOurs, false, htlc, keyRing) + err := addHTLC(commitTx, c.isOurs, false, htlc, keyRing) if err != nil { return err } @@ -2479,7 +2479,7 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, continue } - err := lc.addHTLC(commitTx, c.isOurs, true, htlc, keyRing) + err := addHTLC(commitTx, c.isOurs, true, htlc, keyRing) if err != nil { return err } @@ -4995,7 +4995,7 @@ func genHtlcScript(isIncoming, ourCommit bool, timeout uint32, rHash [32]byte, // locate the added HTLC on the commitment transaction from the // PaymentDescriptor that generated it, the generated script is stored within // the descriptor itself. -func (lc *LightningChannel) addHTLC(commitTx *wire.MsgTx, ourCommit bool, +func addHTLC(commitTx *wire.MsgTx, ourCommit bool, isIncoming bool, paymentDesc *PaymentDescriptor, keyRing *CommitmentKeyRing) error { From 1a4f81ed900a9a56dd4f1a9f1976a89904c33688 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:02 +0100 Subject: [PATCH 04/17] lnwallet: remove duplicate chanCfg fields, use channelState --- lnwallet/channel.go | 81 ++++++++++++++++++----------------- lnwallet/channel_test.go | 40 ++++++++--------- lnwallet/transactions_test.go | 10 ++--- 3 files changed, 66 insertions(+), 65 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index b1523db67..4c5bace65 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -880,13 +880,15 @@ func (lc *LightningChannel) diskCommitToMemCommit(isLocal bool, if localCommitPoint != nil { localCommitKeys = DeriveCommitmentKeys( localCommitPoint, true, tweaklessCommit, - lc.localChanCfg, lc.remoteChanCfg, + &lc.channelState.LocalChanCfg, + &lc.channelState.RemoteChanCfg, ) } if remoteCommitPoint != nil { remoteCommitKeys = DeriveCommitmentKeys( remoteCommitPoint, false, tweaklessCommit, - lc.localChanCfg, lc.remoteChanCfg, + &lc.channelState.LocalChanCfg, + &lc.channelState.RemoteChanCfg, ) } @@ -1360,10 +1362,6 @@ type LightningChannel struct { channelState *channeldb.OpenChannel - localChanCfg *channeldb.ChannelConfig - - remoteChanCfg *channeldb.ChannelConfig - // [local|remote]Log is a (mostly) append-only log storing all the HTLC // updates to this channel. The log is walked backwards as HTLC updates // are applied in order to re-construct a commitment transaction from a @@ -1416,8 +1414,6 @@ func NewLightningChannel(signer input.Signer, remoteCommitChain: newCommitmentChain(), localCommitChain: newCommitmentChain(), channelState: state, - localChanCfg: &state.LocalChanCfg, - remoteChanCfg: &state.RemoteChanCfg, localUpdateLog: localUpdateLog, remoteUpdateLog: remoteUpdateLog, ChanPoint: &state.FundingOutpoint, @@ -1449,8 +1445,10 @@ func NewLightningChannel(signer input.Signer, // createSignDesc derives the SignDescriptor for commitment transactions from // other fields on the LightningChannel. func (lc *LightningChannel) createSignDesc() error { - localKey := lc.localChanCfg.MultiSigKey.PubKey.SerializeCompressed() - remoteKey := lc.remoteChanCfg.MultiSigKey.PubKey.SerializeCompressed() + localKey := lc.channelState.LocalChanCfg.MultiSigKey.PubKey. + SerializeCompressed() + remoteKey := lc.channelState.RemoteChanCfg.MultiSigKey.PubKey. + SerializeCompressed() multiSigScript, err := input.GenMultiSigScript(localKey, remoteKey) if err != nil { @@ -1462,7 +1460,7 @@ func (lc *LightningChannel) createSignDesc() error { return err } lc.signDesc = &input.SignDescriptor{ - KeyDesc: lc.localChanCfg.MultiSigKey, + KeyDesc: lc.channelState.LocalChanCfg.MultiSigKey, WitnessScript: multiSigScript, Output: &wire.TxOut{ PkScript: fundingPkScript, @@ -1722,7 +1720,7 @@ func (lc *LightningChannel) restoreCommitState( tweaklessCommit := lc.channelState.ChanType.IsTweakless() pendingRemoteKeyChain = DeriveCommitmentKeys( pendingCommitPoint, false, tweaklessCommit, - lc.localChanCfg, lc.remoteChanCfg, + &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) } @@ -2324,9 +2322,9 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, // be counted for the purpose of fee calculation. var dustLimit btcutil.Amount if remoteChain { - dustLimit = lc.remoteChanCfg.DustLimit + dustLimit = lc.channelState.RemoteChanCfg.DustLimit } else { - dustLimit = lc.localChanCfg.DustLimit + dustLimit = lc.channelState.LocalChanCfg.DustLimit } c := &commitment{ @@ -2434,11 +2432,11 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, delayBalance, p2wkhBalance btcutil.Amount ) if c.isOurs { - delay = uint32(lc.localChanCfg.CsvDelay) + delay = uint32(lc.channelState.LocalChanCfg.CsvDelay) delayBalance = ourBalance.ToSatoshis() p2wkhBalance = theirBalance.ToSatoshis() } else { - delay = uint32(lc.remoteChanCfg.CsvDelay) + delay = uint32(lc.channelState.RemoteChanCfg.CsvDelay) delayBalance = theirBalance.ToSatoshis() p2wkhBalance = ourBalance.ToSatoshis() } @@ -3143,12 +3141,12 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, switch { case ourBalance < ourInitialBalance && ourBalance < lnwire.NewMSatFromSatoshis( - lc.localChanCfg.ChanReserve): + lc.channelState.LocalChanCfg.ChanReserve): return ErrBelowChanReserve case theirBalance < theirInitialBalance && theirBalance < lnwire.NewMSatFromSatoshis( - lc.remoteChanCfg.ChanReserve): + lc.channelState.RemoteChanCfg.ChanReserve): return ErrBelowChanReserve } @@ -3199,7 +3197,7 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // First check that the remote updates won't violate it's channel // constraints. err := validateUpdates( - filteredView.theirUpdates, lc.remoteChanCfg, + filteredView.theirUpdates, &lc.channelState.RemoteChanCfg, ) if err != nil { return err @@ -3208,7 +3206,7 @@ func (lc *LightningChannel) validateCommitmentSanity(theirLogCounter, // Secondly check that our updates won't violate our channel // constraints. err = validateUpdates( - filteredView.ourUpdates, lc.localChanCfg, + filteredView.ourUpdates, &lc.channelState.LocalChanCfg, ) if err != nil { return err @@ -3276,7 +3274,7 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, []ch // construct the commitment state. keyRing := DeriveCommitmentKeys( commitPoint, false, lc.channelState.ChanType.IsTweakless(), - lc.localChanCfg, lc.remoteChanCfg, + &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) // Create a new commitment view which will calculate the evaluated @@ -3313,7 +3311,8 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, []ch // commitment state. We do so in two phases: first we generate and // submit the set of signature jobs to the worker pool. sigBatch, cancelChan, err := genRemoteHtlcSigJobs(keyRing, - lc.localChanCfg, lc.remoteChanCfg, newCommitView, + &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, + newCommitView, ) if err != nil { return sig, htlcSigs, nil, err @@ -3696,10 +3695,10 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, *htlcView) { commitChain := lc.localCommitChain - dustLimit := lc.localChanCfg.DustLimit + dustLimit := lc.channelState.LocalChanCfg.DustLimit if remoteChain { commitChain = lc.remoteCommitChain - dustLimit = lc.remoteChanCfg.DustLimit + dustLimit = lc.channelState.RemoteChanCfg.DustLimit } // Since the fetched htlc view will include all updates added after the @@ -4033,7 +4032,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, commitPoint := input.ComputeCommitmentPoint(commitSecret[:]) keyRing := DeriveCommitmentKeys( commitPoint, true, lc.channelState.ChanType.IsTweakless(), - lc.localChanCfg, lc.remoteChanCfg, + &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) // With the current commitment point re-calculated, construct the new @@ -4081,8 +4080,8 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, // pool to verify each of the HTLc signatures presented. Once // generated, we'll submit these jobs to the worker pool. verifyJobs, err := genHtlcSigValidationJobs( - localCommitmentView, keyRing, htlcSigs, lc.localChanCfg, - lc.remoteChanCfg, + localCommitmentView, keyRing, htlcSigs, + &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) if err != nil { return err @@ -4095,8 +4094,8 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, // we'll ensure that the newly constructed commitment state has a valid // signature. verifyKey := btcec.PublicKey{ - X: lc.remoteChanCfg.MultiSigKey.PubKey.X, - Y: lc.remoteChanCfg.MultiSigKey.PubKey.Y, + X: lc.channelState.RemoteChanCfg.MultiSigKey.PubKey.X, + Y: lc.channelState.RemoteChanCfg.MultiSigKey.PubKey.Y, Curve: btcec.S256(), } cSig, err := commitSig.ToSignature() @@ -5046,8 +5045,10 @@ func (lc *LightningChannel) getSignedCommitTx() (*wire.MsgTx, error) { // With the final signature generated, create the witness stack // required to spend from the multi-sig output. - ourKey := lc.localChanCfg.MultiSigKey.PubKey.SerializeCompressed() - theirKey := lc.remoteChanCfg.MultiSigKey.PubKey.SerializeCompressed() + ourKey := lc.channelState.LocalChanCfg.MultiSigKey.PubKey. + SerializeCompressed() + theirKey := lc.channelState.RemoteChanCfg.MultiSigKey.PubKey. + SerializeCompressed() commitTx.TxIn[0].Witness = input.SpendMultiSig( lc.signDesc.WitnessScript, ourKey, @@ -5911,8 +5912,8 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, } closeTx := CreateCooperativeCloseTx( - lc.fundingTxIn(), lc.localChanCfg.DustLimit, - lc.remoteChanCfg.DustLimit, ourBalance, theirBalance, + lc.fundingTxIn(), lc.channelState.LocalChanCfg.DustLimit, + lc.channelState.RemoteChanCfg.DustLimit, ourBalance, theirBalance, localDeliveryScript, remoteDeliveryScript, ) @@ -5982,8 +5983,8 @@ func (lc *LightningChannel) CompleteCooperativeClose(localSig, remoteSig []byte, // on this active channel back to both parties. In this current model, // the initiator pays full fees for the cooperative close transaction. closeTx := CreateCooperativeCloseTx( - lc.fundingTxIn(), lc.localChanCfg.DustLimit, - lc.remoteChanCfg.DustLimit, ourBalance, theirBalance, + lc.fundingTxIn(), lc.channelState.LocalChanCfg.DustLimit, + lc.channelState.RemoteChanCfg.DustLimit, ourBalance, theirBalance, localDeliveryScript, remoteDeliveryScript, ) @@ -5998,8 +5999,10 @@ func (lc *LightningChannel) CompleteCooperativeClose(localSig, remoteSig []byte, // Finally, construct the witness stack minding the order of the // pubkeys+sigs on the stack. - ourKey := lc.localChanCfg.MultiSigKey.PubKey.SerializeCompressed() - theirKey := lc.remoteChanCfg.MultiSigKey.PubKey.SerializeCompressed() + ourKey := lc.channelState.LocalChanCfg.MultiSigKey.PubKey. + SerializeCompressed() + theirKey := lc.channelState.RemoteChanCfg.MultiSigKey.PubKey. + SerializeCompressed() witness := input.SpendMultiSig(lc.signDesc.WitnessScript, ourKey, localSig, theirKey, remoteSig) closeTx.TxIn[0].Witness = witness @@ -6446,7 +6449,7 @@ func (lc *LightningChannel) ActiveHtlcs() []channeldb.HTLC { // LocalChanReserve returns our local ChanReserve requirement for the remote party. func (lc *LightningChannel) LocalChanReserve() btcutil.Amount { - return lc.localChanCfg.ChanReserve + return lc.channelState.LocalChanCfg.ChanReserve } // NextLocalHtlcIndex returns the next unallocated local htlc index. To ensure @@ -6471,5 +6474,5 @@ func (lc *LightningChannel) RemoteCommitHeight() uint64 { // FwdMinHtlc returns the minimum HTLC value required by the remote node, i.e. // the minimum value HTLC we can forward on this channel. func (lc *LightningChannel) FwdMinHtlc() lnwire.MilliSatoshi { - return lc.localChanCfg.MinHTLC + return lc.channelState.LocalChanCfg.MinHTLC } diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 665917cf4..c0467af9f 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -615,7 +615,7 @@ func TestForceClose(t *testing.T) { // Alice's listed CSV delay should also match the delay that was // pre-committed to at channel opening. if aliceCommitResolution.MaturityDelay != - uint32(aliceChannel.localChanCfg.CsvDelay) { + uint32(aliceChannel.channelState.LocalChanCfg.CsvDelay) { t.Fatalf("alice: incorrect local CSV delay in ForceCloseSummary, "+ "expected %v, got %v", @@ -816,10 +816,10 @@ func TestForceCloseDustOutput(t *testing.T) { // We set both node's channel reserves to 0, to make sure // they can create small dust ouputs without going under // their channel reserves. - aliceChannel.localChanCfg.ChanReserve = 0 - bobChannel.localChanCfg.ChanReserve = 0 - aliceChannel.remoteChanCfg.ChanReserve = 0 - bobChannel.remoteChanCfg.ChanReserve = 0 + aliceChannel.channelState.LocalChanCfg.ChanReserve = 0 + bobChannel.channelState.LocalChanCfg.ChanReserve = 0 + aliceChannel.channelState.RemoteChanCfg.ChanReserve = 0 + bobChannel.channelState.RemoteChanCfg.ChanReserve = 0 htlcAmount := lnwire.NewMSatFromSatoshis(500) @@ -1268,8 +1268,8 @@ func TestChannelBalanceDustLimit(t *testing.T) { // To allow Alice's balance to get beneath her dust limit, set the // channel reserve to be 0. - aliceChannel.localChanCfg.ChanReserve = 0 - bobChannel.remoteChanCfg.ChanReserve = 0 + aliceChannel.channelState.LocalChanCfg.ChanReserve = 0 + bobChannel.channelState.RemoteChanCfg.ChanReserve = 0 // This amount should leave an amount larger than Alice's dust limit // once fees have been subtracted, but smaller than Bob's dust limit. @@ -2553,7 +2553,7 @@ func TestAddHTLCNegativeBalance(t *testing.T) { // We set the channel reserve to 0, such that we can add HTLCs all the // way to a negative balance. - aliceChannel.localChanCfg.ChanReserve = 0 + aliceChannel.channelState.LocalChanCfg.ChanReserve = 0 // First, we'll add 3 HTLCs of 1 BTC each to Alice's commitment. const numHTLCs = 3 @@ -5346,14 +5346,14 @@ func TestMaxAcceptedHTLCs(t *testing.T) { // Set the remote's required MaxAcceptedHtlcs. This means that alice // can only offer the remote up to numHTLCs HTLCs. - aliceChannel.localChanCfg.MaxAcceptedHtlcs = numHTLCs - bobChannel.remoteChanCfg.MaxAcceptedHtlcs = numHTLCs + aliceChannel.channelState.LocalChanCfg.MaxAcceptedHtlcs = numHTLCs + bobChannel.channelState.RemoteChanCfg.MaxAcceptedHtlcs = numHTLCs // Similarly, set the remote config's MaxAcceptedHtlcs. This means // that the remote will be aware that Alice will only accept up to // numHTLCsRecevied at a time. - aliceChannel.remoteChanCfg.MaxAcceptedHtlcs = numHTLCsReceived - bobChannel.localChanCfg.MaxAcceptedHtlcs = numHTLCsReceived + aliceChannel.channelState.RemoteChanCfg.MaxAcceptedHtlcs = numHTLCsReceived + bobChannel.channelState.LocalChanCfg.MaxAcceptedHtlcs = numHTLCsReceived // Each HTLC amount is 0.1 BTC. htlcAmt := lnwire.NewMSatFromSatoshis(0.1 * btcutil.SatoshiPerBitcoin) @@ -5409,8 +5409,8 @@ func TestMaxPendingAmount(t *testing.T) { // We set the max pending amount of Alice's config. This mean that she // cannot offer Bob HTLCs with a total value above this limit at a given // time. - aliceChannel.localChanCfg.MaxPendingAmount = maxPending - bobChannel.remoteChanCfg.MaxPendingAmount = maxPending + aliceChannel.channelState.LocalChanCfg.MaxPendingAmount = maxPending + bobChannel.channelState.RemoteChanCfg.MaxPendingAmount = maxPending // First, we'll add 2 HTLCs of 1.5 BTC each to Alice's commitment. // This won't trigger Alice's ErrMaxPendingAmount error. @@ -5498,20 +5498,20 @@ func TestChanReserve(t *testing.T) { // Alice will need to keep her reserve above aliceMinReserve, // so set this limit to here local config. - aliceChannel.localChanCfg.ChanReserve = aliceMinReserve + aliceChannel.channelState.LocalChanCfg.ChanReserve = aliceMinReserve // During channel opening Bob will also get to know Alice's // minimum reserve, and this will be found in his remote // config. - bobChannel.remoteChanCfg.ChanReserve = aliceMinReserve + bobChannel.channelState.RemoteChanCfg.ChanReserve = aliceMinReserve // We set Bob's channel reserve to a value that is larger than // his current balance in the channel. This will ensure that // after a channel is first opened, Bob can still receive HTLCs // even though his balance is less than his channel reserve. bobMinReserve := btcutil.Amount(6 * btcutil.SatoshiPerBitcoin) - bobChannel.localChanCfg.ChanReserve = bobMinReserve - aliceChannel.remoteChanCfg.ChanReserve = bobMinReserve + bobChannel.channelState.LocalChanCfg.ChanReserve = bobMinReserve + aliceChannel.channelState.RemoteChanCfg.ChanReserve = bobMinReserve return aliceChannel, bobChannel, cleanUp } @@ -5713,8 +5713,8 @@ func TestMinHTLC(t *testing.T) { // Setting the min value in Alice's local config means that the // remote will not accept any HTLCs of value less than specified. - aliceChannel.localChanCfg.MinHTLC = minValue - bobChannel.remoteChanCfg.MinHTLC = minValue + aliceChannel.channelState.LocalChanCfg.MinHTLC = minValue + bobChannel.channelState.RemoteChanCfg.MinHTLC = minValue // First, we will add an HTLC of 0.5 BTC. This will not trigger // ErrBelowMinHTLC. diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index ba6d07265..47022e866 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -421,10 +421,8 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { // Construct a LightningChannel manually because we don't have nor need all // of the dependencies. channel := LightningChannel{ - channelState: &channelState, - Signer: signer, - localChanCfg: &channelState.LocalChanCfg, - remoteChanCfg: &channelState.RemoteChanCfg, + channelState: &channelState, + Signer: signer, } err = channel.createSignDesc() if err != nil { @@ -845,8 +843,8 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { // commitment tx. htlcResolutions, err := extractHtlcResolutions( chainfee.SatPerKWeight(test.commitment.FeePerKw), true, signer, - htlcs, keys, channel.localChanCfg, channel.remoteChanCfg, - commitTx.TxHash(), + htlcs, keys, &channel.channelState.LocalChanCfg, + &channel.channelState.RemoteChanCfg, commitTx.TxHash(), ) if err != nil { t.Errorf("Case %d: Failed to extract HTLC resolutions: %v", i, err) From fff9dbe6f3a35cf3ff9e7beefc332029d07e8ae3 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:03 +0100 Subject: [PATCH 05/17] lnwallet: pass chan cfgs to CreateCommitTx Instead of passing delays and dustlimits separately, we pass the correct channel config to CreateCommitTx from the POV of the local party that owns the commit tx. To make it more clear which commitment we are actually creating, we rename variables to denote local and remote, to prepare for the case when both outputs might be delayed. --- lnwallet/channel.go | 79 ++++++++++++++++++++--------------- lnwallet/transactions_test.go | 20 +++++++-- lnwallet/wallet.go | 14 ++++--- 3 files changed, 70 insertions(+), 43 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 4c5bace65..ab8410313 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2428,23 +2428,27 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, } var ( - delay uint32 - delayBalance, p2wkhBalance btcutil.Amount + commitTx *wire.MsgTx + err error ) - if c.isOurs { - delay = uint32(lc.channelState.LocalChanCfg.CsvDelay) - delayBalance = ourBalance.ToSatoshis() - p2wkhBalance = theirBalance.ToSatoshis() - } else { - delay = uint32(lc.channelState.RemoteChanCfg.CsvDelay) - delayBalance = theirBalance.ToSatoshis() - p2wkhBalance = ourBalance.ToSatoshis() - } - // Generate a new commitment transaction with all the latest - // unsettled/un-timed out HTLCs. - commitTx, err := CreateCommitTx(lc.fundingTxIn(), keyRing, delay, - delayBalance, p2wkhBalance, c.dustLimit) + // Depending on whether the transaction is ours or not, we call + // CreateCommitTx with parameters mathcing the perspective, to generate + // a new commitment transaction with all the latest unsettled/un-timed + // out HTLCs. + if c.isOurs { + commitTx, err = CreateCommitTx( + lc.fundingTxIn(), keyRing, &lc.channelState.LocalChanCfg, + &lc.channelState.RemoteChanCfg, ourBalance.ToSatoshis(), + theirBalance.ToSatoshis(), + ) + } else { + commitTx, err = CreateCommitTx( + lc.fundingTxIn(), keyRing, &lc.channelState.RemoteChanCfg, + &lc.channelState.LocalChanCfg, theirBalance.ToSatoshis(), + ourBalance.ToSatoshis(), + ) + } if err != nil { return err } @@ -6216,32 +6220,39 @@ func (lc *LightningChannel) generateRevocation(height uint64) (*lnwire.RevokeAnd } // CreateCommitTx creates a commitment transaction, spending from specified -// funding output. The commitment transaction contains two outputs: one paying -// to the "owner" of the commitment transaction which can be spent after a -// relative block delay or revocation event, and the other paying the -// counterparty within the channel, which can be spent immediately. -func CreateCommitTx(fundingOutput wire.TxIn, - keyRing *CommitmentKeyRing, csvTimeout uint32, - amountToSelf, amountToThem, dustLimit btcutil.Amount) (*wire.MsgTx, error) { +// funding output. The commitment transaction contains two outputs: one local +// output paying to the "owner" of the commitment transaction which can be +// spent after a relative block delay or revocation event, and a remote output +// paying the counterparty within the channel, which can be spent immediately +// or after a delay depending on the commitment type.. +func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, + localChanCfg, remoteChanCfg *channeldb.ChannelConfig, + amountToLocal, amountToRemote btcutil.Amount) (*wire.MsgTx, error) { // First, we create the script for the delayed "pay-to-self" output. // This output has 2 main redemption clauses: either we can redeem the // output after a relative block delay, or the remote node can claim // the funds with the revocation key if we broadcast a revoked // commitment transaction. - ourRedeemScript, err := input.CommitScriptToSelf(csvTimeout, keyRing.DelayKey, - keyRing.RevocationKey) + toLocalRedeemScript, err := input.CommitScriptToSelf( + uint32(localChanCfg.CsvDelay), keyRing.DelayKey, + keyRing.RevocationKey, + ) if err != nil { return nil, err } - payToUsScriptHash, err := input.WitnessScriptHash(ourRedeemScript) + toLocalScriptHash, err := input.WitnessScriptHash( + toLocalRedeemScript, + ) if err != nil { return nil, err } - // Next, we create the script paying to them. This is just a regular - // P2WPKH output, without any added CSV delay. - theirWitnessKeyHash, err := input.CommitScriptUnencumbered(keyRing.NoDelayKey) + // Next, we create the script paying to the remote. This is just a + // regular P2WPKH output, without any added CSV delay. + toRemoteWitnessKeyHash, err := input.CommitScriptUnencumbered( + keyRing.NoDelayKey, + ) if err != nil { return nil, err } @@ -6253,16 +6264,16 @@ func CreateCommitTx(fundingOutput wire.TxIn, commitTx.AddTxIn(&fundingOutput) // Avoid creating dust outputs within the commitment transaction. - if amountToSelf >= dustLimit { + if amountToLocal >= localChanCfg.DustLimit { commitTx.AddTxOut(&wire.TxOut{ - PkScript: payToUsScriptHash, - Value: int64(amountToSelf), + PkScript: toLocalScriptHash, + Value: int64(amountToLocal), }) } - if amountToThem >= dustLimit { + if amountToRemote >= localChanCfg.DustLimit { commitTx.AddTxOut(&wire.TxOut{ - PkScript: theirWitnessKeyHash, - Value: int64(amountToThem), + PkScript: toRemoteWitnessKeyHash, + Value: int64(amountToRemote), }) } diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 47022e866..24c2f4550 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -1015,7 +1015,7 @@ func testSpendValidation(t *testing.T, tweakless bool) { fakeFundingTxIn := wire.NewTxIn(fundingOut, nil, nil) const channelBalance = btcutil.Amount(1 * 10e8) - const csvTimeout = uint32(5) + const csvTimeout = 5 // We also set up set some resources for the commitment transaction. // Each side currently has 1 BTC within the channel, with a total @@ -1050,6 +1050,20 @@ func testSpendValidation(t *testing.T, tweakless bool) { Privkeys: []*btcec.PrivateKey{aliceKeyPriv}, } + aliceChanCfg := &channeldb.ChannelConfig{ + ChannelConstraints: channeldb.ChannelConstraints{ + DustLimit: DefaultDustLimit(), + CsvDelay: csvTimeout, + }, + } + + bobChanCfg := &channeldb.ChannelConfig{ + ChannelConstraints: channeldb.ChannelConstraints{ + DustLimit: DefaultDustLimit(), + CsvDelay: csvTimeout, + }, + } + // With all the test data set up, we create the commitment transaction. // We only focus on a single party's transactions, as the scripts are // identical with the roles reversed. @@ -1063,8 +1077,8 @@ func testSpendValidation(t *testing.T, tweakless bool) { NoDelayKey: bobPayKey, } commitmentTx, err := CreateCommitTx( - *fakeFundingTxIn, keyRing, csvTimeout, channelBalance, - channelBalance, DefaultDustLimit(), + *fakeFundingTxIn, keyRing, aliceChanCfg, bobChanCfg, + channelBalance, channelBalance, ) if err != nil { t.Fatalf("unable to create commitment transaction: %v", nil) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index a6b9b8753..61b7cfeea 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -784,9 +784,10 @@ func CreateCommitmentTxns(localBalance, remoteBalance btcutil.Amount, theirChanCfg, ) - ourCommitTx, err := CreateCommitTx(fundingTxIn, localCommitmentKeys, - uint32(ourChanCfg.CsvDelay), localBalance, remoteBalance, - ourChanCfg.DustLimit) + ourCommitTx, err := CreateCommitTx( + fundingTxIn, localCommitmentKeys, ourChanCfg, theirChanCfg, + localBalance, remoteBalance, + ) if err != nil { return nil, nil, err } @@ -796,9 +797,10 @@ func CreateCommitmentTxns(localBalance, remoteBalance btcutil.Amount, return nil, nil, err } - theirCommitTx, err := CreateCommitTx(fundingTxIn, remoteCommitmentKeys, - uint32(theirChanCfg.CsvDelay), remoteBalance, localBalance, - theirChanCfg.DustLimit) + theirCommitTx, err := CreateCommitTx( + fundingTxIn, remoteCommitmentKeys, theirChanCfg, ourChanCfg, + remoteBalance, localBalance, + ) if err != nil { return nil, nil, err } From 83e0d47ba3afb3a0f4ca8896ca1b044520c66186 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:03 +0100 Subject: [PATCH 06/17] lnwallet: move methods to commitment.go PURE CODE MOVE: Moving createCommitmentTx, CreateCommitTx, createStateHintObfuscator, CommitmentKeyRing, DeriveCommitmentKeys, addHTLC, genHtlcScripts We move the methods and structs to a new file commitment.go in preparation for defining all the logic that is dependent on the channel type in this new file. --- lnwallet/channel.go | 447 --------------------------------------- lnwallet/commitment.go | 460 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 460 insertions(+), 447 deletions(-) create mode 100644 lnwallet/commitment.go diff --git a/lnwallet/channel.go b/lnwallet/channel.go index ab8410313..1c2b10d51 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -937,121 +937,6 @@ func (lc *LightningChannel) diskCommitToMemCommit(isLocal bool, return commit, nil } -// CommitmentKeyRing holds all derived keys needed to construct commitment and -// HTLC transactions. The keys are derived differently depending whether the -// commitment transaction is ours or the remote peer's. Private keys associated -// with each key may belong to the commitment owner or the "other party" which -// is referred to in the field comments, regardless of which is local and which -// is remote. -type CommitmentKeyRing struct { - // commitPoint is the "per commitment point" used to derive the tweak - // for each base point. - CommitPoint *btcec.PublicKey - - // LocalCommitKeyTweak is the tweak used to derive the local public key - // from the local payment base point or the local private key from the - // base point secret. This may be included in a SignDescriptor to - // generate signatures for the local payment key. - LocalCommitKeyTweak []byte - - // TODO(roasbeef): need delay tweak as well? - - // LocalHtlcKeyTweak is the teak used to derive the local HTLC key from - // the local HTLC base point. This value is needed in order to - // derive the final key used within the HTLC scripts in the commitment - // transaction. - LocalHtlcKeyTweak []byte - - // LocalHtlcKey is the key that will be used in the "to self" clause of - // any HTLC scripts within the commitment transaction for this key ring - // set. - LocalHtlcKey *btcec.PublicKey - - // RemoteHtlcKey is the key that will be used in clauses within the - // HTLC script that send money to the remote party. - RemoteHtlcKey *btcec.PublicKey - - // DelayKey is the commitment transaction owner's key which is included - // in HTLC success and timeout transaction scripts. - DelayKey *btcec.PublicKey - - // NoDelayKey is the other party's payment key in the commitment tx. - // This is the key used to generate the unencumbered output within the - // commitment transaction. - NoDelayKey *btcec.PublicKey - - // RevocationKey is the key that can be used by the other party to - // redeem outputs from a revoked commitment transaction if it were to - // be published. - RevocationKey *btcec.PublicKey -} - -// DeriveCommitmentKey generates a new commitment key set using the base points -// and commitment point. The keys are derived differently depending whether the -// commitment transaction is ours or the remote peer's. -func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, - isOurCommit, tweaklessCommit bool, - localChanCfg, remoteChanCfg *channeldb.ChannelConfig) *CommitmentKeyRing { - - // First, we'll derive all the keys that don't depend on the context of - // whose commitment transaction this is. - keyRing := &CommitmentKeyRing{ - CommitPoint: commitPoint, - - LocalCommitKeyTweak: input.SingleTweakBytes( - commitPoint, localChanCfg.PaymentBasePoint.PubKey, - ), - LocalHtlcKeyTweak: input.SingleTweakBytes( - commitPoint, localChanCfg.HtlcBasePoint.PubKey, - ), - LocalHtlcKey: input.TweakPubKey( - localChanCfg.HtlcBasePoint.PubKey, commitPoint, - ), - RemoteHtlcKey: input.TweakPubKey( - remoteChanCfg.HtlcBasePoint.PubKey, commitPoint, - ), - } - - // We'll now compute the delay, no delay, and revocation key based on - // the current commitment point. All keys are tweaked each state in - // order to ensure the keys from each state are unlinkable. To create - // the revocation key, we take the opposite party's revocation base - // point and combine that with the current commitment point. - var ( - delayBasePoint *btcec.PublicKey - noDelayBasePoint *btcec.PublicKey - revocationBasePoint *btcec.PublicKey - ) - if isOurCommit { - delayBasePoint = localChanCfg.DelayBasePoint.PubKey - noDelayBasePoint = remoteChanCfg.PaymentBasePoint.PubKey - revocationBasePoint = remoteChanCfg.RevocationBasePoint.PubKey - } else { - delayBasePoint = remoteChanCfg.DelayBasePoint.PubKey - noDelayBasePoint = localChanCfg.PaymentBasePoint.PubKey - revocationBasePoint = localChanCfg.RevocationBasePoint.PubKey - } - - // With the base points assigned, we can now derive the actual keys - // using the base point, and the current commitment tweak. - keyRing.DelayKey = input.TweakPubKey(delayBasePoint, commitPoint) - keyRing.RevocationKey = input.DeriveRevocationPubkey( - revocationBasePoint, commitPoint, - ) - - // If this commitment should omit the tweak for the remote point, then - // we'll use that directly, and ignore the commitPoint tweak. - if tweaklessCommit { - keyRing.NoDelayKey = noDelayBasePoint - } else { - keyRing.NoDelayKey = input.TweakPubKey( - noDelayBasePoint, commitPoint, - ) - } - - return keyRing -} - // commitmentChain represents a chain of unrevoked commitments. The tail of the // chain is the latest fully signed, yet unrevoked commitment. Two chains are // tracked, one for the local node, and another for the remote node. New @@ -1473,24 +1358,6 @@ func (lc *LightningChannel) createSignDesc() error { return nil } -// createStateHintObfuscator derives and assigns the state hint obfuscator for -// the channel, which is used to encode the commitment height in the sequence -// number of commitment transaction inputs. -func (lc *LightningChannel) createStateHintObfuscator() { - state := lc.channelState - if state.IsInitiator { - lc.stateHintObfuscator = DeriveStateHintObfuscator( - state.LocalChanCfg.PaymentBasePoint.PubKey, - state.RemoteChanCfg.PaymentBasePoint.PubKey, - ) - } else { - lc.stateHintObfuscator = DeriveStateHintObfuscator( - state.RemoteChanCfg.PaymentBasePoint.PubKey, - state.LocalChanCfg.PaymentBasePoint.PubKey, - ) - } -} - // ResetState resets the state of the channel back to the default state. This // ensures that any active goroutines which need to act based on on-chain // events do so properly. @@ -2370,164 +2237,6 @@ func (lc *LightningChannel) fundingTxIn() wire.TxIn { return *wire.NewTxIn(&lc.channelState.FundingOutpoint, nil, nil) } -// createCommitmentTx generates the unsigned commitment transaction for a -// commitment view and assigns to txn field. -func (lc *LightningChannel) createCommitmentTx(c *commitment, - filteredHTLCView *htlcView, keyRing *CommitmentKeyRing) error { - - ourBalance := c.ourBalance - theirBalance := c.theirBalance - - numHTLCs := int64(0) - for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust(false, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { - - continue - } - - numHTLCs++ - } - for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust(true, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { - - continue - } - - numHTLCs++ - } - - // Next, we'll calculate the fee for the commitment transaction based - // on its total weight. Once we have the total weight, we'll multiply - // by the current fee-per-kw, then divide by 1000 to get the proper - // fee. - totalCommitWeight := input.CommitWeight + (input.HtlcWeight * numHTLCs) - - // With the weight known, we can now calculate the commitment fee, - // ensuring that we account for any dust outputs trimmed above. - commitFee := c.feePerKw.FeeForWeight(totalCommitWeight) - commitFeeMSat := lnwire.NewMSatFromSatoshis(commitFee) - - // Currently, within the protocol, the initiator always pays the fees. - // So we'll subtract the fee amount from the balance of the current - // initiator. If the initiator is unable to pay the fee fully, then - // their entire output is consumed. - switch { - case lc.channelState.IsInitiator && commitFee > ourBalance.ToSatoshis(): - ourBalance = 0 - - case lc.channelState.IsInitiator: - ourBalance -= commitFeeMSat - - case !lc.channelState.IsInitiator && commitFee > theirBalance.ToSatoshis(): - theirBalance = 0 - - case !lc.channelState.IsInitiator: - theirBalance -= commitFeeMSat - } - - var ( - commitTx *wire.MsgTx - err error - ) - - // Depending on whether the transaction is ours or not, we call - // CreateCommitTx with parameters mathcing the perspective, to generate - // a new commitment transaction with all the latest unsettled/un-timed - // out HTLCs. - if c.isOurs { - commitTx, err = CreateCommitTx( - lc.fundingTxIn(), keyRing, &lc.channelState.LocalChanCfg, - &lc.channelState.RemoteChanCfg, ourBalance.ToSatoshis(), - theirBalance.ToSatoshis(), - ) - } else { - commitTx, err = CreateCommitTx( - lc.fundingTxIn(), keyRing, &lc.channelState.RemoteChanCfg, - &lc.channelState.LocalChanCfg, theirBalance.ToSatoshis(), - ourBalance.ToSatoshis(), - ) - } - if err != nil { - return err - } - - // We'll now add all the HTLC outputs to the commitment transaction. - // Each output includes an off-chain 2-of-2 covenant clause, so we'll - // need the objective local/remote keys for this particular commitment - // as well. For any non-dust HTLCs that are manifested on the commitment - // transaction, we'll also record its CLTV which is required to sort the - // commitment transaction below. The slice is initially sized to the - // number of existing outputs, since any outputs already added are - // commitment outputs and should correspond to zero values for the - // purposes of sorting. - cltvs := make([]uint32, len(commitTx.TxOut)) - for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust(false, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { - continue - } - - err := addHTLC(commitTx, c.isOurs, false, htlc, keyRing) - if err != nil { - return err - } - cltvs = append(cltvs, htlc.Timeout) - } - for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust(true, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { - continue - } - - err := addHTLC(commitTx, c.isOurs, true, htlc, keyRing) - if err != nil { - return err - } - cltvs = append(cltvs, htlc.Timeout) - } - - // Set the state hint of the commitment transaction to facilitate - // quickly recovering the necessary penalty state in the case of an - // uncooperative broadcast. - err = SetStateNumHint(commitTx, c.height, lc.stateHintObfuscator) - if err != nil { - return err - } - - // Sort the transactions according to the agreed upon canonical - // ordering. This lets us skip sending the entire transaction over, - // instead we'll just send signatures. - InPlaceCommitSort(commitTx, cltvs) - - // Next, we'll ensure that we don't accidentally create a commitment - // transaction which would be invalid by consensus. - uTx := btcutil.NewTx(commitTx) - if err := blockchain.CheckTransactionSanity(uTx); err != nil { - return err - } - - // Finally, we'll assert that were not attempting to draw more out of - // the channel that was originally placed within it. - var totalOut btcutil.Amount - for _, txOut := range commitTx.TxOut { - totalOut += btcutil.Amount(txOut.Value) - } - if totalOut > lc.channelState.Capacity { - return fmt.Errorf("height=%v, for ChannelPoint(%v) attempts "+ - "to consume %v while channel capacity is %v", - c.height, lc.channelState.FundingOutpoint, - totalOut, lc.channelState.Capacity) - } - - c.txn = commitTx - c.fee = commitFee - c.ourBalance = ourBalance - c.theirBalance = theirBalance - return nil -} - // evaluateHTLCView processes all update entries in both HTLC update logs, // producing a final view which is the result of properly applying all adds, // settles, timeouts and fee updates found in both logs. The resulting view @@ -4933,101 +4642,6 @@ func (lc *LightningChannel) RemoteUpfrontShutdownScript() lnwire.DeliveryAddress return lc.channelState.RemoteShutdownScript } -// genHtlcScript generates the proper P2WSH public key scripts for the HTLC -// output modified by two-bits denoting if this is an incoming HTLC, and if the -// HTLC is being applied to their commitment transaction or ours. -func genHtlcScript(isIncoming, ourCommit bool, timeout uint32, rHash [32]byte, - keyRing *CommitmentKeyRing) ([]byte, []byte, error) { - - var ( - witnessScript []byte - err error - ) - - // Generate the proper redeem scripts for the HTLC output modified by - // two-bits denoting if this is an incoming HTLC, and if the HTLC is - // being applied to their commitment transaction or ours. - switch { - // The HTLC is paying to us, and being applied to our commitment - // transaction. So we need to use the receiver's version of HTLC the - // script. - case isIncoming && ourCommit: - witnessScript, err = input.ReceiverHTLCScript(timeout, - keyRing.RemoteHtlcKey, keyRing.LocalHtlcKey, - keyRing.RevocationKey, rHash[:]) - - // We're being paid via an HTLC by the remote party, and the HTLC is - // being added to their commitment transaction, so we use the sender's - // version of the HTLC script. - case isIncoming && !ourCommit: - witnessScript, err = input.SenderHTLCScript(keyRing.RemoteHtlcKey, - keyRing.LocalHtlcKey, keyRing.RevocationKey, rHash[:]) - - // We're sending an HTLC which is being added to our commitment - // transaction. Therefore, we need to use the sender's version of the - // HTLC script. - case !isIncoming && ourCommit: - witnessScript, err = input.SenderHTLCScript(keyRing.LocalHtlcKey, - keyRing.RemoteHtlcKey, keyRing.RevocationKey, rHash[:]) - - // Finally, we're paying the remote party via an HTLC, which is being - // added to their commitment transaction. Therefore, we use the - // receiver's version of the HTLC script. - case !isIncoming && !ourCommit: - witnessScript, err = input.ReceiverHTLCScript(timeout, keyRing.LocalHtlcKey, - keyRing.RemoteHtlcKey, keyRing.RevocationKey, rHash[:]) - } - if err != nil { - return nil, nil, err - } - - // Now that we have the redeem scripts, create the P2WSH public key - // script for the output itself. - htlcP2WSH, err := input.WitnessScriptHash(witnessScript) - if err != nil { - return nil, nil, err - } - - return htlcP2WSH, witnessScript, nil -} - -// addHTLC adds a new HTLC to the passed commitment transaction. One of four -// full scripts will be generated for the HTLC output depending on if the HTLC -// is incoming and if it's being applied to our commitment transaction or that -// of the remote node's. Additionally, in order to be able to efficiently -// locate the added HTLC on the commitment transaction from the -// PaymentDescriptor that generated it, the generated script is stored within -// the descriptor itself. -func addHTLC(commitTx *wire.MsgTx, ourCommit bool, - isIncoming bool, paymentDesc *PaymentDescriptor, - keyRing *CommitmentKeyRing) error { - - timeout := paymentDesc.Timeout - rHash := paymentDesc.RHash - - p2wsh, witnessScript, err := genHtlcScript(isIncoming, ourCommit, - timeout, rHash, keyRing) - if err != nil { - return err - } - - // Add the new HTLC outputs to the respective commitment transactions. - amountPending := int64(paymentDesc.Amount.ToSatoshis()) - commitTx.AddTxOut(wire.NewTxOut(amountPending, p2wsh)) - - // Store the pkScript of this particular PaymentDescriptor so we can - // quickly locate it within the commitment transaction later. - if ourCommit { - paymentDesc.ourPkScript = p2wsh - paymentDesc.ourWitnessScript = witnessScript - } else { - paymentDesc.theirPkScript = p2wsh - paymentDesc.theirWitnessScript = witnessScript - } - - return nil -} - // getSignedCommitTx function take the latest commitment transaction and // populate it with witness data. func (lc *LightningChannel) getSignedCommitTx() (*wire.MsgTx, error) { @@ -6219,67 +5833,6 @@ func (lc *LightningChannel) generateRevocation(height uint64) (*lnwire.RevokeAnd return revocationMsg, nil } -// CreateCommitTx creates a commitment transaction, spending from specified -// funding output. The commitment transaction contains two outputs: one local -// output paying to the "owner" of the commitment transaction which can be -// spent after a relative block delay or revocation event, and a remote output -// paying the counterparty within the channel, which can be spent immediately -// or after a delay depending on the commitment type.. -func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, - localChanCfg, remoteChanCfg *channeldb.ChannelConfig, - amountToLocal, amountToRemote btcutil.Amount) (*wire.MsgTx, error) { - - // First, we create the script for the delayed "pay-to-self" output. - // This output has 2 main redemption clauses: either we can redeem the - // output after a relative block delay, or the remote node can claim - // the funds with the revocation key if we broadcast a revoked - // commitment transaction. - toLocalRedeemScript, err := input.CommitScriptToSelf( - uint32(localChanCfg.CsvDelay), keyRing.DelayKey, - keyRing.RevocationKey, - ) - if err != nil { - return nil, err - } - toLocalScriptHash, err := input.WitnessScriptHash( - toLocalRedeemScript, - ) - if err != nil { - return nil, err - } - - // Next, we create the script paying to the remote. This is just a - // regular P2WPKH output, without any added CSV delay. - toRemoteWitnessKeyHash, err := input.CommitScriptUnencumbered( - keyRing.NoDelayKey, - ) - if err != nil { - return nil, err - } - - // Now that both output scripts have been created, we can finally create - // the transaction itself. We use a transaction version of 2 since CSV - // will fail unless the tx version is >= 2. - commitTx := wire.NewMsgTx(2) - commitTx.AddTxIn(&fundingOutput) - - // Avoid creating dust outputs within the commitment transaction. - if amountToLocal >= localChanCfg.DustLimit { - commitTx.AddTxOut(&wire.TxOut{ - PkScript: toLocalScriptHash, - Value: int64(amountToLocal), - }) - } - if amountToRemote >= localChanCfg.DustLimit { - commitTx.AddTxOut(&wire.TxOut{ - PkScript: toRemoteWitnessKeyHash, - Value: int64(amountToRemote), - }) - } - - return commitTx, nil -} - // CreateCooperativeCloseTx creates a transaction which if signed by both // parties, then broadcast cooperatively closes an active channel. The creation // of the closure transaction is modified by a boolean indicating if the party diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go new file mode 100644 index 000000000..bc8bd1615 --- /dev/null +++ b/lnwallet/commitment.go @@ -0,0 +1,460 @@ +package lnwallet + +import ( + "fmt" + + "github.com/btcsuite/btcd/blockchain" + "github.com/btcsuite/btcd/btcec" + "github.com/btcsuite/btcd/wire" + "github.com/btcsuite/btcutil" + "github.com/lightningnetwork/lnd/channeldb" + "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwire" +) + +// CommitmentKeyRing holds all derived keys needed to construct commitment and +// HTLC transactions. The keys are derived differently depending whether the +// commitment transaction is ours or the remote peer's. Private keys associated +// with each key may belong to the commitment owner or the "other party" which +// is referred to in the field comments, regardless of which is local and which +// is remote. +type CommitmentKeyRing struct { + // commitPoint is the "per commitment point" used to derive the tweak + // for each base point. + CommitPoint *btcec.PublicKey + + // LocalCommitKeyTweak is the tweak used to derive the local public key + // from the local payment base point or the local private key from the + // base point secret. This may be included in a SignDescriptor to + // generate signatures for the local payment key. + LocalCommitKeyTweak []byte + + // TODO(roasbeef): need delay tweak as well? + + // LocalHtlcKeyTweak is the teak used to derive the local HTLC key from + // the local HTLC base point. This value is needed in order to + // derive the final key used within the HTLC scripts in the commitment + // transaction. + LocalHtlcKeyTweak []byte + + // LocalHtlcKey is the key that will be used in the "to self" clause of + // any HTLC scripts within the commitment transaction for this key ring + // set. + LocalHtlcKey *btcec.PublicKey + + // RemoteHtlcKey is the key that will be used in clauses within the + // HTLC script that send money to the remote party. + RemoteHtlcKey *btcec.PublicKey + + // DelayKey is the commitment transaction owner's key which is included + // in HTLC success and timeout transaction scripts. + DelayKey *btcec.PublicKey + + // NoDelayKey is the other party's payment key in the commitment tx. + // This is the key used to generate the unencumbered output within the + // commitment transaction. + NoDelayKey *btcec.PublicKey + + // RevocationKey is the key that can be used by the other party to + // redeem outputs from a revoked commitment transaction if it were to + // be published. + RevocationKey *btcec.PublicKey +} + +// DeriveCommitmentKey generates a new commitment key set using the base points +// and commitment point. The keys are derived differently depending whether the +// commitment transaction is ours or the remote peer's. +func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, + isOurCommit, tweaklessCommit bool, + localChanCfg, remoteChanCfg *channeldb.ChannelConfig) *CommitmentKeyRing { + + // First, we'll derive all the keys that don't depend on the context of + // whose commitment transaction this is. + keyRing := &CommitmentKeyRing{ + CommitPoint: commitPoint, + + LocalCommitKeyTweak: input.SingleTweakBytes( + commitPoint, localChanCfg.PaymentBasePoint.PubKey, + ), + LocalHtlcKeyTweak: input.SingleTweakBytes( + commitPoint, localChanCfg.HtlcBasePoint.PubKey, + ), + LocalHtlcKey: input.TweakPubKey( + localChanCfg.HtlcBasePoint.PubKey, commitPoint, + ), + RemoteHtlcKey: input.TweakPubKey( + remoteChanCfg.HtlcBasePoint.PubKey, commitPoint, + ), + } + + // We'll now compute the delay, no delay, and revocation key based on + // the current commitment point. All keys are tweaked each state in + // order to ensure the keys from each state are unlinkable. To create + // the revocation key, we take the opposite party's revocation base + // point and combine that with the current commitment point. + var ( + delayBasePoint *btcec.PublicKey + noDelayBasePoint *btcec.PublicKey + revocationBasePoint *btcec.PublicKey + ) + if isOurCommit { + delayBasePoint = localChanCfg.DelayBasePoint.PubKey + noDelayBasePoint = remoteChanCfg.PaymentBasePoint.PubKey + revocationBasePoint = remoteChanCfg.RevocationBasePoint.PubKey + } else { + delayBasePoint = remoteChanCfg.DelayBasePoint.PubKey + noDelayBasePoint = localChanCfg.PaymentBasePoint.PubKey + revocationBasePoint = localChanCfg.RevocationBasePoint.PubKey + } + + // With the base points assigned, we can now derive the actual keys + // using the base point, and the current commitment tweak. + keyRing.DelayKey = input.TweakPubKey(delayBasePoint, commitPoint) + keyRing.RevocationKey = input.DeriveRevocationPubkey( + revocationBasePoint, commitPoint, + ) + + // If this commitment should omit the tweak for the remote point, then + // we'll use that directly, and ignore the commitPoint tweak. + if tweaklessCommit { + keyRing.NoDelayKey = noDelayBasePoint + } else { + keyRing.NoDelayKey = input.TweakPubKey( + noDelayBasePoint, commitPoint, + ) + } + + return keyRing +} + +// createStateHintObfuscator derives and assigns the state hint obfuscator for +// the channel, which is used to encode the commitment height in the sequence +// number of commitment transaction inputs. +func (lc *LightningChannel) createStateHintObfuscator() { + state := lc.channelState + if state.IsInitiator { + lc.stateHintObfuscator = DeriveStateHintObfuscator( + state.LocalChanCfg.PaymentBasePoint.PubKey, + state.RemoteChanCfg.PaymentBasePoint.PubKey, + ) + } else { + lc.stateHintObfuscator = DeriveStateHintObfuscator( + state.RemoteChanCfg.PaymentBasePoint.PubKey, + state.LocalChanCfg.PaymentBasePoint.PubKey, + ) + } +} + +// createCommitmentTx generates the unsigned commitment transaction for a +// commitment view and assigns to txn field. +func (lc *LightningChannel) createCommitmentTx(c *commitment, + filteredHTLCView *htlcView, keyRing *CommitmentKeyRing) error { + + ourBalance := c.ourBalance + theirBalance := c.theirBalance + + numHTLCs := int64(0) + for _, htlc := range filteredHTLCView.ourUpdates { + if htlcIsDust(false, c.isOurs, c.feePerKw, + htlc.Amount.ToSatoshis(), c.dustLimit) { + + continue + } + + numHTLCs++ + } + for _, htlc := range filteredHTLCView.theirUpdates { + if htlcIsDust(true, c.isOurs, c.feePerKw, + htlc.Amount.ToSatoshis(), c.dustLimit) { + + continue + } + + numHTLCs++ + } + + // Next, we'll calculate the fee for the commitment transaction based + // on its total weight. Once we have the total weight, we'll multiply + // by the current fee-per-kw, then divide by 1000 to get the proper + // fee. + totalCommitWeight := input.CommitWeight + (input.HtlcWeight * numHTLCs) + + // With the weight known, we can now calculate the commitment fee, + // ensuring that we account for any dust outputs trimmed above. + commitFee := c.feePerKw.FeeForWeight(totalCommitWeight) + commitFeeMSat := lnwire.NewMSatFromSatoshis(commitFee) + + // Currently, within the protocol, the initiator always pays the fees. + // So we'll subtract the fee amount from the balance of the current + // initiator. If the initiator is unable to pay the fee fully, then + // their entire output is consumed. + switch { + case lc.channelState.IsInitiator && commitFee > ourBalance.ToSatoshis(): + ourBalance = 0 + + case lc.channelState.IsInitiator: + ourBalance -= commitFeeMSat + + case !lc.channelState.IsInitiator && commitFee > theirBalance.ToSatoshis(): + theirBalance = 0 + + case !lc.channelState.IsInitiator: + theirBalance -= commitFeeMSat + } + + var ( + commitTx *wire.MsgTx + err error + ) + + // Depending on whether the transaction is ours or not, we call + // CreateCommitTx with parameters mathcing the perspective, to generate + // a new commitment transaction with all the latest unsettled/un-timed + // out HTLCs. + if c.isOurs { + commitTx, err = CreateCommitTx( + lc.fundingTxIn(), keyRing, &lc.channelState.LocalChanCfg, + &lc.channelState.RemoteChanCfg, ourBalance.ToSatoshis(), + theirBalance.ToSatoshis(), + ) + } else { + commitTx, err = CreateCommitTx( + lc.fundingTxIn(), keyRing, &lc.channelState.RemoteChanCfg, + &lc.channelState.LocalChanCfg, theirBalance.ToSatoshis(), + ourBalance.ToSatoshis(), + ) + } + if err != nil { + return err + } + + // We'll now add all the HTLC outputs to the commitment transaction. + // Each output includes an off-chain 2-of-2 covenant clause, so we'll + // need the objective local/remote keys for this particular commitment + // as well. For any non-dust HTLCs that are manifested on the commitment + // transaction, we'll also record its CLTV which is required to sort the + // commitment transaction below. The slice is initially sized to the + // number of existing outputs, since any outputs already added are + // commitment outputs and should correspond to zero values for the + // purposes of sorting. + cltvs := make([]uint32, len(commitTx.TxOut)) + for _, htlc := range filteredHTLCView.ourUpdates { + if htlcIsDust(false, c.isOurs, c.feePerKw, + htlc.Amount.ToSatoshis(), c.dustLimit) { + continue + } + + err := addHTLC(commitTx, c.isOurs, false, htlc, keyRing) + if err != nil { + return err + } + cltvs = append(cltvs, htlc.Timeout) + } + for _, htlc := range filteredHTLCView.theirUpdates { + if htlcIsDust(true, c.isOurs, c.feePerKw, + htlc.Amount.ToSatoshis(), c.dustLimit) { + continue + } + + err := addHTLC(commitTx, c.isOurs, true, htlc, keyRing) + if err != nil { + return err + } + cltvs = append(cltvs, htlc.Timeout) + } + + // Set the state hint of the commitment transaction to facilitate + // quickly recovering the necessary penalty state in the case of an + // uncooperative broadcast. + err = SetStateNumHint(commitTx, c.height, lc.stateHintObfuscator) + if err != nil { + return err + } + + // Sort the transactions according to the agreed upon canonical + // ordering. This lets us skip sending the entire transaction over, + // instead we'll just send signatures. + InPlaceCommitSort(commitTx, cltvs) + + // Next, we'll ensure that we don't accidentally create a commitment + // transaction which would be invalid by consensus. + uTx := btcutil.NewTx(commitTx) + if err := blockchain.CheckTransactionSanity(uTx); err != nil { + return err + } + + // Finally, we'll assert that were not attempting to draw more out of + // the channel that was originally placed within it. + var totalOut btcutil.Amount + for _, txOut := range commitTx.TxOut { + totalOut += btcutil.Amount(txOut.Value) + } + if totalOut > lc.channelState.Capacity { + return fmt.Errorf("height=%v, for ChannelPoint(%v) attempts "+ + "to consume %v while channel capacity is %v", + c.height, lc.channelState.FundingOutpoint, + totalOut, lc.channelState.Capacity) + } + + c.txn = commitTx + c.fee = commitFee + c.ourBalance = ourBalance + c.theirBalance = theirBalance + return nil +} + +// CreateCommitTx creates a commitment transaction, spending from specified +// funding output. The commitment transaction contains two outputs: one local +// output paying to the "owner" of the commitment transaction which can be +// spent after a relative block delay or revocation event, and a remote output +// paying the counterparty within the channel, which can be spent immediately +// or after a delay depending on the commitment type.. +func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, + localChanCfg, remoteChanCfg *channeldb.ChannelConfig, + amountToLocal, amountToRemote btcutil.Amount) (*wire.MsgTx, error) { + + // First, we create the script for the delayed "pay-to-self" output. + // This output has 2 main redemption clauses: either we can redeem the + // output after a relative block delay, or the remote node can claim + // the funds with the revocation key if we broadcast a revoked + // commitment transaction. + toLocalRedeemScript, err := input.CommitScriptToSelf( + uint32(localChanCfg.CsvDelay), keyRing.DelayKey, + keyRing.RevocationKey, + ) + if err != nil { + return nil, err + } + toLocalScriptHash, err := input.WitnessScriptHash( + toLocalRedeemScript, + ) + if err != nil { + return nil, err + } + + // Next, we create the script paying to the remote. This is just a + // regular P2WPKH output, without any added CSV delay. + toRemoteWitnessKeyHash, err := input.CommitScriptUnencumbered( + keyRing.NoDelayKey, + ) + if err != nil { + return nil, err + } + + // Now that both output scripts have been created, we can finally create + // the transaction itself. We use a transaction version of 2 since CSV + // will fail unless the tx version is >= 2. + commitTx := wire.NewMsgTx(2) + commitTx.AddTxIn(&fundingOutput) + + // Avoid creating dust outputs within the commitment transaction. + if amountToLocal >= localChanCfg.DustLimit { + commitTx.AddTxOut(&wire.TxOut{ + PkScript: toLocalScriptHash, + Value: int64(amountToLocal), + }) + } + if amountToRemote >= localChanCfg.DustLimit { + commitTx.AddTxOut(&wire.TxOut{ + PkScript: toRemoteWitnessKeyHash, + Value: int64(amountToRemote), + }) + } + + return commitTx, nil +} + +// genHtlcScript generates the proper P2WSH public key scripts for the HTLC +// output modified by two-bits denoting if this is an incoming HTLC, and if the +// HTLC is being applied to their commitment transaction or ours. +func genHtlcScript(isIncoming, ourCommit bool, timeout uint32, rHash [32]byte, + keyRing *CommitmentKeyRing) ([]byte, []byte, error) { + + var ( + witnessScript []byte + err error + ) + + // Generate the proper redeem scripts for the HTLC output modified by + // two-bits denoting if this is an incoming HTLC, and if the HTLC is + // being applied to their commitment transaction or ours. + switch { + // The HTLC is paying to us, and being applied to our commitment + // transaction. So we need to use the receiver's version of HTLC the + // script. + case isIncoming && ourCommit: + witnessScript, err = input.ReceiverHTLCScript(timeout, + keyRing.RemoteHtlcKey, keyRing.LocalHtlcKey, + keyRing.RevocationKey, rHash[:]) + + // We're being paid via an HTLC by the remote party, and the HTLC is + // being added to their commitment transaction, so we use the sender's + // version of the HTLC script. + case isIncoming && !ourCommit: + witnessScript, err = input.SenderHTLCScript(keyRing.RemoteHtlcKey, + keyRing.LocalHtlcKey, keyRing.RevocationKey, rHash[:]) + + // We're sending an HTLC which is being added to our commitment + // transaction. Therefore, we need to use the sender's version of the + // HTLC script. + case !isIncoming && ourCommit: + witnessScript, err = input.SenderHTLCScript(keyRing.LocalHtlcKey, + keyRing.RemoteHtlcKey, keyRing.RevocationKey, rHash[:]) + + // Finally, we're paying the remote party via an HTLC, which is being + // added to their commitment transaction. Therefore, we use the + // receiver's version of the HTLC script. + case !isIncoming && !ourCommit: + witnessScript, err = input.ReceiverHTLCScript(timeout, keyRing.LocalHtlcKey, + keyRing.RemoteHtlcKey, keyRing.RevocationKey, rHash[:]) + } + if err != nil { + return nil, nil, err + } + + // Now that we have the redeem scripts, create the P2WSH public key + // script for the output itself. + htlcP2WSH, err := input.WitnessScriptHash(witnessScript) + if err != nil { + return nil, nil, err + } + + return htlcP2WSH, witnessScript, nil +} + +// addHTLC adds a new HTLC to the passed commitment transaction. One of four +// full scripts will be generated for the HTLC output depending on if the HTLC +// is incoming and if it's being applied to our commitment transaction or that +// of the remote node's. Additionally, in order to be able to efficiently +// locate the added HTLC on the commitment transaction from the +// PaymentDescriptor that generated it, the generated script is stored within +// the descriptor itself. +func addHTLC(commitTx *wire.MsgTx, ourCommit bool, + isIncoming bool, paymentDesc *PaymentDescriptor, + keyRing *CommitmentKeyRing) error { + + timeout := paymentDesc.Timeout + rHash := paymentDesc.RHash + + p2wsh, witnessScript, err := genHtlcScript(isIncoming, ourCommit, + timeout, rHash, keyRing) + if err != nil { + return err + } + + // Add the new HTLC outputs to the respective commitment transactions. + amountPending := int64(paymentDesc.Amount.ToSatoshis()) + commitTx.AddTxOut(wire.NewTxOut(amountPending, p2wsh)) + + // Store the pkScript of this particular PaymentDescriptor so we can + // quickly locate it within the commitment transaction later. + if ourCommit { + paymentDesc.ourPkScript = p2wsh + paymentDesc.ourWitnessScript = witnessScript + } else { + paymentDesc.theirPkScript = p2wsh + paymentDesc.theirWitnessScript = witnessScript + } + + return nil +} From 13a108e578356753d868e5a2609c9401b5a1dfcc Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:03 +0100 Subject: [PATCH 07/17] lnwallet: make fundingTxIn not depend on LightningWallet --- lnwallet/channel.go | 8 ++++---- lnwallet/commitment.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 1c2b10d51..4caa8bbfa 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2233,8 +2233,8 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, return c, nil } -func (lc *LightningChannel) fundingTxIn() wire.TxIn { - return *wire.NewTxIn(&lc.channelState.FundingOutpoint, nil, nil) +func fundingTxIn(chanState *channeldb.OpenChannel) wire.TxIn { + return *wire.NewTxIn(&chanState.FundingOutpoint, nil, nil) } // evaluateHTLCView processes all update entries in both HTLC update logs, @@ -5530,7 +5530,7 @@ func (lc *LightningChannel) CreateCloseProposal(proposedFee btcutil.Amount, } closeTx := CreateCooperativeCloseTx( - lc.fundingTxIn(), lc.channelState.LocalChanCfg.DustLimit, + fundingTxIn(lc.channelState), lc.channelState.LocalChanCfg.DustLimit, lc.channelState.RemoteChanCfg.DustLimit, ourBalance, theirBalance, localDeliveryScript, remoteDeliveryScript, ) @@ -5601,7 +5601,7 @@ func (lc *LightningChannel) CompleteCooperativeClose(localSig, remoteSig []byte, // on this active channel back to both parties. In this current model, // the initiator pays full fees for the cooperative close transaction. closeTx := CreateCooperativeCloseTx( - lc.fundingTxIn(), lc.channelState.LocalChanCfg.DustLimit, + fundingTxIn(lc.channelState), lc.channelState.LocalChanCfg.DustLimit, lc.channelState.RemoteChanCfg.DustLimit, ourBalance, theirBalance, localDeliveryScript, remoteDeliveryScript, ) diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index bc8bd1615..8e08e86db 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -213,13 +213,13 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, // out HTLCs. if c.isOurs { commitTx, err = CreateCommitTx( - lc.fundingTxIn(), keyRing, &lc.channelState.LocalChanCfg, + fundingTxIn(lc.channelState), keyRing, &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ourBalance.ToSatoshis(), theirBalance.ToSatoshis(), ) } else { commitTx, err = CreateCommitTx( - lc.fundingTxIn(), keyRing, &lc.channelState.RemoteChanCfg, + fundingTxIn(lc.channelState), keyRing, &lc.channelState.RemoteChanCfg, &lc.channelState.LocalChanCfg, theirBalance.ToSatoshis(), ourBalance.ToSatoshis(), ) From 613d771dafd2d89db29442923a677eca5b3e91ad Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:03 +0100 Subject: [PATCH 08/17] lnwallet: create CommitmentBuilder We define a new struct CommitmentBuilder that will be used to craft the final commitment transaction based on the current active channel type. --- lnwallet/channel.go | 14 ++++---- lnwallet/commitment.go | 65 +++++++++++++++++++++++------------ lnwallet/test_utils.go | 6 ++-- lnwallet/transactions_test.go | 8 ++--- 4 files changed, 58 insertions(+), 35 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 4caa8bbfa..d39dd84bb 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1226,10 +1226,6 @@ type LightningChannel struct { // Capacity is the total capacity of this channel. Capacity btcutil.Amount - // stateHintObfuscator is a 48-bit state hint that's used to obfuscate - // the current state number on the commitment transactions. - stateHintObfuscator [StateHintSize]byte - // currentHeight is the current height of our local commitment chain. // This is also the same as the number of updates to the channel we've // accepted. @@ -1247,6 +1243,8 @@ type LightningChannel struct { channelState *channeldb.OpenChannel + commitBuilder *CommitmentBuilder + // [local|remote]Log is a (mostly) append-only log storing all the HTLC // updates to this channel. The log is walked backwards as HTLC updates // are applied in order to re-construct a commitment transaction from a @@ -1299,6 +1297,7 @@ func NewLightningChannel(signer input.Signer, remoteCommitChain: newCommitmentChain(), localCommitChain: newCommitmentChain(), channelState: state, + commitBuilder: NewCommitmentBuilder(state), localUpdateLog: localUpdateLog, remoteUpdateLog: remoteUpdateLog, ChanPoint: &state.FundingOutpoint, @@ -1322,8 +1321,6 @@ func NewLightningChannel(signer input.Signer, return nil, err } - lc.createStateHintObfuscator() - return lc, nil } @@ -2208,7 +2205,10 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, } // Actually generate unsigned commitment transaction for this view. - if err := lc.createCommitmentTx(c, filteredHTLCView, keyRing); err != nil { + err := lc.commitBuilder.createCommitmentTx( + c, filteredHTLCView, keyRing, + ) + if err != nil { return nil, err } diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 8e08e86db..e163fae0e 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -127,27 +127,48 @@ func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, return keyRing } +// CommitmentBuilder is a type that wraps the type of channel we are dealing +// with, and abstracts the various ways of constructing commitment +// transactions. +type CommitmentBuilder struct { + // chanState is the underlying channels's state struct, used to + // determine the type of channel we are dealing with, and relevant + // parameters. + chanState *channeldb.OpenChannel + + // obfuscator is a 48-bit state hint that's used to obfuscate the + // current state number on the commitment transactions. + obfuscator [StateHintSize]byte +} + +// NewCommitmentBuilder creates a new CommitmentBuilder from chanState. +func NewCommitmentBuilder(chanState *channeldb.OpenChannel) *CommitmentBuilder { + return &CommitmentBuilder{ + chanState: chanState, + obfuscator: createStateHintObfuscator(chanState), + } +} + // createStateHintObfuscator derives and assigns the state hint obfuscator for // the channel, which is used to encode the commitment height in the sequence // number of commitment transaction inputs. -func (lc *LightningChannel) createStateHintObfuscator() { - state := lc.channelState +func createStateHintObfuscator(state *channeldb.OpenChannel) [StateHintSize]byte { if state.IsInitiator { - lc.stateHintObfuscator = DeriveStateHintObfuscator( + return DeriveStateHintObfuscator( state.LocalChanCfg.PaymentBasePoint.PubKey, state.RemoteChanCfg.PaymentBasePoint.PubKey, ) - } else { - lc.stateHintObfuscator = DeriveStateHintObfuscator( - state.RemoteChanCfg.PaymentBasePoint.PubKey, - state.LocalChanCfg.PaymentBasePoint.PubKey, - ) } + + return DeriveStateHintObfuscator( + state.RemoteChanCfg.PaymentBasePoint.PubKey, + state.LocalChanCfg.PaymentBasePoint.PubKey, + ) } // createCommitmentTx generates the unsigned commitment transaction for a // commitment view and assigns to txn field. -func (lc *LightningChannel) createCommitmentTx(c *commitment, +func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, filteredHTLCView *htlcView, keyRing *CommitmentKeyRing) error { ourBalance := c.ourBalance @@ -189,16 +210,16 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, // initiator. If the initiator is unable to pay the fee fully, then // their entire output is consumed. switch { - case lc.channelState.IsInitiator && commitFee > ourBalance.ToSatoshis(): + case cb.chanState.IsInitiator && commitFee > ourBalance.ToSatoshis(): ourBalance = 0 - case lc.channelState.IsInitiator: + case cb.chanState.IsInitiator: ourBalance -= commitFeeMSat - case !lc.channelState.IsInitiator && commitFee > theirBalance.ToSatoshis(): + case !cb.chanState.IsInitiator && commitFee > theirBalance.ToSatoshis(): theirBalance = 0 - case !lc.channelState.IsInitiator: + case !cb.chanState.IsInitiator: theirBalance -= commitFeeMSat } @@ -208,19 +229,19 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, ) // Depending on whether the transaction is ours or not, we call - // CreateCommitTx with parameters mathcing the perspective, to generate + // CreateCommitTx with parameters matching the perspective, to generate // a new commitment transaction with all the latest unsettled/un-timed // out HTLCs. if c.isOurs { commitTx, err = CreateCommitTx( - fundingTxIn(lc.channelState), keyRing, &lc.channelState.LocalChanCfg, - &lc.channelState.RemoteChanCfg, ourBalance.ToSatoshis(), + fundingTxIn(cb.chanState), keyRing, &cb.chanState.LocalChanCfg, + &cb.chanState.RemoteChanCfg, ourBalance.ToSatoshis(), theirBalance.ToSatoshis(), ) } else { commitTx, err = CreateCommitTx( - fundingTxIn(lc.channelState), keyRing, &lc.channelState.RemoteChanCfg, - &lc.channelState.LocalChanCfg, theirBalance.ToSatoshis(), + fundingTxIn(cb.chanState), keyRing, &cb.chanState.RemoteChanCfg, + &cb.chanState.LocalChanCfg, theirBalance.ToSatoshis(), ourBalance.ToSatoshis(), ) } @@ -266,7 +287,7 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, // Set the state hint of the commitment transaction to facilitate // quickly recovering the necessary penalty state in the case of an // uncooperative broadcast. - err = SetStateNumHint(commitTx, c.height, lc.stateHintObfuscator) + err = SetStateNumHint(commitTx, c.height, cb.obfuscator) if err != nil { return err } @@ -289,11 +310,11 @@ func (lc *LightningChannel) createCommitmentTx(c *commitment, for _, txOut := range commitTx.TxOut { totalOut += btcutil.Amount(txOut.Value) } - if totalOut > lc.channelState.Capacity { + if totalOut > cb.chanState.Capacity { return fmt.Errorf("height=%v, for ChannelPoint(%v) attempts "+ "to consume %v while channel capacity is %v", - c.height, lc.channelState.FundingOutpoint, - totalOut, lc.channelState.Capacity) + c.height, cb.chanState.FundingOutpoint, + totalOut, cb.chanState.Capacity) } c.txn = commitTx diff --git a/lnwallet/test_utils.go b/lnwallet/test_utils.go index d9f8d3cab..f5df61f68 100644 --- a/lnwallet/test_utils.go +++ b/lnwallet/test_utils.go @@ -324,6 +324,8 @@ func CreateTestChannels(tweaklessCommits bool) ( } alicePool.Start() + obfuscator := createStateHintObfuscator(aliceChannelState) + bobPool := NewSigPool(1, bobSigner) channelBob, err := NewLightningChannel( bobSigner, bobChannelState, bobPool, @@ -334,13 +336,13 @@ func CreateTestChannels(tweaklessCommits bool) ( bobPool.Start() err = SetStateNumHint( - aliceCommitTx, 0, channelAlice.stateHintObfuscator, + aliceCommitTx, 0, obfuscator, ) if err != nil { return nil, nil, nil, err } err = SetStateNumHint( - bobCommitTx, 0, channelAlice.stateHintObfuscator, + bobCommitTx, 0, obfuscator, ) if err != nil { return nil, nil, nil, err diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 24c2f4550..294f5f08b 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -421,14 +421,14 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { // Construct a LightningChannel manually because we don't have nor need all // of the dependencies. channel := LightningChannel{ - channelState: &channelState, - Signer: signer, + channelState: &channelState, + Signer: signer, + commitBuilder: NewCommitmentBuilder(&channelState), } err = channel.createSignDesc() if err != nil { t.Fatalf("Failed to generate channel sign descriptor: %v", err) } - channel.createStateHintObfuscator() // The commitmentPoint is technically hidden in the spec, but we need it to // generate the correct tweak. @@ -803,7 +803,7 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { dustLimit: tc.dustLimit, isOurs: true, } - err = channel.createCommitmentTx( + err = channel.commitBuilder.createCommitmentTx( commitmentView, theHTLCView, keys, ) if err != nil { From 76ce51301ed3ff716e4d4cb82eb88682b25df122 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:03 +0100 Subject: [PATCH 09/17] lnwallet: return instead evaluated commitment instead of mutating createCommitmentTx would earlier mutate the passed commitment struct after evaluating the htlc view and calculating the final balances, which was confusing since the balances are supposed to only be *after* subtracting fees. Instead we take the needed parameters as arguments, and return the final balances, tx and fee to populate the commitment struct in a proper way. --- lnwallet/channel.go | 36 +++++++------- lnwallet/commitment.go | 89 ++++++++++++++++++++++------------- lnwallet/transactions_test.go | 29 ++++++++---- 3 files changed, 94 insertions(+), 60 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index d39dd84bb..0c0f854fa 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -2167,33 +2167,41 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, keyRing *CommitmentKeyRing) (*commitment, error) { commitChain := lc.localCommitChain + dustLimit := lc.channelState.LocalChanCfg.DustLimit if remoteChain { commitChain = lc.remoteCommitChain + dustLimit = lc.channelState.RemoteChanCfg.DustLimit } nextHeight := commitChain.tip().height + 1 // Run through all the HTLCs that will be covered by this transaction // in order to update their commitment addition height, and to adjust - // the balances on the commitment transaction accordingly. + // the balances on the commitment transaction accordingly. Note that + // these balances will be *before* taking a commitment fee from the + // initiator. htlcView := lc.fetchHTLCView(theirLogIndex, ourLogIndex) ourBalance, theirBalance, _, filteredHTLCView := lc.computeView( htlcView, remoteChain, true, ) feePerKw := filteredHTLCView.feePerKw - // Determine how many current HTLCs are over the dust limit, and should - // be counted for the purpose of fee calculation. - var dustLimit btcutil.Amount - if remoteChain { - dustLimit = lc.channelState.RemoteChanCfg.DustLimit - } else { - dustLimit = lc.channelState.LocalChanCfg.DustLimit + // Actually generate unsigned commitment transaction for this view. + commitTx, err := lc.commitBuilder.createUnsignedCommitmentTx( + ourBalance, theirBalance, !remoteChain, feePerKw, nextHeight, + filteredHTLCView, keyRing, + ) + if err != nil { + return nil, err } + // With the commitment view created, store the resulting balances and + // transaction with the other parameters for this height. c := &commitment{ - ourBalance: ourBalance, - theirBalance: theirBalance, + ourBalance: commitTx.ourBalance, + theirBalance: commitTx.theirBalance, + txn: commitTx.txn, + fee: commitTx.fee, ourMessageIndex: ourLogIndex, ourHtlcIndex: ourHtlcIndex, theirMessageIndex: theirLogIndex, @@ -2204,14 +2212,6 @@ func (lc *LightningChannel) fetchCommitmentView(remoteChain bool, isOurs: !remoteChain, } - // Actually generate unsigned commitment transaction for this view. - err := lc.commitBuilder.createCommitmentTx( - c, filteredHTLCView, keyRing, - ) - if err != nil { - return nil, err - } - // In order to ensure _none_ of the HTLC's associated with this new // commitment are mutated, we'll manually copy over each HTLC to its // respective slice. diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index e163fae0e..c3abe2e18 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -9,6 +9,7 @@ import ( "github.com/btcsuite/btcutil" "github.com/lightningnetwork/lnd/channeldb" "github.com/lightningnetwork/lnd/input" + "github.com/lightningnetwork/lnd/lnwallet/chainfee" "github.com/lightningnetwork/lnd/lnwire" ) @@ -166,18 +167,41 @@ func createStateHintObfuscator(state *channeldb.OpenChannel) [StateHintSize]byte ) } -// createCommitmentTx generates the unsigned commitment transaction for a -// commitment view and assigns to txn field. -func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, - filteredHTLCView *htlcView, keyRing *CommitmentKeyRing) error { +// unsignedCommitmentTx is the final commitment created from evaluating an HTLC +// view at a given height, along with some meta data. +type unsignedCommitmentTx struct { + // txn is the final, unsigned commitment transaction for this view. + txn *wire.MsgTx - ourBalance := c.ourBalance - theirBalance := c.theirBalance + // fee is the total fee of the commitment transaction. + fee btcutil.Amount + + // ourBalance|theirBalance is the balances of this commitment. This can + // be different than the balances before creating the commitment + // transaction as one party must pay the commitment fee. + ourBalance lnwire.MilliSatoshi + theirBalance lnwire.MilliSatoshi +} + +// createUnsignedCommitmentTx generates the unsigned commitment transaction for +// a commitment view and returns it as part of the unsignedCommitmentTx. The +// passed in balances should be balances *before* subtracting any commitment +// fees. +func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, + theirBalance lnwire.MilliSatoshi, isOurs bool, + feePerKw chainfee.SatPerKWeight, height uint64, + filteredHTLCView *htlcView, + keyRing *CommitmentKeyRing) (*unsignedCommitmentTx, error) { + + dustLimit := cb.chanState.LocalChanCfg.DustLimit + if !isOurs { + dustLimit = cb.chanState.RemoteChanCfg.DustLimit + } numHTLCs := int64(0) for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust(false, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(false, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } @@ -185,8 +209,8 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, numHTLCs++ } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust(true, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(true, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } @@ -202,7 +226,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // With the weight known, we can now calculate the commitment fee, // ensuring that we account for any dust outputs trimmed above. - commitFee := c.feePerKw.FeeForWeight(totalCommitWeight) + commitFee := feePerKw.FeeForWeight(totalCommitWeight) commitFeeMSat := lnwire.NewMSatFromSatoshis(commitFee) // Currently, within the protocol, the initiator always pays the fees. @@ -232,7 +256,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // CreateCommitTx with parameters matching the perspective, to generate // a new commitment transaction with all the latest unsettled/un-timed // out HTLCs. - if c.isOurs { + if isOurs { commitTx, err = CreateCommitTx( fundingTxIn(cb.chanState), keyRing, &cb.chanState.LocalChanCfg, &cb.chanState.RemoteChanCfg, ourBalance.ToSatoshis(), @@ -246,7 +270,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, ) } if err != nil { - return err + return nil, err } // We'll now add all the HTLC outputs to the commitment transaction. @@ -260,26 +284,26 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // purposes of sorting. cltvs := make([]uint32, len(commitTx.TxOut)) for _, htlc := range filteredHTLCView.ourUpdates { - if htlcIsDust(false, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(false, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } - err := addHTLC(commitTx, c.isOurs, false, htlc, keyRing) + err := addHTLC(commitTx, isOurs, false, htlc, keyRing) if err != nil { - return err + return nil, err } cltvs = append(cltvs, htlc.Timeout) } for _, htlc := range filteredHTLCView.theirUpdates { - if htlcIsDust(true, c.isOurs, c.feePerKw, - htlc.Amount.ToSatoshis(), c.dustLimit) { + if htlcIsDust(true, isOurs, feePerKw, + htlc.Amount.ToSatoshis(), dustLimit) { continue } - err := addHTLC(commitTx, c.isOurs, true, htlc, keyRing) + err := addHTLC(commitTx, isOurs, true, htlc, keyRing) if err != nil { - return err + return nil, err } cltvs = append(cltvs, htlc.Timeout) } @@ -287,9 +311,9 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // Set the state hint of the commitment transaction to facilitate // quickly recovering the necessary penalty state in the case of an // uncooperative broadcast. - err = SetStateNumHint(commitTx, c.height, cb.obfuscator) + err = SetStateNumHint(commitTx, height, cb.obfuscator) if err != nil { - return err + return nil, err } // Sort the transactions according to the agreed upon canonical @@ -301,7 +325,7 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, // transaction which would be invalid by consensus. uTx := btcutil.NewTx(commitTx) if err := blockchain.CheckTransactionSanity(uTx); err != nil { - return err + return nil, err } // Finally, we'll assert that were not attempting to draw more out of @@ -311,17 +335,18 @@ func (cb *CommitmentBuilder) createCommitmentTx(c *commitment, totalOut += btcutil.Amount(txOut.Value) } if totalOut > cb.chanState.Capacity { - return fmt.Errorf("height=%v, for ChannelPoint(%v) attempts "+ - "to consume %v while channel capacity is %v", - c.height, cb.chanState.FundingOutpoint, + return nil, fmt.Errorf("height=%v, for ChannelPoint(%v) "+ + "attempts to consume %v while channel capacity is %v", + height, cb.chanState.FundingOutpoint, totalOut, cb.chanState.Capacity) } - c.txn = commitTx - c.fee = commitFee - c.ourBalance = ourBalance - c.theirBalance = theirBalance - return nil + return &unsignedCommitmentTx{ + txn: commitTx, + fee: commitFee, + ourBalance: ourBalance, + theirBalance: theirBalance, + }, nil } // CreateCommitTx creates a commitment transaction, spending from specified diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 294f5f08b..2ef864197 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -794,23 +794,32 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { } theHTLCView := htlcViewFromHTLCs(htlcs) + feePerKw := chainfee.SatPerKWeight(test.commitment.FeePerKw) + isOurs := true + height := test.commitment.CommitHeight + // Create unsigned commitment transaction. - commitmentView := &commitment{ - height: test.commitment.CommitHeight, - ourBalance: test.commitment.LocalBalance, - theirBalance: test.commitment.RemoteBalance, - feePerKw: chainfee.SatPerKWeight(test.commitment.FeePerKw), - dustLimit: tc.dustLimit, - isOurs: true, - } - err = channel.commitBuilder.createCommitmentTx( - commitmentView, theHTLCView, keys, + view, err := channel.commitBuilder.createUnsignedCommitmentTx( + test.commitment.LocalBalance, + test.commitment.RemoteBalance, isOurs, feePerKw, + height, theHTLCView, keys, ) if err != nil { t.Errorf("Case %d: Failed to create new commitment tx: %v", i, err) continue } + commitmentView := &commitment{ + ourBalance: view.ourBalance, + theirBalance: view.theirBalance, + txn: view.txn, + fee: view.fee, + height: height, + feePerKw: feePerKw, + dustLimit: tc.dustLimit, + isOurs: isOurs, + } + // Initialize LocalCommit, which is used in getSignedCommitTx. channelState.LocalCommitment = test.commitment channelState.LocalCommitment.Htlcs = htlcs From 7b9c54996bec16f1f979d19d4e5b3c348e932a19 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:03 +0100 Subject: [PATCH 10/17] lnwallet/commitment: clarify keyring key ownership --- lnwallet/commitment.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index c3abe2e18..bf95dd598 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -20,7 +20,7 @@ import ( // is referred to in the field comments, regardless of which is local and which // is remote. type CommitmentKeyRing struct { - // commitPoint is the "per commitment point" used to derive the tweak + // CommitPoint is the "per commitment point" used to derive the tweak // for each base point. CommitPoint *btcec.PublicKey @@ -28,23 +28,35 @@ type CommitmentKeyRing struct { // from the local payment base point or the local private key from the // base point secret. This may be included in a SignDescriptor to // generate signatures for the local payment key. + // + // NOTE: This will always refer to "our" local key, regardless of + // whether this is our commit or not. LocalCommitKeyTweak []byte // TODO(roasbeef): need delay tweak as well? - // LocalHtlcKeyTweak is the teak used to derive the local HTLC key from - // the local HTLC base point. This value is needed in order to + // LocalHtlcKeyTweak is the tweak used to derive the local HTLC key + // from the local HTLC base point. This value is needed in order to // derive the final key used within the HTLC scripts in the commitment // transaction. + // + // NOTE: This will always refer to "our" local HTLC key, regardless of + // whether this is our commit or not. LocalHtlcKeyTweak []byte - // LocalHtlcKey is the key that will be used in the "to self" clause of - // any HTLC scripts within the commitment transaction for this key ring - // set. + // LocalHtlcKey is the key that will be used in any clause paying to + // our node of any HTLC scripts within the commitment transaction for + // this key ring set. + // + // NOTE: This will always refer to "our" local HTLC key, regardless of + // whether this is our commit or not. LocalHtlcKey *btcec.PublicKey // RemoteHtlcKey is the key that will be used in clauses within the // HTLC script that send money to the remote party. + // + // NOTE: This will always refer to "their" remote HTLC key, regardless + // of whether this is our commit or not. RemoteHtlcKey *btcec.PublicKey // DelayKey is the commitment transaction owner's key which is included From 4fde31229cd7ee7be8b6dae0e9180d163d74ef2d Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:04 +0100 Subject: [PATCH 11/17] lnwallet: rename DelayKey->ToLocalKey, NoDelayKey->ToRemoteKey Since both parties are going to have their ouputs delayed, we move way from the DelayKey naming, and instead use ToLocalKey and ToRemoteKey. --- contractcourt/chain_watcher.go | 8 +-- lnwallet/channel.go | 26 +++++----- lnwallet/commitment.go | 52 ++++++++++++------- lnwallet/transactions_test.go | 8 +-- watchtower/wtclient/backup_task.go | 4 +- .../wtclient/backup_task_internal_test.go | 8 +-- watchtower/wtclient/client_test.go | 4 +- 7 files changed, 61 insertions(+), 49 deletions(-) diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index 7ea3da142..7a226de25 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -351,7 +351,7 @@ func isOurCommitment(localChanCfg, remoteChanCfg channeldb.ChannelConfig, // With the keys derived, we'll construct the remote script that'll be // present if they have a non-dust balance on the commitment. remotePkScript, err := input.CommitScriptUnencumbered( - commitKeyRing.NoDelayKey, + commitKeyRing.ToRemoteKey, ) if err != nil { return false, err @@ -361,7 +361,7 @@ func isOurCommitment(localChanCfg, remoteChanCfg channeldb.ChannelConfig, // the remote party allowing them to claim this output before the CSV // delay if we breach. localScript, err := input.CommitScriptToSelf( - uint32(localChanCfg.CsvDelay), commitKeyRing.DelayKey, + uint32(localChanCfg.CsvDelay), commitKeyRing.ToLocalKey, commitKeyRing.RevocationKey, ) if err != nil { @@ -928,8 +928,8 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail retribution.KeyRing.CommitPoint.Curve = nil retribution.KeyRing.LocalHtlcKey = nil retribution.KeyRing.RemoteHtlcKey = nil - retribution.KeyRing.DelayKey = nil - retribution.KeyRing.NoDelayKey = nil + retribution.KeyRing.ToLocalKey = nil + retribution.KeyRing.ToRemoteKey = nil retribution.KeyRing.RevocationKey = nil return spew.Sdump(retribution) })) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 0c0f854fa..eb78cdd23 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1880,7 +1880,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // within the final witness. remoteDelay := uint32(chanState.RemoteChanCfg.CsvDelay) remotePkScript, err := input.CommitScriptToSelf( - remoteDelay, keyRing.DelayKey, keyRing.RevocationKey, + remoteDelay, keyRing.ToLocalKey, keyRing.RevocationKey, ) if err != nil { return nil, err @@ -1889,7 +1889,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, if err != nil { return nil, err } - localPkScript, err := input.CommitScriptUnencumbered(keyRing.NoDelayKey) + localPkScript, err := input.CommitScriptUnencumbered(keyRing.ToRemoteKey) if err != nil { return nil, err } @@ -1984,7 +1984,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // remote commitment transaction, and *they* go to the second // level. secondLevelWitnessScript, err := input.SecondLevelHtlcScript( - keyRing.RevocationKey, keyRing.DelayKey, remoteDelay, + keyRing.RevocationKey, keyRing.ToLocalKey, remoteDelay, ) if err != nil { return nil, err @@ -2563,7 +2563,7 @@ func genRemoteHtlcSigJobs(keyRing *CommitmentKeyRing, sigJob.Tx, err = createHtlcTimeoutTx( op, outputAmt, htlc.Timeout, uint32(remoteChanCfg.CsvDelay), - keyRing.RevocationKey, keyRing.DelayKey, + keyRing.RevocationKey, keyRing.ToLocalKey, ) if err != nil { return nil, nil, err @@ -2614,7 +2614,7 @@ func genRemoteHtlcSigJobs(keyRing *CommitmentKeyRing, } sigJob.Tx, err = createHtlcSuccessTx( op, outputAmt, uint32(remoteChanCfg.CsvDelay), - keyRing.RevocationKey, keyRing.DelayKey, + keyRing.RevocationKey, keyRing.ToLocalKey, ) if err != nil { return nil, nil, err @@ -3526,7 +3526,7 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment, successTx, err := createHtlcSuccessTx(op, outputAmt, uint32(localChanCfg.CsvDelay), - keyRing.RevocationKey, keyRing.DelayKey) + keyRing.RevocationKey, keyRing.ToLocalKey) if err != nil { return nil, err } @@ -3579,7 +3579,7 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment, timeoutTx, err := createHtlcTimeoutTx(op, outputAmt, htlc.Timeout, uint32(localChanCfg.CsvDelay), - keyRing.RevocationKey, keyRing.DelayKey, + keyRing.RevocationKey, keyRing.ToLocalKey, ) if err != nil { return nil, err @@ -4772,7 +4772,7 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // Before we can generate the proper sign descriptor, we'll need to // locate the output index of our non-delayed output on the commitment // transaction. - selfP2WKH, err := input.CommitScriptUnencumbered(keyRing.NoDelayKey) + selfP2WKH, err := input.CommitScriptUnencumbered(keyRing.ToRemoteKey) if err != nil { return nil, fmt.Errorf("unable to create self commit "+ "script: %v", err) @@ -5015,7 +5015,7 @@ func newOutgoingHtlcResolution(signer input.Signer, // transaction. timeoutTx, err := createHtlcTimeoutTx( op, secondLevelOutputAmt, htlc.RefundTimeout, csvDelay, - keyRing.RevocationKey, keyRing.DelayKey, + keyRing.RevocationKey, keyRing.ToLocalKey, ) if err != nil { return nil, err @@ -5055,7 +5055,7 @@ func newOutgoingHtlcResolution(signer input.Signer, // transaction creates so we can generate the signDesc required to // complete the claim process after a delay period. htlcSweepScript, err := input.SecondLevelHtlcScript( - keyRing.RevocationKey, keyRing.DelayKey, csvDelay, + keyRing.RevocationKey, keyRing.ToLocalKey, csvDelay, ) if err != nil { return nil, err @@ -5149,7 +5149,7 @@ func newIncomingHtlcResolution(signer input.Signer, localChanCfg *channeldb.Chan secondLevelOutputAmt := htlc.Amt.ToSatoshis() - htlcFee successTx, err := createHtlcSuccessTx( op, secondLevelOutputAmt, csvDelay, - keyRing.RevocationKey, keyRing.DelayKey, + keyRing.RevocationKey, keyRing.ToLocalKey, ) if err != nil { return nil, err @@ -5192,7 +5192,7 @@ func newIncomingHtlcResolution(signer input.Signer, localChanCfg *channeldb.Chan // creates so we can generate the proper signDesc to sweep it after the // CSV delay has passed. htlcSweepScript, err := input.SecondLevelHtlcScript( - keyRing.RevocationKey, keyRing.DelayKey, csvDelay, + keyRing.RevocationKey, keyRing.ToLocalKey, csvDelay, ) if err != nil { return nil, err @@ -5410,7 +5410,7 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si commitPoint, true, chanState.ChanType.IsTweakless(), &chanState.LocalChanCfg, &chanState.RemoteChanCfg, ) - selfScript, err := input.CommitScriptToSelf(csvTimeout, keyRing.DelayKey, + selfScript, err := input.CommitScriptToSelf(csvTimeout, keyRing.ToLocalKey, keyRing.RevocationKey) if err != nil { return nil, err diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index bf95dd598..2be650298 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -59,18 +59,30 @@ type CommitmentKeyRing struct { // of whether this is our commit or not. RemoteHtlcKey *btcec.PublicKey - // DelayKey is the commitment transaction owner's key which is included - // in HTLC success and timeout transaction scripts. - DelayKey *btcec.PublicKey + // ToLocalKey is the commitment transaction owner's key which is + // included in HTLC success and timeout transaction scripts. This is + // the public key used for the to_local output of the commitment + // transaction. + // + // NOTE: Who's key this is depends on the current perspective. If this + // is our commitment this will be our key. + ToLocalKey *btcec.PublicKey - // NoDelayKey is the other party's payment key in the commitment tx. - // This is the key used to generate the unencumbered output within the + // ToRemoteKey is the non-owner's payment key in the commitment tx. + // This is the key used to generate the to_remote output within the // commitment transaction. - NoDelayKey *btcec.PublicKey + // + // NOTE: Who's key this is depends on the current perspective. If this + // is our commitment this will be their key. + ToRemoteKey *btcec.PublicKey // RevocationKey is the key that can be used by the other party to // redeem outputs from a revoked commitment transaction if it were to // be published. + // + // NOTE: Who can sign for this key depends on the current perspective. + // If this is our commitment, it means the remote node can sign for + // this key in case of a breach. RevocationKey *btcec.PublicKey } @@ -100,29 +112,29 @@ func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, ), } - // We'll now compute the delay, no delay, and revocation key based on - // the current commitment point. All keys are tweaked each state in + // We'll now compute the to_local, to_remote, and revocation key based + // on the current commitment point. All keys are tweaked each state in // order to ensure the keys from each state are unlinkable. To create // the revocation key, we take the opposite party's revocation base // point and combine that with the current commitment point. var ( - delayBasePoint *btcec.PublicKey - noDelayBasePoint *btcec.PublicKey + toLocalBasePoint *btcec.PublicKey + toRemoteBasePoint *btcec.PublicKey revocationBasePoint *btcec.PublicKey ) if isOurCommit { - delayBasePoint = localChanCfg.DelayBasePoint.PubKey - noDelayBasePoint = remoteChanCfg.PaymentBasePoint.PubKey + toLocalBasePoint = localChanCfg.DelayBasePoint.PubKey + toRemoteBasePoint = remoteChanCfg.PaymentBasePoint.PubKey revocationBasePoint = remoteChanCfg.RevocationBasePoint.PubKey } else { - delayBasePoint = remoteChanCfg.DelayBasePoint.PubKey - noDelayBasePoint = localChanCfg.PaymentBasePoint.PubKey + toLocalBasePoint = remoteChanCfg.DelayBasePoint.PubKey + toRemoteBasePoint = localChanCfg.PaymentBasePoint.PubKey revocationBasePoint = localChanCfg.RevocationBasePoint.PubKey } // With the base points assigned, we can now derive the actual keys // using the base point, and the current commitment tweak. - keyRing.DelayKey = input.TweakPubKey(delayBasePoint, commitPoint) + keyRing.ToLocalKey = input.TweakPubKey(toLocalBasePoint, commitPoint) keyRing.RevocationKey = input.DeriveRevocationPubkey( revocationBasePoint, commitPoint, ) @@ -130,10 +142,10 @@ func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, // If this commitment should omit the tweak for the remote point, then // we'll use that directly, and ignore the commitPoint tweak. if tweaklessCommit { - keyRing.NoDelayKey = noDelayBasePoint + keyRing.ToRemoteKey = toRemoteBasePoint } else { - keyRing.NoDelayKey = input.TweakPubKey( - noDelayBasePoint, commitPoint, + keyRing.ToRemoteKey = input.TweakPubKey( + toRemoteBasePoint, commitPoint, ) } @@ -377,7 +389,7 @@ func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, // the funds with the revocation key if we broadcast a revoked // commitment transaction. toLocalRedeemScript, err := input.CommitScriptToSelf( - uint32(localChanCfg.CsvDelay), keyRing.DelayKey, + uint32(localChanCfg.CsvDelay), keyRing.ToLocalKey, keyRing.RevocationKey, ) if err != nil { @@ -393,7 +405,7 @@ func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, // Next, we create the script paying to the remote. This is just a // regular P2WPKH output, without any added CSV delay. toRemoteWitnessKeyHash, err := input.CommitScriptUnencumbered( - keyRing.NoDelayKey, + keyRing.ToRemoteKey, ) if err != nil { return nil, err diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 2ef864197..20f250352 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -439,8 +439,8 @@ func TestCommitmentAndHTLCTransactions(t *testing.T) { LocalHtlcKeyTweak: tweak, LocalHtlcKey: tc.localPaymentPubKey, RemoteHtlcKey: tc.remotePaymentPubKey, - DelayKey: tc.localDelayPubKey, - NoDelayKey: tc.remotePaymentPubKey, + ToLocalKey: tc.localDelayPubKey, + ToRemoteKey: tc.remotePaymentPubKey, RevocationKey: tc.localRevocationPubKey, } @@ -1081,9 +1081,9 @@ func testSpendValidation(t *testing.T, tweakless bool) { // of 5 blocks before sweeping the output, while bob can spend // immediately with either the revocation key, or his regular key. keyRing := &CommitmentKeyRing{ - DelayKey: aliceDelayKey, + ToLocalKey: aliceDelayKey, RevocationKey: revokePubKey, - NoDelayKey: bobPayKey, + ToRemoteKey: bobPayKey, } commitmentTx, err := CreateCommitTx( *fakeFundingTxIn, keyRing, aliceChanCfg, bobChanCfg, diff --git a/watchtower/wtclient/backup_task.go b/watchtower/wtclient/backup_task.go index abdabe2f9..c112c1016 100644 --- a/watchtower/wtclient/backup_task.go +++ b/watchtower/wtclient/backup_task.go @@ -189,7 +189,7 @@ func (t *backupTask) craftSessionPayload( justiceKit := &blob.JusticeKit{ SweepAddress: t.sweepPkScript, RevocationPubKey: toBlobPubKey(keyRing.RevocationKey), - LocalDelayPubKey: toBlobPubKey(keyRing.DelayKey), + LocalDelayPubKey: toBlobPubKey(keyRing.ToLocalKey), CSVDelay: t.breachInfo.RemoteDelay, } @@ -199,7 +199,7 @@ func (t *backupTask) craftSessionPayload( // output to spend from. if t.toRemoteInput != nil { justiceKit.CommitToRemotePubKey = toBlobPubKey( - keyRing.NoDelayKey, + keyRing.ToRemoteKey, ) } diff --git a/watchtower/wtclient/backup_task_internal_test.go b/watchtower/wtclient/backup_task_internal_test.go index d845631ce..3c8ed17f7 100644 --- a/watchtower/wtclient/backup_task_internal_test.go +++ b/watchtower/wtclient/backup_task_internal_test.go @@ -121,8 +121,8 @@ func genTaskTest( BreachTransaction: breachTxn, KeyRing: &lnwallet.CommitmentKeyRing{ RevocationKey: revPK, - DelayKey: toLocalPK, - NoDelayKey: toRemotePK, + ToLocalKey: toLocalPK, + ToRemoteKey: toRemotePK, }, RemoteDelay: csvDelay, } @@ -565,9 +565,9 @@ func testBackupTask(t *testing.T, test backupTaskTest) { } keyRing := test.breachInfo.KeyRing - expToLocalPK := keyRing.DelayKey.SerializeCompressed() + expToLocalPK := keyRing.ToLocalKey.SerializeCompressed() expRevPK := keyRing.RevocationKey.SerializeCompressed() - expToRemotePK := keyRing.NoDelayKey.SerializeCompressed() + expToRemotePK := keyRing.ToRemoteKey.SerializeCompressed() // Assert that the blob contained the serialized revocation and to-local // pubkeys. diff --git a/watchtower/wtclient/client_test.go b/watchtower/wtclient/client_test.go index bd16ac065..ea681d575 100644 --- a/watchtower/wtclient/client_test.go +++ b/watchtower/wtclient/client_test.go @@ -291,8 +291,8 @@ func (c *mockChannel) createRemoteCommitTx(t *testing.T) { commitKeyRing := &lnwallet.CommitmentKeyRing{ RevocationKey: c.revPK, - NoDelayKey: c.toLocalPK, - DelayKey: c.toRemotePK, + ToRemoteKey: c.toLocalPK, + ToLocalKey: c.toRemotePK, } retribution := &lnwallet.BreachRetribution{ From 5e3718a1b522809937852c9f391931b8c4de0c4c Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:04 +0100 Subject: [PATCH 12/17] lnwallet: use channel type to derive keys We abstract away how keys are generated for the different channel types types (currently tweak(less)). Intention is that more of the logic that is unique for each commitment type lives in commitment.go, making the channel state machine oblivious to the keys and outputs being created on the commitment tx for a given channel state. --- breacharbiter_test.go | 2 +- contractcourt/chain_watcher.go | 13 +++++-------- htlcswitch/test_utils.go | 2 +- lnwallet/channel.go | 34 +++++++++++++++------------------- lnwallet/commitment.go | 11 +++++++---- lnwallet/test_utils.go | 16 ++++++++-------- lnwallet/wallet.go | 16 ++++++---------- test_utils.go | 2 +- 8 files changed, 44 insertions(+), 52 deletions(-) diff --git a/breacharbiter_test.go b/breacharbiter_test.go index 5a3cb50f8..72448ab54 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -1799,7 +1799,7 @@ func createInitChannels(revocationWindow int) (*lnwallet.LightningChannel, *lnwa aliceCommitTx, bobCommitTx, err := lnwallet.CreateCommitmentTxns( channelBal, channelBal, &aliceCfg, &bobCfg, aliceCommitPoint, - bobCommitPoint, *fundingTxIn, true, + bobCommitPoint, *fundingTxIn, channeldb.SingleFunderTweaklessBit, ) if err != nil { return nil, nil, nil, err diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index 7a226de25..c37677b7d 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -331,7 +331,8 @@ func (c *chainWatcher) SubscribeChannelEvents() *ChainEventSubscription { // based off of only the set of outputs included. func isOurCommitment(localChanCfg, remoteChanCfg channeldb.ChannelConfig, commitSpend *chainntnfs.SpendDetail, broadcastStateNum uint64, - revocationProducer shachain.Producer, tweakless bool) (bool, error) { + revocationProducer shachain.Producer, + chanType channeldb.ChannelType) (bool, error) { // First, we'll re-derive our commitment point for this state since // this is what we use to randomize each of the keys for this state. @@ -345,7 +346,7 @@ func isOurCommitment(localChanCfg, remoteChanCfg channeldb.ChannelConfig, // and remote keys for this state. We use our point as only we can // revoke our own commitment. commitKeyRing := lnwallet.DeriveCommitmentKeys( - commitPoint, true, tweakless, &localChanCfg, &remoteChanCfg, + commitPoint, true, chanType, &localChanCfg, &remoteChanCfg, ) // With the keys derived, we'll construct the remote script that'll be @@ -422,11 +423,6 @@ func (c *chainWatcher) closeObserver(spendNtfn *chainntnfs.SpendEvent) { // revoked state...!!! commitTxBroadcast := commitSpend.SpendingTx - // An additional piece of information we need to properly - // dispatch a close event if is this channel was using the - // tweakless remove key format or not. - tweaklessCommit := c.cfg.chanState.ChanType.IsTweakless() - localCommit, remoteCommit, err := c.cfg.chanState.LatestCommitments() if err != nil { log.Errorf("Unable to fetch channel state for "+ @@ -484,7 +480,7 @@ func (c *chainWatcher) closeObserver(spendNtfn *chainntnfs.SpendEvent) { c.cfg.chanState.LocalChanCfg, c.cfg.chanState.RemoteChanCfg, commitSpend, broadcastStateNum, c.cfg.chanState.RevocationProducer, - tweaklessCommit, + c.cfg.chanState.ChanType, ) if err != nil { log.Errorf("unable to determine self commit for "+ @@ -596,6 +592,7 @@ func (c *chainWatcher) closeObserver(spendNtfn *chainntnfs.SpendEvent) { // close and sweep immediately using a fake commitPoint // as it isn't actually needed for recovery anymore. commitPoint := c.cfg.chanState.RemoteCurrentRevocation + tweaklessCommit := c.cfg.chanState.ChanType.IsTweakless() if !tweaklessCommit { commitPoint = c.waitForCommitmentPoint() if commitPoint == nil { diff --git a/htlcswitch/test_utils.go b/htlcswitch/test_utils.go index 9ed8179a8..da6b07edd 100644 --- a/htlcswitch/test_utils.go +++ b/htlcswitch/test_utils.go @@ -262,7 +262,7 @@ func createTestChannel(alicePrivKey, bobPrivKey []byte, aliceCommitTx, bobCommitTx, err := lnwallet.CreateCommitmentTxns( aliceAmount, bobAmount, &aliceCfg, &bobCfg, aliceCommitPoint, - bobCommitPoint, *fundingTxIn, true, + bobCommitPoint, *fundingTxIn, channeldb.SingleFunderTweaklessBit, ) if err != nil { return nil, nil, nil, err diff --git a/lnwallet/channel.go b/lnwallet/channel.go index eb78cdd23..c0743ea0c 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -866,11 +866,6 @@ func (lc *LightningChannel) diskCommitToMemCommit(isLocal bool, diskCommit *channeldb.ChannelCommitment, localCommitPoint, remoteCommitPoint *btcec.PublicKey) (*commitment, error) { - // If this commit is tweakless, then it'll affect the way we derive our - // keys, which will affect the commitment transaction reconstruction. - // So we'll determine this first, before we do anything else. - tweaklessCommit := lc.channelState.ChanType.IsTweakless() - // First, we'll need to re-derive the commitment key ring for each // party used within this particular state. If this is a pending commit // (we extended but weren't able to complete the commitment dance @@ -879,14 +874,14 @@ func (lc *LightningChannel) diskCommitToMemCommit(isLocal bool, var localCommitKeys, remoteCommitKeys *CommitmentKeyRing if localCommitPoint != nil { localCommitKeys = DeriveCommitmentKeys( - localCommitPoint, true, tweaklessCommit, + localCommitPoint, true, lc.channelState.ChanType, &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) } if remoteCommitPoint != nil { remoteCommitKeys = DeriveCommitmentKeys( - remoteCommitPoint, false, tweaklessCommit, + remoteCommitPoint, false, lc.channelState.ChanType, &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) @@ -1581,9 +1576,8 @@ func (lc *LightningChannel) restoreCommitState( // We'll also re-create the set of commitment keys needed to // fully re-derive the state. - tweaklessCommit := lc.channelState.ChanType.IsTweakless() pendingRemoteKeyChain = DeriveCommitmentKeys( - pendingCommitPoint, false, tweaklessCommit, + pendingCommitPoint, false, lc.channelState.ChanType, &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) } @@ -1869,9 +1863,8 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // With the commitment point generated, we can now generate the four // keys we'll need to reconstruct the commitment state, - tweaklessCommit := chanState.ChanType.IsTweakless() keyRing := DeriveCommitmentKeys( - commitmentPoint, false, tweaklessCommit, + commitmentPoint, false, chanState.ChanType, &chanState.LocalChanCfg, &chanState.RemoteChanCfg, ) @@ -1939,6 +1932,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // If this is a tweakless commitment, then we can safely blank // out the SingleTweak value as it isn't needed. + tweaklessCommit := chanState.ChanType.IsTweakless() if tweaklessCommit { localSignDesc.SingleTweak = nil } @@ -2986,7 +2980,7 @@ func (lc *LightningChannel) SignNextCommitment() (lnwire.Sig, []lnwire.Sig, []ch // used within fetchCommitmentView to derive all the keys necessary to // construct the commitment state. keyRing := DeriveCommitmentKeys( - commitPoint, false, lc.channelState.ChanType.IsTweakless(), + commitPoint, false, lc.channelState.ChanType, &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) @@ -3744,7 +3738,7 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, } commitPoint := input.ComputeCommitmentPoint(commitSecret[:]) keyRing := DeriveCommitmentKeys( - commitPoint, true, lc.channelState.ChanType.IsTweakless(), + commitPoint, true, lc.channelState.ChanType, &lc.channelState.LocalChanCfg, &lc.channelState.RemoteChanCfg, ) @@ -4749,10 +4743,9 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // First, we'll generate the commitment point and the revocation point // so we can re-construct the HTLC state and also our payment key. - tweaklessCommit := chanState.ChanType.IsTweakless() keyRing := DeriveCommitmentKeys( - commitPoint, false, tweaklessCommit, &chanState.LocalChanCfg, - &chanState.RemoteChanCfg, + commitPoint, false, chanState.ChanType, + &chanState.LocalChanCfg, &chanState.RemoteChanCfg, ) // Next, we'll obtain HTLC resolutions for all the outgoing HTLC's we @@ -4817,6 +4810,7 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // If this is a tweakless commitment, then we can safely blank // out the SingleTweak value as it isn't needed. + tweaklessCommit := chanState.ChanType.IsTweakless() if tweaklessCommit { commitResolution.SelfOutputSignDesc.SingleTweak = nil } @@ -5407,11 +5401,13 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si } commitPoint := input.ComputeCommitmentPoint(revocation[:]) keyRing := DeriveCommitmentKeys( - commitPoint, true, chanState.ChanType.IsTweakless(), + commitPoint, true, chanState.ChanType, &chanState.LocalChanCfg, &chanState.RemoteChanCfg, ) - selfScript, err := input.CommitScriptToSelf(csvTimeout, keyRing.ToLocalKey, - keyRing.RevocationKey) + + selfScript, err := input.CommitScriptToSelf( + csvTimeout, keyRing.ToLocalKey, keyRing.RevocationKey, + ) if err != nil { return nil, err } diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 2be650298..99f577689 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -86,13 +86,16 @@ type CommitmentKeyRing struct { RevocationKey *btcec.PublicKey } -// DeriveCommitmentKey generates a new commitment key set using the base points -// and commitment point. The keys are derived differently depending whether the -// commitment transaction is ours or the remote peer's. +// DeriveCommitmentKeys generates a new commitment key set using the base points +// and commitment point. The keys are derived differently depending on the type +// of channel, and whether the commitment transaction is ours or the remote +// peer's. func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, - isOurCommit, tweaklessCommit bool, + isOurCommit bool, chanType channeldb.ChannelType, localChanCfg, remoteChanCfg *channeldb.ChannelConfig) *CommitmentKeyRing { + tweaklessCommit := chanType.IsTweakless() + // First, we'll derive all the keys that don't depend on the context of // whose commitment transaction this is. keyRing := &CommitmentKeyRing{ diff --git a/lnwallet/test_utils.go b/lnwallet/test_utils.go index f5df61f68..5253acdda 100644 --- a/lnwallet/test_utils.go +++ b/lnwallet/test_utils.go @@ -206,9 +206,14 @@ func CreateTestChannels(tweaklessCommits bool) ( } aliceCommitPoint := input.ComputeCommitmentPoint(aliceFirstRevoke[:]) + chanType := channeldb.SingleFunderTweaklessBit + if !tweaklessCommits { + chanType = channeldb.SingleFunderBit + } + aliceCommitTx, bobCommitTx, err := CreateCommitmentTxns( channelBal, channelBal, &aliceCfg, &bobCfg, aliceCommitPoint, - bobCommitPoint, *fundingTxIn, tweaklessCommits, + bobCommitPoint, *fundingTxIn, chanType, ) if err != nil { return nil, nil, nil, err @@ -275,7 +280,7 @@ func CreateTestChannels(tweaklessCommits bool) ( IdentityPub: aliceKeys[0].PubKey(), FundingOutpoint: *prevOut, ShortChannelID: shortChanID, - ChanType: channeldb.SingleFunderTweaklessBit, + ChanType: chanType, IsInitiator: true, Capacity: channelCapacity, RemoteCurrentRevocation: bobCommitPoint, @@ -293,7 +298,7 @@ func CreateTestChannels(tweaklessCommits bool) ( IdentityPub: bobKeys[0].PubKey(), FundingOutpoint: *prevOut, ShortChannelID: shortChanID, - ChanType: channeldb.SingleFunderTweaklessBit, + ChanType: chanType, IsInitiator: false, Capacity: channelCapacity, RemoteCurrentRevocation: aliceCommitPoint, @@ -305,11 +310,6 @@ func CreateTestChannels(tweaklessCommits bool) ( Packager: channeldb.NewChannelPackager(shortChanID), } - if !tweaklessCommits { - aliceChannelState.ChanType = channeldb.SingleFunderBit - bobChannelState.ChanType = channeldb.SingleFunderBit - } - aliceSigner := &input.MockSigner{Privkeys: aliceKeys} bobSigner := &input.MockSigner{Privkeys: bobKeys} diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index 61b7cfeea..a466a6d49 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -772,16 +772,14 @@ func (l *LightningWallet) handleFundingCancelRequest(req *fundingReserveCancelMs func CreateCommitmentTxns(localBalance, remoteBalance btcutil.Amount, ourChanCfg, theirChanCfg *channeldb.ChannelConfig, localCommitPoint, remoteCommitPoint *btcec.PublicKey, - fundingTxIn wire.TxIn, - tweaklessCommit bool) (*wire.MsgTx, *wire.MsgTx, error) { + fundingTxIn wire.TxIn, chanType channeldb.ChannelType) ( + *wire.MsgTx, *wire.MsgTx, error) { localCommitmentKeys := DeriveCommitmentKeys( - localCommitPoint, true, tweaklessCommit, ourChanCfg, - theirChanCfg, + localCommitPoint, true, chanType, ourChanCfg, theirChanCfg, ) remoteCommitmentKeys := DeriveCommitmentKeys( - remoteCommitPoint, false, tweaklessCommit, ourChanCfg, - theirChanCfg, + remoteCommitPoint, false, chanType, ourChanCfg, theirChanCfg, ) ourCommitTx, err := CreateCommitTx( @@ -930,13 +928,12 @@ func (l *LightningWallet) handleContributionMsg(req *addContributionMsg) { // With the funding tx complete, create both commitment transactions. localBalance := pendingReservation.partialState.LocalCommitment.LocalBalance.ToSatoshis() remoteBalance := pendingReservation.partialState.LocalCommitment.RemoteBalance.ToSatoshis() - tweaklessCommits := pendingReservation.partialState.ChanType.IsTweakless() ourCommitTx, theirCommitTx, err := CreateCommitmentTxns( localBalance, remoteBalance, ourContribution.ChannelConfig, theirContribution.ChannelConfig, ourContribution.FirstCommitmentPoint, theirContribution.FirstCommitmentPoint, fundingTxIn, - tweaklessCommits, + pendingReservation.partialState.ChanType, ) if err != nil { req.err <- err @@ -1290,14 +1287,13 @@ func (l *LightningWallet) handleSingleFunderSigs(req *addSingleFunderSigsMsg) { // remote node's commitment transactions. localBalance := pendingReservation.partialState.LocalCommitment.LocalBalance.ToSatoshis() remoteBalance := pendingReservation.partialState.LocalCommitment.RemoteBalance.ToSatoshis() - tweaklessCommits := pendingReservation.partialState.ChanType.IsTweakless() ourCommitTx, theirCommitTx, err := CreateCommitmentTxns( localBalance, remoteBalance, pendingReservation.ourContribution.ChannelConfig, pendingReservation.theirContribution.ChannelConfig, pendingReservation.ourContribution.FirstCommitmentPoint, pendingReservation.theirContribution.FirstCommitmentPoint, - *fundingTxIn, tweaklessCommits, + *fundingTxIn, pendingReservation.partialState.ChanType, ) if err != nil { req.err <- err diff --git a/test_utils.go b/test_utils.go index afb16cd54..757d33e4a 100644 --- a/test_utils.go +++ b/test_utils.go @@ -195,7 +195,7 @@ func createTestPeer(notifier chainntnfs.ChainNotifier, publTx chan *wire.MsgTx, aliceCommitTx, bobCommitTx, err := lnwallet.CreateCommitmentTxns( channelBal, channelBal, &aliceCfg, &bobCfg, aliceCommitPoint, - bobCommitPoint, *fundingTxIn, true, + bobCommitPoint, *fundingTxIn, channeldb.SingleFunderTweaklessBit, ) if err != nil { return nil, nil, nil, nil, err From f92c7a3af05d7b704b2ccb2d56679597aa9b0c76 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:04 +0100 Subject: [PATCH 13/17] lnwallet: nil single tweak when creating keyring To make the channel state machine less concerned about the type of commitment, we nil the local tweak when creating the keyring, depending on the commitment type. --- lnwallet/channel.go | 14 -------------- lnwallet/commitment.go | 7 +++++++ 2 files changed, 7 insertions(+), 14 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index c0743ea0c..a8bde14ef 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1929,13 +1929,6 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, }, HashType: txscript.SigHashAll, } - - // If this is a tweakless commitment, then we can safely blank - // out the SingleTweak value as it isn't needed. - tweaklessCommit := chanState.ChanType.IsTweakless() - if tweaklessCommit { - localSignDesc.SingleTweak = nil - } } // Similarly, if the remote balance exceeds the remote party's dust @@ -4807,13 +4800,6 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si }, MaturityDelay: 0, } - - // If this is a tweakless commitment, then we can safely blank - // out the SingleTweak value as it isn't needed. - tweaklessCommit := chanState.ChanType.IsTweakless() - if tweaklessCommit { - commitResolution.SelfOutputSignDesc.SingleTweak = nil - } } closeSummary := channeldb.ChannelCloseSummary{ diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 99f577689..59f5f197b 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -146,6 +146,13 @@ func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, // we'll use that directly, and ignore the commitPoint tweak. if tweaklessCommit { keyRing.ToRemoteKey = toRemoteBasePoint + + // If this is not our commitment, the above ToRemoteKey will be + // ours, and we blank out the local commitment tweak to + // indicate that the key should not be tweaked when signing. + if !isOurCommit { + keyRing.LocalCommitKeyTweak = nil + } } else { keyRing.ToRemoteKey = input.TweakPubKey( toRemoteBasePoint, commitPoint, From 9b5809a884bdad32e0fe9c7c1629d727d594f08a Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:04 +0100 Subject: [PATCH 14/17] input: update SignDescriptor doc to note only segwit is supported Also update the WitnessScript doc to note it should be set also for p2wkh. --- input/signdescriptor.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/input/signdescriptor.go b/input/signdescriptor.go index 0dfb5d464..2aa2af9c2 100644 --- a/input/signdescriptor.go +++ b/input/signdescriptor.go @@ -17,9 +17,9 @@ var ( ErrTweakOverdose = errors.New("sign descriptor should only have one tweak") ) -// SignDescriptor houses the necessary information required to successfully sign -// a given output. This struct is used by the Signer interface in order to gain -// access to critical data needed to generate a valid signature. +// SignDescriptor houses the necessary information required to successfully +// sign a given segwit output. This struct is used by the Signer interface in +// order to gain access to critical data needed to generate a valid signature. type SignDescriptor struct { // KeyDesc is a descriptor that precisely describes *which* key to use // for signing. This may provide the raw public key directly, or @@ -56,8 +56,9 @@ type SignDescriptor struct { DoubleTweak *btcec.PrivateKey // WitnessScript is the full script required to properly redeem the - // output. This field will only be populated if a p2wsh or a p2sh - // output is being signed. + // output. This field should be set to the full script if a p2wsh + // output is being signed. For p2wkh it should be set to the hashed + // script (PkScript). WitnessScript []byte // Output is the target output which should be signed. The PkScript and From a56ed72bd7d136f8eb85c01aabc9fc8cab544f49 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:04 +0100 Subject: [PATCH 15/17] lnwallet: use channel type to derive remote script Based on the current channel type, we derive the script used for the to_remote output. Currently only the unencumbered p2wkh type is used, but that will change with upcoming channel types. --- contractcourt/chain_watcher.go | 7 +-- lnwallet/channel.go | 91 +++++++++++++++++++--------------- lnwallet/commitment.go | 55 +++++++++++++++----- lnwallet/transactions_test.go | 6 ++- lnwallet/wallet.go | 8 +-- 5 files changed, 105 insertions(+), 62 deletions(-) diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index c37677b7d..e9eb8e396 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -351,8 +351,9 @@ func isOurCommitment(localChanCfg, remoteChanCfg channeldb.ChannelConfig, // With the keys derived, we'll construct the remote script that'll be // present if they have a non-dust balance on the commitment. - remotePkScript, err := input.CommitScriptUnencumbered( - commitKeyRing.ToRemoteKey, + remoteDelay := uint32(remoteChanCfg.CsvDelay) + remoteScript, err := lnwallet.CommitScriptToRemote( + chanType, remoteDelay, commitKeyRing.ToRemoteKey, ) if err != nil { return false, err @@ -383,7 +384,7 @@ func isOurCommitment(localChanCfg, remoteChanCfg channeldb.ChannelConfig, case bytes.Equal(localPkScript, pkScript): return true, nil - case bytes.Equal(remotePkScript, pkScript): + case bytes.Equal(remoteScript.PkScript, pkScript): return true, nil } } diff --git a/lnwallet/channel.go b/lnwallet/channel.go index a8bde14ef..210328e49 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1871,36 +1871,42 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // Next, reconstruct the scripts as they were present at this state // number so we can have the proper witness script to sign and include // within the final witness. - remoteDelay := uint32(chanState.RemoteChanCfg.CsvDelay) - remotePkScript, err := input.CommitScriptToSelf( - remoteDelay, keyRing.ToLocalKey, keyRing.RevocationKey, + theirDelay := uint32(chanState.RemoteChanCfg.CsvDelay) + theirPkScript, err := input.CommitScriptToSelf( + theirDelay, keyRing.ToLocalKey, keyRing.RevocationKey, ) if err != nil { return nil, err } - remoteWitnessHash, err := input.WitnessScriptHash(remotePkScript) + theirWitnessHash, err := input.WitnessScriptHash(theirPkScript) if err != nil { return nil, err } - localPkScript, err := input.CommitScriptUnencumbered(keyRing.ToRemoteKey) + + // Since it is the remote breach we are reconstructing, the output going + // to us will be a to-remote script with our local params. + ourDelay := uint32(chanState.LocalChanCfg.CsvDelay) + ourScript, err := CommitScriptToRemote( + chanState.ChanType, ourDelay, keyRing.ToRemoteKey, + ) if err != nil { return nil, err } // In order to fully populate the breach retribution struct, we'll need - // to find the exact index of the local+remote commitment outputs. - localOutpoint := wire.OutPoint{ + // to find the exact index of the commitment outputs. + ourOutpoint := wire.OutPoint{ Hash: commitHash, } - remoteOutpoint := wire.OutPoint{ + theirOutpoint := wire.OutPoint{ Hash: commitHash, } for i, txOut := range revokedSnapshot.CommitTx.TxOut { switch { - case bytes.Equal(txOut.PkScript, localPkScript): - localOutpoint.Index = uint32(i) - case bytes.Equal(txOut.PkScript, remoteWitnessHash): - remoteOutpoint.Index = uint32(i) + case bytes.Equal(txOut.PkScript, ourScript.PkScript): + ourOutpoint.Index = uint32(i) + case bytes.Equal(txOut.PkScript, theirWitnessHash): + theirOutpoint.Index = uint32(i) } } @@ -1908,39 +1914,39 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // commitment outputs. If either is considered dust using the remote // party's dust limit, the respective sign descriptor will be nil. var ( - localSignDesc *input.SignDescriptor - remoteSignDesc *input.SignDescriptor + ourSignDesc *input.SignDescriptor + theirSignDesc *input.SignDescriptor ) - // Compute the local and remote balances in satoshis. - localAmt := revokedSnapshot.LocalBalance.ToSatoshis() - remoteAmt := revokedSnapshot.RemoteBalance.ToSatoshis() + // Compute the balances in satoshis. + ourAmt := revokedSnapshot.LocalBalance.ToSatoshis() + theirAmt := revokedSnapshot.RemoteBalance.ToSatoshis() - // If the local balance exceeds the remote party's dust limit, - // instantiate the local sign descriptor. - if localAmt >= chanState.RemoteChanCfg.DustLimit { - localSignDesc = &input.SignDescriptor{ + // If our balance exceeds the remote party's dust limit, instantiate + // the sign descriptor for our output. + if ourAmt >= chanState.RemoteChanCfg.DustLimit { + ourSignDesc = &input.SignDescriptor{ SingleTweak: keyRing.LocalCommitKeyTweak, KeyDesc: chanState.LocalChanCfg.PaymentBasePoint, - WitnessScript: localPkScript, + WitnessScript: ourScript.WitnessScript, Output: &wire.TxOut{ - PkScript: localPkScript, - Value: int64(localAmt), + PkScript: ourScript.PkScript, + Value: int64(ourAmt), }, HashType: txscript.SigHashAll, } } - // Similarly, if the remote balance exceeds the remote party's dust - // limit, assemble the remote sign descriptor. - if remoteAmt >= chanState.RemoteChanCfg.DustLimit { - remoteSignDesc = &input.SignDescriptor{ + // Similarly, if their balance exceeds the remote party's dust limit, + // assemble the sign descriptor for their output, which we can sweep. + if theirAmt >= chanState.RemoteChanCfg.DustLimit { + theirSignDesc = &input.SignDescriptor{ KeyDesc: chanState.LocalChanCfg.RevocationBasePoint, DoubleTweak: commitmentSecret, - WitnessScript: remotePkScript, + WitnessScript: theirPkScript, Output: &wire.TxOut{ - PkScript: remoteWitnessHash, - Value: int64(remoteAmt), + PkScript: theirWitnessHash, + Value: int64(theirAmt), }, HashType: txscript.SigHashAll, } @@ -1971,7 +1977,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // remote commitment transaction, and *they* go to the second // level. secondLevelWitnessScript, err := input.SecondLevelHtlcScript( - keyRing.RevocationKey, keyRing.ToLocalKey, remoteDelay, + keyRing.RevocationKey, keyRing.ToLocalKey, theirDelay, ) if err != nil { return nil, err @@ -2037,13 +2043,13 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, BreachHeight: breachHeight, RevokedStateNum: stateNum, PendingHTLCs: revokedSnapshot.Htlcs, - LocalOutpoint: localOutpoint, - LocalOutputSignDesc: localSignDesc, - RemoteOutpoint: remoteOutpoint, - RemoteOutputSignDesc: remoteSignDesc, + LocalOutpoint: ourOutpoint, + LocalOutputSignDesc: ourSignDesc, + RemoteOutpoint: theirOutpoint, + RemoteOutputSignDesc: theirSignDesc, HtlcRetributions: htlcRetributions, KeyRing: keyRing, - RemoteDelay: remoteDelay, + RemoteDelay: theirDelay, }, nil } @@ -4758,7 +4764,10 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // Before we can generate the proper sign descriptor, we'll need to // locate the output index of our non-delayed output on the commitment // transaction. - selfP2WKH, err := input.CommitScriptUnencumbered(keyRing.ToRemoteKey) + localDelay := uint32(chanState.LocalChanCfg.CsvDelay) + selfScript, err := CommitScriptToRemote( + chanState.ChanType, localDelay, keyRing.ToRemoteKey, + ) if err != nil { return nil, fmt.Errorf("unable to create self commit "+ "script: %v", err) @@ -4770,7 +4779,7 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si ) for outputIndex, txOut := range commitTxBroadcast.TxOut { - if bytes.Equal(txOut.PkScript, selfP2WKH) { + if bytes.Equal(txOut.PkScript, selfScript.PkScript) { selfPoint = &wire.OutPoint{ Hash: *commitSpend.SpenderTxHash, Index: uint32(outputIndex), @@ -4791,10 +4800,10 @@ func NewUnilateralCloseSummary(chanState *channeldb.OpenChannel, signer input.Si SelfOutputSignDesc: input.SignDescriptor{ KeyDesc: localPayBase, SingleTweak: keyRing.LocalCommitKeyTweak, - WitnessScript: selfP2WKH, + WitnessScript: selfScript.WitnessScript, Output: &wire.TxOut{ Value: localBalance, - PkScript: selfP2WKH, + PkScript: selfScript.PkScript, }, HashType: txscript.SigHashAll, }, diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 59f5f197b..b00e60435 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -162,6 +162,37 @@ func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, return keyRing } +// ScriptInfo holds a redeem script and hash. +type ScriptInfo struct { + // PkScript is the output's PkScript. + PkScript []byte + + // WitnessScript is the full script required to properly redeem the + // output. This field should be set to the full script if a p2wsh + // output is being signed. For p2wkh it should be set equal to the + // PkScript. + WitnessScript []byte +} + +// CommitScriptToRemote creates the script that will pay to the non-owner of +// the commitment transaction, adding a delay to the script based on the +// channel type. +func CommitScriptToRemote(_ channeldb.ChannelType, csvTimeout uint32, + key *btcec.PublicKey) (*ScriptInfo, error) { + + p2wkh, err := input.CommitScriptUnencumbered(key) + if err != nil { + return nil, err + } + + // Since this is a regular P2WKH, the WitnessScipt and PkScript should + // both be set to the script hash. + return &ScriptInfo{ + WitnessScript: p2wkh, + PkScript: p2wkh, + }, nil +} + // CommitmentBuilder is a type that wraps the type of channel we are dealing // with, and abstracts the various ways of constructing commitment // transactions. @@ -292,15 +323,15 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, // out HTLCs. if isOurs { commitTx, err = CreateCommitTx( - fundingTxIn(cb.chanState), keyRing, &cb.chanState.LocalChanCfg, - &cb.chanState.RemoteChanCfg, ourBalance.ToSatoshis(), - theirBalance.ToSatoshis(), + cb.chanState.ChanType, fundingTxIn(cb.chanState), keyRing, + &cb.chanState.LocalChanCfg, &cb.chanState.RemoteChanCfg, + ourBalance.ToSatoshis(), theirBalance.ToSatoshis(), ) } else { commitTx, err = CreateCommitTx( - fundingTxIn(cb.chanState), keyRing, &cb.chanState.RemoteChanCfg, - &cb.chanState.LocalChanCfg, theirBalance.ToSatoshis(), - ourBalance.ToSatoshis(), + cb.chanState.ChanType, fundingTxIn(cb.chanState), keyRing, + &cb.chanState.RemoteChanCfg, &cb.chanState.LocalChanCfg, + theirBalance.ToSatoshis(), ourBalance.ToSatoshis(), ) } if err != nil { @@ -389,7 +420,8 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, // spent after a relative block delay or revocation event, and a remote output // paying the counterparty within the channel, which can be spent immediately // or after a delay depending on the commitment type.. -func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, +func CreateCommitTx(chanType channeldb.ChannelType, + fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, localChanCfg, remoteChanCfg *channeldb.ChannelConfig, amountToLocal, amountToRemote btcutil.Amount) (*wire.MsgTx, error) { @@ -412,10 +444,9 @@ func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, return nil, err } - // Next, we create the script paying to the remote. This is just a - // regular P2WPKH output, without any added CSV delay. - toRemoteWitnessKeyHash, err := input.CommitScriptUnencumbered( - keyRing.ToRemoteKey, + // Next, we create the script paying to the remote. + toRemoteScript, err := CommitScriptToRemote( + chanType, uint32(remoteChanCfg.CsvDelay), keyRing.ToRemoteKey, ) if err != nil { return nil, err @@ -436,7 +467,7 @@ func CreateCommitTx(fundingOutput wire.TxIn, keyRing *CommitmentKeyRing, } if amountToRemote >= localChanCfg.DustLimit { commitTx.AddTxOut(&wire.TxOut{ - PkScript: toRemoteWitnessKeyHash, + PkScript: toRemoteScript.PkScript, Value: int64(amountToRemote), }) } diff --git a/lnwallet/transactions_test.go b/lnwallet/transactions_test.go index 20f250352..4f5cc65a4 100644 --- a/lnwallet/transactions_test.go +++ b/lnwallet/transactions_test.go @@ -1048,8 +1048,10 @@ func testSpendValidation(t *testing.T, tweakless bool) { // our commitments, if it's tweakless, his key will just be his regular // pubkey. bobPayKey := input.TweakPubKey(bobKeyPub, commitPoint) + channelType := channeldb.SingleFunderBit if tweakless { bobPayKey = bobKeyPub + channelType = channeldb.SingleFunderTweaklessBit } aliceCommitTweak := input.SingleTweakBytes(commitPoint, aliceKeyPub) @@ -1086,8 +1088,8 @@ func testSpendValidation(t *testing.T, tweakless bool) { ToRemoteKey: bobPayKey, } commitmentTx, err := CreateCommitTx( - *fakeFundingTxIn, keyRing, aliceChanCfg, bobChanCfg, - channelBalance, channelBalance, + channelType, *fakeFundingTxIn, keyRing, aliceChanCfg, + bobChanCfg, channelBalance, channelBalance, ) if err != nil { t.Fatalf("unable to create commitment transaction: %v", nil) diff --git a/lnwallet/wallet.go b/lnwallet/wallet.go index a466a6d49..5143b9f5c 100644 --- a/lnwallet/wallet.go +++ b/lnwallet/wallet.go @@ -783,8 +783,8 @@ func CreateCommitmentTxns(localBalance, remoteBalance btcutil.Amount, ) ourCommitTx, err := CreateCommitTx( - fundingTxIn, localCommitmentKeys, ourChanCfg, theirChanCfg, - localBalance, remoteBalance, + chanType, fundingTxIn, localCommitmentKeys, ourChanCfg, + theirChanCfg, localBalance, remoteBalance, ) if err != nil { return nil, nil, err @@ -796,8 +796,8 @@ func CreateCommitmentTxns(localBalance, remoteBalance btcutil.Amount, } theirCommitTx, err := CreateCommitTx( - fundingTxIn, remoteCommitmentKeys, theirChanCfg, ourChanCfg, - remoteBalance, localBalance, + chanType, fundingTxIn, remoteCommitmentKeys, theirChanCfg, + ourChanCfg, remoteBalance, localBalance, ) if err != nil { return nil, nil, err From 3711597fefeb6d49a6aeacaf43726db5ee88a755 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:04 +0100 Subject: [PATCH 16/17] input: remove duplicate commit weight constant --- htlcswitch/link_test.go | 10 +++++----- input/size.go | 12 +++--------- lnwallet/channel.go | 4 ++-- lnwallet/channel_test.go | 2 +- lnwallet/commitment.go | 2 +- 5 files changed, 12 insertions(+), 18 deletions(-) diff --git a/htlcswitch/link_test.go b/htlcswitch/link_test.go index dc796ba31..9efe89dca 100644 --- a/htlcswitch/link_test.go +++ b/htlcswitch/link_test.go @@ -1973,7 +1973,7 @@ func TestChannelLinkBandwidthConsistency(t *testing.T) { t.Fatalf("unable to query fee estimator: %v", err) } htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HtlcWeight), + feePerKw.FeeForWeight(input.HTLCWeight), ) // The starting bandwidth of the channel should be exactly the amount @@ -2462,7 +2462,7 @@ func TestChannelLinkBandwidthConsistencyOverflow(t *testing.T) { // TODO(roasbeef): increase sleep time.Sleep(time.Second * 1) - commitWeight := input.CommitWeight + input.HtlcWeight*numHTLCs + commitWeight := int64(input.CommitWeight + input.HTLCWeight*numHTLCs) htlcFee := lnwire.NewMSatFromSatoshis( feePerKw.FeeForWeight(commitWeight), ) @@ -2646,7 +2646,7 @@ func TestChannelLinkTrimCircuitsPending(t *testing.T) { defaultCommitFee := alice.channel.StateSnapshot().CommitFee htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HtlcWeight), + feePerKw.FeeForWeight(input.HTLCWeight), ) // The starting bandwidth of the channel should be exactly the amount @@ -2925,7 +2925,7 @@ func TestChannelLinkTrimCircuitsNoCommit(t *testing.T) { defaultCommitFee := alice.channel.StateSnapshot().CommitFee htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HtlcWeight), + feePerKw.FeeForWeight(input.HTLCWeight), ) // The starting bandwidth of the channel should be exactly the amount @@ -3181,7 +3181,7 @@ func TestChannelLinkBandwidthChanReserve(t *testing.T) { t.Fatalf("unable to query fee estimator: %v", err) } htlcFee := lnwire.NewMSatFromSatoshis( - feePerKw.FeeForWeight(input.HtlcWeight), + feePerKw.FeeForWeight(input.HTLCWeight), ) // The starting bandwidth of the channel should be exactly the amount diff --git a/input/size.go b/input/size.go index 07055dc84..68fa85e4e 100644 --- a/input/size.go +++ b/input/size.go @@ -5,15 +5,6 @@ import ( "github.com/btcsuite/btcd/wire" ) -const ( - // CommitWeight is the weight of the base commitment transaction which - // includes: one p2wsh input, out p2wkh output, and one p2wsh output. - CommitWeight int64 = 724 - - // HtlcWeight is the weight of an HTLC output. - HtlcWeight int64 = 172 -) - const ( // witnessScaleFactor determines the level of "discount" witness data // receives compared to "base" data. A scale factor of 4, denotes that @@ -168,6 +159,9 @@ const ( // WitnessCommitmentTxWeight 224 weight WitnessCommitmentTxWeight = WitnessHeaderSize + WitnessSize + // CommitWeight 724 weight + CommitWeight = BaseCommitmentTxWeight + WitnessCommitmentTxWeight + // HTLCWeight 172 weight HTLCWeight = witnessScaleFactor * HTLCSize diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 210328e49..ff2228b43 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -3450,7 +3450,7 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, continue } - totalHtlcWeight += input.HtlcWeight + totalHtlcWeight += input.HTLCWeight } for _, htlc := range filteredHTLCView.theirUpdates { if htlcIsDust(!remoteChain, !remoteChain, feePerKw, @@ -3458,7 +3458,7 @@ func (lc *LightningChannel) computeView(view *htlcView, remoteChain bool, continue } - totalHtlcWeight += input.HtlcWeight + totalHtlcWeight += input.HTLCWeight } totalCommitWeight := input.CommitWeight + totalHtlcWeight diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index c0467af9f..214613117 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -600,7 +600,7 @@ func TestForceClose(t *testing.T) { // Factoring in the fee rate, Alice's amount should properly reflect // that we've added two additional HTLC to the commitment transaction. - totalCommitWeight := input.CommitWeight + (input.HtlcWeight * 2) + totalCommitWeight := int64(input.CommitWeight + (input.HTLCWeight * 2)) feePerKw := chainfee.SatPerKWeight( aliceChannel.channelState.LocalCommitment.FeePerKw, ) diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index b00e60435..67b5cbc1d 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -287,7 +287,7 @@ func (cb *CommitmentBuilder) createUnsignedCommitmentTx(ourBalance, // on its total weight. Once we have the total weight, we'll multiply // by the current fee-per-kw, then divide by 1000 to get the proper // fee. - totalCommitWeight := input.CommitWeight + (input.HtlcWeight * numHTLCs) + totalCommitWeight := input.CommitWeight + (input.HTLCWeight * numHTLCs) // With the weight known, we can now calculate the commitment fee, // ensuring that we account for any dust outputs trimmed above. From 9c3218c51e8cc27bfcccd47018caaf72b1be4464 Mon Sep 17 00:00:00 2001 From: "Johan T. Halseth" Date: Mon, 6 Jan 2020 11:42:05 +0100 Subject: [PATCH 17/17] lnwallet/channel: reuse derived SingleTweak on local force close When creating the keyring, the tweak is already calculated in the remote commitment case. We add the calculation also for our own commitment, so we can use it in all cases without deriving the tweak. --- lnwallet/channel.go | 5 +---- lnwallet/commitment.go | 9 ++++++++- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index ff2228b43..fbb58d0cf 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -5436,9 +5436,6 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si // nil. var commitResolution *CommitOutputResolution if len(delayScript) != 0 { - singleTweak := input.SingleTweakBytes( - commitPoint, chanState.LocalChanCfg.DelayBasePoint.PubKey, - ) localBalance := localCommit.LocalBalance commitResolution = &CommitOutputResolution{ SelfOutPoint: wire.OutPoint{ @@ -5447,7 +5444,7 @@ func NewLocalForceCloseSummary(chanState *channeldb.OpenChannel, signer input.Si }, SelfOutputSignDesc: input.SignDescriptor{ KeyDesc: chanState.LocalChanCfg.DelayBasePoint, - SingleTweak: singleTweak, + SingleTweak: keyRing.LocalCommitKeyTweak, WitnessScript: selfScript, Output: &wire.TxOut{ PkScript: delayScript, diff --git a/lnwallet/commitment.go b/lnwallet/commitment.go index 67b5cbc1d..459dd83bd 100644 --- a/lnwallet/commitment.go +++ b/lnwallet/commitment.go @@ -96,13 +96,20 @@ func DeriveCommitmentKeys(commitPoint *btcec.PublicKey, tweaklessCommit := chanType.IsTweakless() + // Depending on if this is our commit or not, we'll choose the correct + // base point. + localBasePoint := localChanCfg.PaymentBasePoint + if isOurCommit { + localBasePoint = localChanCfg.DelayBasePoint + } + // First, we'll derive all the keys that don't depend on the context of // whose commitment transaction this is. keyRing := &CommitmentKeyRing{ CommitPoint: commitPoint, LocalCommitKeyTweak: input.SingleTweakBytes( - commitPoint, localChanCfg.PaymentBasePoint.PubKey, + commitPoint, localBasePoint.PubKey, ), LocalHtlcKeyTweak: input.SingleTweakBytes( commitPoint, localChanCfg.HtlcBasePoint.PubKey,