diff --git a/lnwallet/channel.go b/lnwallet/channel.go index 22717eda..c7804c0f 100644 --- a/lnwallet/channel.go +++ b/lnwallet/channel.go @@ -1881,8 +1881,9 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, if err != nil { return nil, err } - commitmentSecret, commitmentPoint := btcec.PrivKeyFromBytes(btcec.S256(), - revocationPreimage[:]) + commitmentSecret, commitmentPoint := btcec.PrivKeyFromBytes( + btcec.S256(), revocationPreimage[:], + ) // With the commitment point generated, we can now generate the four // keys we'll need to reconstruct the commitment state, @@ -1893,8 +1894,9 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, // number so we can have the proper witness script to sign and include // within the final witness. remoteDelay := uint32(chanState.RemoteChanCfg.CsvDelay) - remotePkScript, err := commitScriptToSelf(remoteDelay, keyRing.DelayKey, - keyRing.RevocationKey) + remotePkScript, err := commitScriptToSelf( + remoteDelay, keyRing.DelayKey, keyRing.RevocationKey, + ) if err != nil { return nil, err } @@ -1999,10 +2001,10 @@ func NewBreachRetribution(chanState *channeldb.OpenChannel, stateNum uint64, return nil, err } - // Otherwise, is this was an outgoing HTLC that we sent, then - // from the PoV of the remote commitment state, they're the - // receiver of this HTLC. } else { + // Otherwise, is this was an outgoing HTLC that we + // sent, then from the PoV of the remote commitment + // state, they're the receiver of this HTLC. htlcScript, err = receiverHTLCScript( htlc.RefundTimeout, keyRing.LocalHtlcKey, keyRing.RemoteHtlcKey, keyRing.RevocationKey, @@ -2582,9 +2584,11 @@ func genRemoteHtlcSigJobs(keyRing *CommitmentKeyRing, Hash: txHash, Index: uint32(htlc.remoteOutputIndex), } - sigJob.tx, err = createHtlcTimeoutTx(op, outputAmt, - htlc.Timeout, uint32(remoteChanCfg.CsvDelay), - keyRing.RevocationKey, keyRing.DelayKey) + sigJob.tx, err = createHtlcTimeoutTx( + op, outputAmt, htlc.Timeout, + uint32(remoteChanCfg.CsvDelay), + keyRing.RevocationKey, keyRing.DelayKey, + ) if err != nil { return nil, nil, err } @@ -2632,9 +2636,10 @@ func genRemoteHtlcSigJobs(keyRing *CommitmentKeyRing, Hash: txHash, Index: uint32(htlc.remoteOutputIndex), } - sigJob.tx, err = createHtlcSuccessTx(op, outputAmt, - uint32(remoteChanCfg.CsvDelay), keyRing.RevocationKey, - keyRing.DelayKey) + sigJob.tx, err = createHtlcSuccessTx( + op, outputAmt, uint32(remoteChanCfg.CsvDelay), + keyRing.RevocationKey, keyRing.DelayKey, + ) if err != nil { return nil, nil, err } @@ -3483,20 +3488,23 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment, i := 0 for index := range localCommitmentView.txn.TxOut { var ( - sigHash func() ([]byte, error) - sig *btcec.Signature - err error + htlcIndex uint64 + sigHash func() ([]byte, error) + sig *btcec.Signature + err error ) outputIndex := int32(index) switch { - // If this output index is found within the incoming HTLC index, - // then this means that we need to generate an HTLC success - // transaction in order to validate the signature. + // If this output index is found within the incoming HTLC + // index, then this means that we need to generate an HTLC + // success transaction in order to validate the signature. case localCommitmentView.incomingHTLCIndex[outputIndex] != nil: htlc := localCommitmentView.incomingHTLCIndex[outputIndex] + htlcIndex = htlc.HtlcIndex + sigHash = func() ([]byte, error) { op := wire.OutPoint{ Hash: txHash, @@ -3547,6 +3555,8 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment, case localCommitmentView.outgoingHTLCIndex[outputIndex] != nil: htlc := localCommitmentView.outgoingHTLCIndex[outputIndex] + htlcIndex = htlc.HtlcIndex + sigHash = func() ([]byte, error) { op := wire.OutPoint{ Hash: txHash, @@ -3598,9 +3608,10 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment, } verifyJobs = append(verifyJobs, verifyJob{ - pubKey: keyRing.RemoteHtlcKey, - sig: sig, - sigHash: sigHash, + htlcIndex: htlcIndex, + pubKey: keyRing.RemoteHtlcKey, + sig: sig, + sigHash: sigHash, }) i++ @@ -3617,7 +3628,7 @@ func genHtlcSigValidationJobs(localCommitmentView *commitment, } // InvalidCommitSigError is a struct that implements the error interface to -// report a failure to validation a commitment signature for a remote peer. +// report a failure to validate a commitment signature for a remote peer. // We'll use the items in this struct to generate a rich error message for the // remote peer when we receive an invalid signature from it. Doing so can // greatly aide in debugging cross implementation issues. @@ -3635,7 +3646,7 @@ type InvalidCommitSigError struct { // caused an invalid commitment signature. func (i *InvalidCommitSigError) Error() string { return fmt.Sprintf("rejected commitment: commit_height=%v, "+ - "invalid_sig=%x, commit_tx=%x, sig_hash=%x", i.commitHeight, + "invalid_commit_sig=%x, commit_tx=%x, sig_hash=%x", i.commitHeight, i.commitSig[:], i.commitTx, i.sigHash[:]) } @@ -3643,6 +3654,35 @@ func (i *InvalidCommitSigError) Error() string { // error interface. var _ error = (*InvalidCommitSigError)(nil) +// InvalidCommitSigError is a struc that implements the error interface to +// report a failure to validate an htlc signature from a remote peer. We'll use +// the items in this struct to generate a rich error message for the remote +// peer when we receive an invalid signature from it. Doing so can greatly aide +// in debugging across implementation issues. +type InvalidHtlcSigError struct { + commitHeight uint64 + + htlcSig []byte + + htlcIndex uint64 + + sigHash []byte + + commitTx []byte +} + +// Error returns a detailed error string including the exact transaction that +// caused an invalid htlc signature. +func (i *InvalidHtlcSigError) Error() string { + return fmt.Sprintf("rejected commitment: commit_height=%v, "+ + "invalid_htlc_sig=%x, commit_tx=%x, sig_hash=%x", i.commitHeight, + i.htlcSig, i.commitTx, i.sigHash[:]) +} + +// A compile time flag to ensure that InvalidCommitSigError implements the +// error interface. +var _ error = (*InvalidCommitSigError)(nil) + // ReceiveNewCommitment process a signature for a new commitment state sent by // the remote party. This method should be called in response to the // remote party initiating a new change, or when the remote party sends a @@ -3772,10 +3812,30 @@ func (lc *LightningChannel) ReceiveNewCommitment(commitSig lnwire.Sig, // In the case that a single signature is invalid, we'll exit // early and cancel all the outstanding verification jobs. select { - case err := <-verifyResps: - if err != nil { + case htlcErr := <-verifyResps: + if htlcErr != nil { close(cancelChan) - return fmt.Errorf("invalid htlc signature: %v", err) + + sig, err := lnwire.NewSigFromSignature( + htlcErr.sig, + ) + if err != nil { + return err + } + sigHash, err := htlcErr.sigHash() + if err != nil { + return err + } + + var txBytes bytes.Buffer + localCommitTx.Serialize(&txBytes) + return &InvalidHtlcSigError{ + commitHeight: nextHeight, + htlcSig: sig.ToSignatureBytes(), + htlcIndex: htlcErr.htlcIndex, + sigHash: sigHash, + commitTx: txBytes.Bytes(), + } } case <-lc.quit: return fmt.Errorf("channel shutting down") diff --git a/lnwallet/sigpool.go b/lnwallet/sigpool.go index a93c9630..defbf8ba 100644 --- a/lnwallet/sigpool.go +++ b/lnwallet/sigpool.go @@ -41,6 +41,10 @@ type verifyJob struct { // passed signature is known to have signed. sigHash func() ([]byte, error) + // htlcIndex is the index of the HTLC from the PoV of the remote + // party's update log. + htlcIndex uint64 + // cancel is a channel that should be closed if the caller wishes to // cancel all pending verification jobs part of a single batch. This // channel is to be closed in the case that a single signature in a @@ -52,7 +56,16 @@ type verifyJob struct { // is to be sent over. In the see that the signature is valid, a nil // error will be passed. Otherwise, a concrete error detailing the // issue will be passed. - errResp chan error + errResp chan *htlcIndexErr +} + +// verifyJobErr is a special type of error that also includes a pointer to the +// original validation job. Ths error message allows us to craft more detailed +// errors at upper layers. +type htlcIndexErr struct { + error + + *verifyJob } // signJob is a job sent to the sigPool to generate a valid signature according @@ -69,7 +82,8 @@ type signJob struct { // proper sighash for the input to be signed. tx *wire.MsgTx - // outputIndex... + // outputIndex is the output index of the HTLC on the commitment + // transaction being signed. outputIndex int32 // cancel is a channel that should be closed if the caller wishes to @@ -216,7 +230,10 @@ func (s *sigPool) poolWorker() { sigHash, err := verifyMsg.sigHash() if err != nil { select { - case verifyMsg.errResp <- err: + case verifyMsg.errResp <- &htlcIndexErr{ + error: err, + verifyJob: &verifyMsg, + }: continue case <-verifyMsg.cancel: continue @@ -229,7 +246,10 @@ func (s *sigPool) poolWorker() { err := fmt.Errorf("invalid signature "+ "sighash: %x, sig: %x", sigHash, rawSig.Serialize()) select { - case verifyMsg.errResp <- err: + case verifyMsg.errResp <- &htlcIndexErr{ + error: err, + verifyJob: &verifyMsg, + }: case <-verifyMsg.cancel: case <-s.quit: return @@ -272,9 +292,9 @@ func (s *sigPool) SubmitSignBatch(signJobs []signJob) { // allows the caller to cancel all pending jobs in the case that they wish to // bail early. func (s *sigPool) SubmitVerifyBatch(verifyJobs []verifyJob, - cancelChan chan struct{}) <-chan error { + cancelChan chan struct{}) <-chan *htlcIndexErr { - errChan := make(chan error, len(verifyJobs)) + errChan := make(chan *htlcIndexErr, len(verifyJobs)) for _, job := range verifyJobs { job.cancel = cancelChan