diff --git a/htlcswitch/interfaces.go b/htlcswitch/interfaces.go index d4bcc0a8..5e162795 100644 --- a/htlcswitch/interfaces.go +++ b/htlcswitch/interfaces.go @@ -175,6 +175,8 @@ type TowerClient interface { // state. If the method returns nil, the backup is guaranteed to be // successful unless the tower is unavailable and client is force quit, // or the justice transaction would create dust outputs when trying to - // abide by the negotiated policy. - BackupState(*lnwire.ChannelID, *lnwallet.BreachRetribution) error + // abide by the negotiated policy. If the channel we're trying to back + // up doesn't have a tweak for the remote party's output, then + // isTweakless should be true. + BackupState(*lnwire.ChannelID, *lnwallet.BreachRetribution, bool) error } diff --git a/htlcswitch/link.go b/htlcswitch/link.go index 351a7623..0539bde1 100644 --- a/htlcswitch/link.go +++ b/htlcswitch/link.go @@ -954,7 +954,7 @@ func (l *channelLink) htlcManager() { l.fail( LinkFailureError{ - code: ErrSyncError, + code: ErrRecoveryError, ForceClose: false, }, "unable to synchronize channel "+ @@ -1844,8 +1844,13 @@ func (l *channelLink) handleUpstreamMsg(msg lnwire.Message) { return } + chanType := l.channel.State().ChanType + isTweakless := chanType == channeldb.SingleFunderTweakless + chanID := l.ChanID() - err = l.cfg.TowerClient.BackupState(&chanID, breachInfo) + err = l.cfg.TowerClient.BackupState( + &chanID, breachInfo, isTweakless, + ) if err != nil { l.fail(LinkFailureError{code: ErrInternalError}, "unable to queue breach backup: %v", err) diff --git a/htlcswitch/linkfailure.go b/htlcswitch/linkfailure.go index 2f44e434..c806c4b2 100644 --- a/htlcswitch/linkfailure.go +++ b/htlcswitch/linkfailure.go @@ -34,6 +34,11 @@ const ( // ErrInvalidRevocation indicates that the remote peer send us an // invalid revocation message. ErrInvalidRevocation + + // ErrRecoveryError the channel was unable to be resumed, we need the + // remote party to force close the channel out on chain now as a + // result. + ErrRecoveryError ) // LinkFailureError encapsulates an error that will make us fail the current @@ -74,6 +79,8 @@ func (e LinkFailureError) Error() string { return "invalid commitment" case ErrInvalidRevocation: return "invalid revocation" + case ErrRecoveryError: + return "unable to resume channel, recovery required" default: return "unknown error" } diff --git a/watchtower/lookout/justice_descriptor_test.go b/watchtower/lookout/justice_descriptor_test.go index 1f880c23..7fd93c8c 100644 --- a/watchtower/lookout/justice_descriptor_test.go +++ b/watchtower/lookout/justice_descriptor_test.go @@ -254,7 +254,7 @@ func testJusticeDescriptor(t *testing.T, blobType blob.Type) { // DER-encoded signature under the to-remote pubkey. The sighash flag is // also present, so we trim it. toRemoteWitness, err := input.CommitSpendNoDelay( - signer, toRemoteSignDesc, justiceTxn, + signer, toRemoteSignDesc, justiceTxn, false, ) if err != nil { t.Fatalf("unable to sign to-remote input: %v", err) diff --git a/watchtower/wtclient/backup_task.go b/watchtower/wtclient/backup_task.go index 9f284b98..abdabe2f 100644 --- a/watchtower/wtclient/backup_task.go +++ b/watchtower/wtclient/backup_task.go @@ -54,7 +54,7 @@ type backupTask struct { // variables. func newBackupTask(chanID *lnwire.ChannelID, breachInfo *lnwallet.BreachRetribution, - sweepPkScript []byte) *backupTask { + sweepPkScript []byte, isTweakless bool) *backupTask { // Parse the non-dust outputs from the breach transaction, // simultaneously computing the total amount contained in the inputs @@ -85,12 +85,18 @@ func newBackupTask(chanID *lnwire.ChannelID, totalAmt += breachInfo.RemoteOutputSignDesc.Output.Value } if breachInfo.LocalOutputSignDesc != nil { + witnessType := input.CommitmentNoDelay + if isTweakless { + witnessType = input.CommitSpendNoDelayTweakless + } + toRemoteInput = input.NewBaseInput( &breachInfo.LocalOutpoint, - input.CommitmentNoDelay, + witnessType, breachInfo.LocalOutputSignDesc, 0, ) + totalAmt += breachInfo.LocalOutputSignDesc.Output.Value } @@ -271,6 +277,8 @@ func (t *backupTask) craftSessionPayload( case input.CommitmentRevoke: copy(justiceKit.CommitToLocalSig[:], signature[:]) + case input.CommitSpendNoDelayTweakless: + fallthrough case input.CommitmentNoDelay: copy(justiceKit.CommitToRemoteSig[:], signature[:]) } diff --git a/watchtower/wtclient/backup_task_internal_test.go b/watchtower/wtclient/backup_task_internal_test.go index 5bc0dbb4..cff6f2b8 100644 --- a/watchtower/wtclient/backup_task_internal_test.go +++ b/watchtower/wtclient/backup_task_internal_test.go @@ -73,6 +73,7 @@ type backupTaskTest struct { bindErr error expSweepScript []byte signer input.Signer + tweakless bool } // genTaskTest creates a instance of a backupTaskTest using the passed @@ -89,7 +90,8 @@ func genTaskTest( rewardScript []byte, expSweepAmt int64, expRewardAmt int64, - bindErr error) backupTaskTest { + bindErr error, + tweakless bool) backupTaskTest { // Parse the key pairs for all keys used in the test. revSK, revPK := btcec.PrivKeyFromBytes( @@ -188,9 +190,15 @@ func genTaskTest( Hash: txid, Index: index, } + + witnessType := input.CommitmentNoDelay + if tweakless { + witnessType = input.CommitSpendNoDelayTweakless + } + toRemoteInput = input.NewBaseInput( &breachInfo.LocalOutpoint, - input.CommitmentNoDelay, + witnessType, breachInfo.LocalOutputSignDesc, 0, ) @@ -218,6 +226,7 @@ func genTaskTest( bindErr: bindErr, expSweepScript: makeAddrSlice(22), signer: signer, + tweakless: tweakless, } } @@ -233,153 +242,6 @@ var ( addrScript, _ = txscript.PayToAddrScript(addr) ) -var backupTaskTests = []backupTaskTest{ - genTaskTest( - "commit no-reward, both outputs", - 100, // stateNum - 200000, // toLocalAmt - 100000, // toRemoteAmt - blobTypeCommitNoReward, // blobType - 1000, // sweepFeeRate - nil, // rewardScript - 299241, // expSweepAmt - 0, // expRewardAmt - nil, // bindErr - ), - genTaskTest( - "commit no-reward, to-local output only", - 1000, // stateNum - 200000, // toLocalAmt - 0, // toRemoteAmt - blobTypeCommitNoReward, // blobType - 1000, // sweepFeeRate - nil, // rewardScript - 199514, // expSweepAmt - 0, // expRewardAmt - nil, // bindErr - ), - genTaskTest( - "commit no-reward, to-remote output only", - 1, // stateNum - 0, // toLocalAmt - 100000, // toRemoteAmt - blobTypeCommitNoReward, // blobType - 1000, // sweepFeeRate - nil, // rewardScript - 99561, // expSweepAmt - 0, // expRewardAmt - nil, // bindErr - ), - genTaskTest( - "commit no-reward, to-remote output only, creates dust", - 1, // stateNum - 0, // toLocalAmt - 100000, // toRemoteAmt - blobTypeCommitNoReward, // blobType - 227500, // sweepFeeRate - nil, // rewardScript - 0, // expSweepAmt - 0, // expRewardAmt - wtpolicy.ErrCreatesDust, // bindErr - ), - genTaskTest( - "commit no-reward, no outputs, fee rate exceeds inputs", - 300, // stateNum - 0, // toLocalAmt - 0, // toRemoteAmt - blobTypeCommitNoReward, // blobType - 1000, // sweepFeeRate - nil, // rewardScript - 0, // expSweepAmt - 0, // expRewardAmt - wtpolicy.ErrFeeExceedsInputs, // bindErr - ), - genTaskTest( - "commit no-reward, no outputs, fee rate of 0 creates dust", - 300, // stateNum - 0, // toLocalAmt - 0, // toRemoteAmt - blobTypeCommitNoReward, // blobType - 0, // sweepFeeRate - nil, // rewardScript - 0, // expSweepAmt - 0, // expRewardAmt - wtpolicy.ErrCreatesDust, // bindErr - ), - genTaskTest( - "commit reward, both outputs", - 100, // stateNum - 200000, // toLocalAmt - 100000, // toRemoteAmt - blobTypeCommitReward, // blobType - 1000, // sweepFeeRate - addrScript, // rewardScript - 296117, // expSweepAmt - 3000, // expRewardAmt - nil, // bindErr - ), - genTaskTest( - "commit reward, to-local output only", - 1000, // stateNum - 200000, // toLocalAmt - 0, // toRemoteAmt - blobTypeCommitReward, // blobType - 1000, // sweepFeeRate - addrScript, // rewardScript - 197390, // expSweepAmt - 2000, // expRewardAmt - nil, // bindErr - ), - genTaskTest( - "commit reward, to-remote output only", - 1, // stateNum - 0, // toLocalAmt - 100000, // toRemoteAmt - blobTypeCommitReward, // blobType - 1000, // sweepFeeRate - addrScript, // rewardScript - 98437, // expSweepAmt - 1000, // expRewardAmt - nil, // bindErr - ), - genTaskTest( - "commit reward, to-remote output only, creates dust", - 1, // stateNum - 0, // toLocalAmt - 100000, // toRemoteAmt - blobTypeCommitReward, // blobType - 175000, // sweepFeeRate - addrScript, // rewardScript - 0, // expSweepAmt - 0, // expRewardAmt - wtpolicy.ErrCreatesDust, // bindErr - ), - genTaskTest( - "commit reward, no outputs, fee rate exceeds inputs", - 300, // stateNum - 0, // toLocalAmt - 0, // toRemoteAmt - blobTypeCommitReward, // blobType - 1000, // sweepFeeRate - addrScript, // rewardScript - 0, // expSweepAmt - 0, // expRewardAmt - wtpolicy.ErrFeeExceedsInputs, // bindErr - ), - genTaskTest( - "commit reward, no outputs, fee rate of 0 creates dust", - 300, // stateNum - 0, // toLocalAmt - 0, // toRemoteAmt - blobTypeCommitReward, // blobType - 0, // sweepFeeRate - addrScript, // rewardScript - 0, // expSweepAmt - 0, // expRewardAmt - wtpolicy.ErrCreatesDust, // bindErr - ), -} - // TestBackupTaskBind tests the initialization and binding of a backupTask to a // ClientSession. After a successful bind, all parameters of the justice // transaction should be solidified, so we assert there correctness. In an @@ -390,8 +252,174 @@ var backupTaskTests = []backupTaskTest{ func TestBackupTask(t *testing.T) { t.Parallel() + var backupTaskTests []backupTaskTest + for _, tweakless := range []bool{true, false} { + backupTaskTests = append(backupTaskTests, []backupTaskTest{ + genTaskTest( + "commit no-reward, both outputs", + 100, // stateNum + 200000, // toLocalAmt + 100000, // toRemoteAmt + blobTypeCommitNoReward, // blobType + 1000, // sweepFeeRate + nil, // rewardScript + 299241, // expSweepAmt + 0, // expRewardAmt + nil, // bindErr + tweakless, + ), + genTaskTest( + "commit no-reward, to-local output only", + 1000, // stateNum + 200000, // toLocalAmt + 0, // toRemoteAmt + blobTypeCommitNoReward, // blobType + 1000, // sweepFeeRate + nil, // rewardScript + 199514, // expSweepAmt + 0, // expRewardAmt + nil, // bindErr + tweakless, + ), + genTaskTest( + "commit no-reward, to-remote output only", + 1, // stateNum + 0, // toLocalAmt + 100000, // toRemoteAmt + blobTypeCommitNoReward, // blobType + 1000, // sweepFeeRate + nil, // rewardScript + 99561, // expSweepAmt + 0, // expRewardAmt + nil, // bindErr + tweakless, + ), + genTaskTest( + "commit no-reward, to-remote output only, creates dust", + 1, // stateNum + 0, // toLocalAmt + 100000, // toRemoteAmt + blobTypeCommitNoReward, // blobType + 227500, // sweepFeeRate + nil, // rewardScript + 0, // expSweepAmt + 0, // expRewardAmt + wtpolicy.ErrCreatesDust, // bindErr + tweakless, + ), + genTaskTest( + "commit no-reward, no outputs, fee rate exceeds inputs", + 300, // stateNum + 0, // toLocalAmt + 0, // toRemoteAmt + blobTypeCommitNoReward, // blobType + 1000, // sweepFeeRate + nil, // rewardScript + 0, // expSweepAmt + 0, // expRewardAmt + wtpolicy.ErrFeeExceedsInputs, // bindErr + tweakless, + ), + genTaskTest( + "commit no-reward, no outputs, fee rate of 0 creates dust", + 300, // stateNum + 0, // toLocalAmt + 0, // toRemoteAmt + blobTypeCommitNoReward, // blobType + 0, // sweepFeeRate + nil, // rewardScript + 0, // expSweepAmt + 0, // expRewardAmt + wtpolicy.ErrCreatesDust, // bindErr + tweakless, + ), + genTaskTest( + "commit reward, both outputs", + 100, // stateNum + 200000, // toLocalAmt + 100000, // toRemoteAmt + blobTypeCommitReward, // blobType + 1000, // sweepFeeRate + addrScript, // rewardScript + 296117, // expSweepAmt + 3000, // expRewardAmt + nil, // bindErr + tweakless, + ), + genTaskTest( + "commit reward, to-local output only", + 1000, // stateNum + 200000, // toLocalAmt + 0, // toRemoteAmt + blobTypeCommitReward, // blobType + 1000, // sweepFeeRate + addrScript, // rewardScript + 197390, // expSweepAmt + 2000, // expRewardAmt + nil, // bindErr + tweakless, + ), + genTaskTest( + "commit reward, to-remote output only", + 1, // stateNum + 0, // toLocalAmt + 100000, // toRemoteAmt + blobTypeCommitReward, // blobType + 1000, // sweepFeeRate + addrScript, // rewardScript + 98437, // expSweepAmt + 1000, // expRewardAmt + nil, // bindErr + tweakless, + ), + genTaskTest( + "commit reward, to-remote output only, creates dust", + 1, // stateNum + 0, // toLocalAmt + 100000, // toRemoteAmt + blobTypeCommitReward, // blobType + 175000, // sweepFeeRate + addrScript, // rewardScript + 0, // expSweepAmt + 0, // expRewardAmt + wtpolicy.ErrCreatesDust, // bindErr + tweakless, + ), + genTaskTest( + "commit reward, no outputs, fee rate exceeds inputs", + 300, // stateNum + 0, // toLocalAmt + 0, // toRemoteAmt + blobTypeCommitReward, // blobType + 1000, // sweepFeeRate + addrScript, // rewardScript + 0, // expSweepAmt + 0, // expRewardAmt + wtpolicy.ErrFeeExceedsInputs, // bindErr + tweakless, + ), + genTaskTest( + "commit reward, no outputs, fee rate of 0 creates dust", + 300, // stateNum + 0, // toLocalAmt + 0, // toRemoteAmt + blobTypeCommitReward, // blobType + 0, // sweepFeeRate + addrScript, // rewardScript + 0, // expSweepAmt + 0, // expRewardAmt + wtpolicy.ErrCreatesDust, // bindErr + tweakless, + ), + }...) + } + for _, test := range backupTaskTests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + testBackupTask(t, test) }) } @@ -399,7 +427,10 @@ func TestBackupTask(t *testing.T) { func testBackupTask(t *testing.T, test backupTaskTest) { // Create a new backupTask from the channel id and breach info. - task := newBackupTask(&test.chanID, test.breachInfo, test.expSweepScript) + task := newBackupTask( + &test.chanID, test.breachInfo, test.expSweepScript, + test.tweakless, + ) // Assert that all parameters set during initialization are properly // populated. diff --git a/watchtower/wtclient/client.go b/watchtower/wtclient/client.go index 795f1798..76aa2a4b 100644 --- a/watchtower/wtclient/client.go +++ b/watchtower/wtclient/client.go @@ -90,8 +90,10 @@ type Client interface { // state. If the method returns nil, the backup is guaranteed to be // successful unless the client is force quit, or the justice // transaction would create dust outputs when trying to abide by the - // negotiated policy. - BackupState(*lnwire.ChannelID, *lnwallet.BreachRetribution) error + // negotiated policy. If the channel we're trying to back up doesn't + // have a tweak for the remote party's output, then isTweakless should + // be true. + BackupState(*lnwire.ChannelID, *lnwallet.BreachRetribution, bool) error // Start initializes the watchtower client, allowing it process requests // to backup revoked channel states. @@ -564,7 +566,7 @@ func (c *TowerClient) RegisterChannel(chanID lnwire.ChannelID) error { // - breached outputs contain too little value to sweep at the target sweep fee // rate. func (c *TowerClient) BackupState(chanID *lnwire.ChannelID, - breachInfo *lnwallet.BreachRetribution) error { + breachInfo *lnwallet.BreachRetribution, isTweakless bool) error { // Retrieve the cached sweep pkscript used for this channel. c.backupMu.Lock() @@ -589,7 +591,9 @@ func (c *TowerClient) BackupState(chanID *lnwire.ChannelID, c.chanCommitHeights[*chanID] = breachInfo.RevokedStateNum c.backupMu.Unlock() - task := newBackupTask(chanID, breachInfo, summary.SweepPkScript) + task := newBackupTask( + chanID, breachInfo, summary.SweepPkScript, isTweakless, + ) return c.pipeline.QueueBackupTask(task) } diff --git a/watchtower/wtclient/client_test.go b/watchtower/wtclient/client_test.go index e9019ad9..bd16ac06 100644 --- a/watchtower/wtclient/client_test.go +++ b/watchtower/wtclient/client_test.go @@ -628,7 +628,7 @@ func (h *testHarness) backupState(id, i uint64, expErr error) { _, retribution := h.channel(id).getState(i) chanID := chanIDFromInt(id) - err := h.client.BackupState(&chanID, retribution) + err := h.client.BackupState(&chanID, retribution, false) if err != expErr { h.t.Fatalf("back error mismatch, want: %v, got: %v", expErr, err)