From eb2f5cecf6c3f271d14036d76b188ce91958c900 Mon Sep 17 00:00:00 2001 From: Conner Fromknecht Date: Wed, 19 Dec 2018 21:50:46 -0800 Subject: [PATCH] multi: remove breach tx arg from NewBreachRetribution args This commit removes the breach transaction from the arguments passed to NewBreachRetribution. We already keep all prior remote commitments on disk in the commitment log, and load that transaction from disk inside the method. In practice, the one loaded from disk will be the same one that is passed in by the caller, so there should be no change in behavior as we've already derived the appropriate state number. This changes makes integration with the watchtower client simpler, since we no longer need to acquire the breaching commitment transaction to be able to construct the BreachRetribution. This simplifies not only the logic surrounding transient backsups, but also on startup (and later, retroactively backing up historic updates). --- breacharbiter_test.go | 2 +- contractcourt/chain_watcher.go | 8 ++------ lnwallet/channel.go | 9 ++++----- lnwallet/channel_test.go | 3 +-- 4 files changed, 8 insertions(+), 14 deletions(-) diff --git a/breacharbiter_test.go b/breacharbiter_test.go index d0d2cc3d..1f407289 100644 --- a/breacharbiter_test.go +++ b/breacharbiter_test.go @@ -1181,7 +1181,7 @@ func TestBreachSecondLevelTransfer(t *testing.T) { // Notify the breach arbiter about the breach. retribution, err := lnwallet.NewBreachRetribution( - alice.State(), height, forceCloseTx, 1) + alice.State(), height, 1) if err != nil { t.Fatalf("unable to create breach retribution: %v", err) } diff --git a/contractcourt/chain_watcher.go b/contractcourt/chain_watcher.go index 1bc1ee18..745c7b5d 100644 --- a/contractcourt/chain_watcher.go +++ b/contractcourt/chain_watcher.go @@ -725,18 +725,14 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail return fmt.Errorf("unable to mark channel as borked: %v", err) } - var ( - commitTxBroadcast = spendEvent.SpendingTx - spendHeight = uint32(spendEvent.SpendingHeight) - ) + spendHeight := uint32(spendEvent.SpendingHeight) // Create a new reach retribution struct which contains all the data // needed to swiftly bring the cheating peer to justice. // // TODO(roasbeef): move to same package retribution, err := lnwallet.NewBreachRetribution( - c.cfg.chanState, broadcastStateNum, commitTxBroadcast, - spendHeight, + c.cfg.chanState, broadcastStateNum, spendHeight, ) if err != nil { return fmt.Errorf("unable to create breach retribution: %v", err) diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 6175b36e..94f34938 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1897,11 +1897,8 @@ type BreachRetribution struct { // passed channel, at a particular revoked state number, and one which targets // the passed commitment transaction. func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, - broadcastCommitment *wire.MsgTx, breachHeight uint32) (*BreachRetribution, error) { - commitHash := broadcastCommitment.TxHash() - // Query the on-disk revocation log for the snapshot which was recorded // at this particular state num. revokedSnapshot, err := chanState.FindPreviousState(stateNum) @@ -1909,6 +1906,8 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, return nil, err } + commitHash := revokedSnapshot.CommitTx.TxHash() + // With the state number broadcast known, we can now derive/restore the // proper revocation preimage necessary to sweep the remote party's // output. @@ -1952,7 +1951,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, remoteOutpoint := wire.OutPoint{ Hash: commitHash, } - for i, txOut := range broadcastCommitment.TxOut { + for i, txOut := range revokedSnapshot.CommitTx.TxOut { switch { case bytes.Equal(txOut.PkScript, localPkScript): localOutpoint.Index = uint32(i) @@ -2090,7 +2089,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // swiftly bring justice to the cheating remote party. return &BreachRetribution{ ChainHash: chanState.ChainHash, - BreachTransaction: broadcastCommitment, + BreachTransaction: revokedSnapshot.CommitTx, BreachHeight: breachHeight, RevokedStateNum: stateNum, PendingHTLCs: revokedSnapshot.Htlcs, diff --git a/lnwallet/channel_test.go b/lnwallet/channel_test.go index 5eba3bc4..1f622652 100644 --- a/lnwallet/channel_test.go +++ b/lnwallet/channel_test.go @@ -5389,7 +5389,6 @@ func TestNewBreachRetributionSkipsDustHtlcs(t *testing.T) { // At this point, we'll capture the current state number, as well as // the current commitment. - revokedCommit := bobChannel.channelState.LocalCommitment.CommitTx revokedStateNum := aliceChannel.channelState.LocalCommitment.CommitHeight // We'll now have Bob settle those HTLC's to Alice and then advance @@ -5411,7 +5410,7 @@ func TestNewBreachRetributionSkipsDustHtlcs(t *testing.T) { // At this point, we'll now simulate a contract breach by Bob using the // NewBreachRetribution method. breachRet, err := NewBreachRetribution( - aliceChannel.channelState, revokedStateNum, revokedCommit, 100, + aliceChannel.channelState, revokedStateNum, 100, ) if err != nil { t.Fatalf("unable to create breach retribution: %v", err)