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).
This commit is contained in:
Conner Fromknecht 2018-12-19 21:50:46 -08:00
parent 9f7d2f6cbc
commit eb2f5cecf6
No known key found for this signature in database
GPG Key ID: E7D737B67FA592C7
4 changed files with 8 additions and 14 deletions

@ -1181,7 +1181,7 @@ func TestBreachSecondLevelTransfer(t *testing.T) {
// Notify the breach arbiter about the breach. // Notify the breach arbiter about the breach.
retribution, err := lnwallet.NewBreachRetribution( retribution, err := lnwallet.NewBreachRetribution(
alice.State(), height, forceCloseTx, 1) alice.State(), height, 1)
if err != nil { if err != nil {
t.Fatalf("unable to create breach retribution: %v", err) t.Fatalf("unable to create breach retribution: %v", err)
} }

@ -725,18 +725,14 @@ func (c *chainWatcher) dispatchContractBreach(spendEvent *chainntnfs.SpendDetail
return fmt.Errorf("unable to mark channel as borked: %v", err) return fmt.Errorf("unable to mark channel as borked: %v", err)
} }
var ( spendHeight := uint32(spendEvent.SpendingHeight)
commitTxBroadcast = spendEvent.SpendingTx
spendHeight = uint32(spendEvent.SpendingHeight)
)
// Create a new reach retribution struct which contains all the data // Create a new reach retribution struct which contains all the data
// needed to swiftly bring the cheating peer to justice. // needed to swiftly bring the cheating peer to justice.
// //
// TODO(roasbeef): move to same package // TODO(roasbeef): move to same package
retribution, err := lnwallet.NewBreachRetribution( retribution, err := lnwallet.NewBreachRetribution(
c.cfg.chanState, broadcastStateNum, commitTxBroadcast, c.cfg.chanState, broadcastStateNum, spendHeight,
spendHeight,
) )
if err != nil { if err != nil {
return fmt.Errorf("unable to create breach retribution: %v", err) return fmt.Errorf("unable to create breach retribution: %v", err)

@ -1897,11 +1897,8 @@ type BreachRetribution struct {
// passed channel, at a particular revoked state number, and one which targets // passed channel, at a particular revoked state number, and one which targets
// the passed commitment transaction. // the passed commitment transaction.
func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
broadcastCommitment *wire.MsgTx,
breachHeight uint32) (*BreachRetribution, error) { breachHeight uint32) (*BreachRetribution, error) {
commitHash := broadcastCommitment.TxHash()
// Query the on-disk revocation log for the snapshot which was recorded // Query the on-disk revocation log for the snapshot which was recorded
// at this particular state num. // at this particular state num.
revokedSnapshot, err := chanState.FindPreviousState(stateNum) revokedSnapshot, err := chanState.FindPreviousState(stateNum)
@ -1909,6 +1906,8 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
return nil, err return nil, err
} }
commitHash := revokedSnapshot.CommitTx.TxHash()
// With the state number broadcast known, we can now derive/restore the // With the state number broadcast known, we can now derive/restore the
// proper revocation preimage necessary to sweep the remote party's // proper revocation preimage necessary to sweep the remote party's
// output. // output.
@ -1952,7 +1951,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
remoteOutpoint := wire.OutPoint{ remoteOutpoint := wire.OutPoint{
Hash: commitHash, Hash: commitHash,
} }
for i, txOut := range broadcastCommitment.TxOut { for i, txOut := range revokedSnapshot.CommitTx.TxOut {
switch { switch {
case bytes.Equal(txOut.PkScript, localPkScript): case bytes.Equal(txOut.PkScript, localPkScript):
localOutpoint.Index = uint32(i) localOutpoint.Index = uint32(i)
@ -2090,7 +2089,7 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64,
// swiftly bring justice to the cheating remote party. // swiftly bring justice to the cheating remote party.
return &BreachRetribution{ return &BreachRetribution{
ChainHash: chanState.ChainHash, ChainHash: chanState.ChainHash,
BreachTransaction: broadcastCommitment, BreachTransaction: revokedSnapshot.CommitTx,
BreachHeight: breachHeight, BreachHeight: breachHeight,
RevokedStateNum: stateNum, RevokedStateNum: stateNum,
PendingHTLCs: revokedSnapshot.Htlcs, PendingHTLCs: revokedSnapshot.Htlcs,

@ -5389,7 +5389,6 @@ func TestNewBreachRetributionSkipsDustHtlcs(t *testing.T) {
// At this point, we'll capture the current state number, as well as // At this point, we'll capture the current state number, as well as
// the current commitment. // the current commitment.
revokedCommit := bobChannel.channelState.LocalCommitment.CommitTx
revokedStateNum := aliceChannel.channelState.LocalCommitment.CommitHeight revokedStateNum := aliceChannel.channelState.LocalCommitment.CommitHeight
// We'll now have Bob settle those HTLC's to Alice and then advance // 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 // At this point, we'll now simulate a contract breach by Bob using the
// NewBreachRetribution method. // NewBreachRetribution method.
breachRet, err := NewBreachRetribution( breachRet, err := NewBreachRetribution(
aliceChannel.channelState, revokedStateNum, revokedCommit, 100, aliceChannel.channelState, revokedStateNum, 100,
) )
if err != nil { if err != nil {
t.Fatalf("unable to create breach retribution: %v", err) t.Fatalf("unable to create breach retribution: %v", err)