lnwallet: ensure HTLC values are properly converted to SAT in commit tx

This commit fixes a bug within the HTLC construction and commitment
transaction construction that would result in HTLC _values_ within the
commitment transaction being off by a factor of 1000. This was due to
the fact that we failed to convert the amount of an HTLC, in mSAT, to
SAT before placing it as an output within the commitment transaction.
When attempt to locate the output index of a particular half, we use
the unconverted amount, meaning it was unnoticed.

This commit adds a new assertion within the TestSimpleAddSettleWorkflow
test to ensure that the HTLC is found within the commitment transaction
with the proper value in satoshi.
This commit is contained in:
Olaoluwa Osuntokun 2017-08-30 15:25:01 -07:00
parent af52aa838e
commit a52d405998
No known key found for this signature in database
GPG Key ID: 9CC5B105D03521A2
2 changed files with 32 additions and 3 deletions

@ -321,7 +321,7 @@ func locateOutputIndex(p *PaymentDescriptor, tx *wire.MsgTx, ourCommit bool,
for i, txOut := range tx.TxOut {
if bytes.Equal(txOut.PkScript, pkScript) &&
txOut.Value == int64(p.Amount) {
txOut.Value == int64(p.Amount.ToSatoshis()) {
// If this payment hash and index has already been
// found, then we'll continue in order to avoid any
@ -3274,7 +3274,7 @@ func (lc *LightningChannel) addHTLC(commitTx *wire.MsgTx, ourCommit bool,
}
// Add the new HTLC outputs to the respective commitment transactions.
amountPending := int64(paymentDesc.Amount)
amountPending := int64(paymentDesc.Amount.ToSatoshis())
commitTx.AddTxOut(wire.NewTxOut(amountPending, p2wsh))
// Store the pkScript of this particular PaymentDescriptor so we can

@ -392,6 +392,19 @@ func createHTLC(data int, amount lnwire.MilliSatoshi) (*lnwire.UpdateAddHTLC, [3
}, returnPreimage
}
func assertOutputExistsByValue(t *testing.T, commitTx *wire.MsgTx,
value btcutil.Amount) {
for _, txOut := range commitTx.TxOut {
if txOut.Value == int64(value) {
return
}
}
t.Fatalf("unable to find output of value %v within tx %v", value,
spew.Sdump(commitTx))
}
// TestSimpleAddSettleWorkflow tests a simple channel scenario wherein the
// local node (Alice in this case) creates a new outgoing HTLC to bob, commits
// this change, then bob immediately commits a settlement of the HTLC after the
@ -415,9 +428,10 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
paymentPreimage := bytes.Repeat([]byte{1}, 32)
paymentHash := sha256.Sum256(paymentPreimage)
htlcAmt := lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin)
htlc := &lnwire.UpdateAddHTLC{
PaymentHash: paymentHash,
Amount: lnwire.NewMSatFromSatoshis(btcutil.SatoshiPerBitcoin),
Amount: htlcAmt,
Expiry: uint32(5),
}
@ -529,6 +543,21 @@ func TestSimpleAddSettleWorkflow(t *testing.T) {
aliceChannel.currentHeight, 1)
}
// Both commitment transactions should have three outputs, and one of
// them should be exactly the amount of the HTLC.
if len(aliceChannel.channelState.CommitTx.TxOut) != 3 {
t.Fatalf("alice should have three commitment outputs, instead "+
"have %v", len(aliceChannel.channelState.CommitTx.TxOut))
}
if len(bobChannel.channelState.CommitTx.TxOut) != 3 {
t.Fatalf("bob should have three commitment outputs, instead "+
"have %v", len(bobChannel.channelState.CommitTx.TxOut))
}
assertOutputExistsByValue(t, &aliceChannel.channelState.CommitTx,
htlcAmt.ToSatoshis())
assertOutputExistsByValue(t, &bobChannel.channelState.CommitTx,
htlcAmt.ToSatoshis())
// Now we'll repeat a similar exchange, this time with Bob settling the
// HTLC once he learns of the preimage.
var preimage [32]byte