Skip to content

Commit 10a44ca

Browse files
author
Antoine Riard
committed
Rename amount variables in InputMaterial
As per BIP143, value of the output spent is part of the transaction digest algorithm, providing a wrong value will generate a falsely hash and fail signature verification. Clarity about amount spent is that's why important.
1 parent cf53d53 commit 10a44ca

File tree

2 files changed

+35
-35
lines changed

2 files changed

+35
-35
lines changed

‎lightning/src/ln/channelmonitor.rs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -455,27 +455,27 @@ pub(crate) enum InputMaterial {
455455
pubkey: Option<PublicKey>,
456456
key: SecretKey,
457457
is_htlc: bool,
458-
amount: u64,
458+
revoked_amount: u64,
459459
},
460460
RemoteHTLC {
461461
script: Script,
462462
key: SecretKey,
463463
preimage: Option<PaymentPreimage>,
464-
amount: u64,
464+
remote_amount: u64,
465465
locktime: u32,
466466
},
467467
LocalHTLC {
468468
script: Script,
469469
sigs: (Signature, Signature),
470470
preimage: Option<PaymentPreimage>,
471-
amount: u64,
471+
local_amount: u64,
472472
}
473473
}
474474

475475
impl Writeable for InputMaterial {
476476
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
477477
match self {
478-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount} => {
478+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount} => {
479479
writer.write_all(&[0; 1])?;
480480
script.write(writer)?;
481481
pubkey.write(writer)?;
@@ -485,23 +485,23 @@ impl Writeable for InputMaterial {
485485
} else {
486486
writer.write_all(&[1; 1])?;
487487
}
488-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
488+
writer.write_all(&byte_utils::be64_to_array(*revoked_amount))?;
489489
},
490-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
490+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
491491
writer.write_all(&[1; 1])?;
492492
script.write(writer)?;
493493
key.write(writer)?;
494494
preimage.write(writer)?;
495-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
495+
writer.write_all(&byte_utils::be64_to_array(*remote_amount))?;
496496
writer.write_all(&byte_utils::be32_to_array(*locktime))?;
497497
},
498-
&InputMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref amount } => {
498+
&InputMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref local_amount } => {
499499
writer.write_all(&[2; 1])?;
500500
script.write(writer)?;
501501
sigs.0.write(writer)?;
502502
sigs.1.write(writer)?;
503503
preimage.write(writer)?;
504-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
504+
writer.write_all(&byte_utils::be64_to_array(*local_amount))?;
505505
}
506506
}
507507
Ok(())
@@ -520,26 +520,26 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
520520
1 => false,
521521
_ => return Err(DecodeError::InvalidValue),
522522
};
523-
let amount = Readable::read(reader)?;
523+
let revoked_amount = Readable::read(reader)?;
524524
InputMaterial::Revoked {
525525
script,
526526
pubkey,
527527
key,
528528
is_htlc,
529-
amount
529+
revoked_amount
530530
}
531531
},
532532
1 => {
533533
let script = Readable::read(reader)?;
534534
let key = Readable::read(reader)?;
535535
let preimage = Readable::read(reader)?;
536-
let amount = Readable::read(reader)?;
536+
let remote_amount = Readable::read(reader)?;
537537
let locktime = Readable::read(reader)?;
538538
InputMaterial::RemoteHTLC {
539539
script,
540540
key,
541541
preimage,
542-
amount,
542+
remote_amount,
543543
locktime
544544
}
545545
},
@@ -548,12 +548,12 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
548548
let their_sig = Readable::read(reader)?;
549549
let our_sig = Readable::read(reader)?;
550550
let preimage = Readable::read(reader)?;
551-
let amount = Readable::read(reader)?;
551+
let local_amount = Readable::read(reader)?;
552552
InputMaterial::LocalHTLC {
553553
script,
554554
sigs: (their_sig, our_sig),
555555
preimage,
556-
amount
556+
local_amount
557557
}
558558
}
559559
_ => return Err(DecodeError::InvalidValue),
@@ -1488,7 +1488,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
14881488
// First, process non-htlc outputs (to_local & to_remote)
14891489
for (idx, outp) in tx.output.iter().enumerate() {
14901490
if outp.script_pubkey == revokeable_p2wsh {
1491-
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value };
1491+
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, revoked_amount: outp.value };
14921492
outpoints.push(ClaimRequest { absolute_timelock: height + self.our_to_self_delay as u32, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: idx as u32 }, witness_data});
14931493
} else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
14941494
spendable_outputs.push(SpendableOutputDescriptor::DynamicOutputP2WPKH {
@@ -1509,7 +1509,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
15091509
tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
15101510
return (claim_requests_per_txid, (commitment_txid, watch_outputs), spendable_outputs); // Corrupted per_commitment_data, fuck this user
15111511
}
1512-
let witness_data = InputMaterial::Revoked { script: expected_script, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, amount: tx.output[transaction_output_index as usize].value };
1512+
let witness_data = InputMaterial::Revoked { script: expected_script, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: true, revoked_amount: tx.output[transaction_output_index as usize].value };
15131513
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
15141514
}
15151515
}
@@ -1673,7 +1673,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
16731673
let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
16741674
let aggregable = if !htlc.offered { false } else { true };
16751675
if preimage.is_some() || !htlc.offered {
1676-
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
1676+
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, remote_amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
16771677
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
16781678
}
16791679
}
@@ -1729,7 +1729,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17291729
let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
17301730

17311731
log_trace!(self, "Remote HTLC broadcast {}:{}", htlc_txid, 0);
1732-
let witness_data = InputMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: tx.output[0].value };
1732+
let witness_data = InputMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, revoked_amount: tx.output[0].value };
17331733
let outpoints = vec!(ClaimRequest { absolute_timelock: height + self.our_to_self_delay as u32, aggregable: true, outpoint: BitcoinOutPoint { txid: htlc_txid, vout: 0}, witness_data });
17341734
let mut claimable_outpoints = HashMap::with_capacity(1);
17351735
claimable_outpoints.insert(htlc_txid, outpoints);
@@ -1779,7 +1779,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17791779

17801780
add_dynamic_output!(htlc_timeout_tx, 0);
17811781
let mut per_input_material = HashMap::with_capacity(1);
1782-
per_input_material.insert(htlc_timeout_tx.input[0].previous_output, InputMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, our_sig), preimage: None, amount: htlc.amount_msat / 1000});
1782+
per_input_material.insert(htlc_timeout_tx.input[0].previous_output, InputMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, our_sig), preimage: None, local_amount: htlc.amount_msat / 1000});
17831783
//TODO: with option_simplified_commitment track outpoint too
17841784
log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid);
17851785
res.push(htlc_timeout_tx);
@@ -1795,7 +1795,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17951795

17961796
add_dynamic_output!(htlc_success_tx, 0);
17971797
let mut per_input_material = HashMap::with_capacity(1);
1798-
per_input_material.insert(htlc_success_tx.input[0].previous_output, InputMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, our_sig), preimage: Some(*payment_preimage), amount: htlc.amount_msat / 1000});
1798+
per_input_material.insert(htlc_success_tx.input[0].previous_output, InputMaterial::LocalHTLC { script: htlc_script, sigs: (*their_sig, our_sig), preimage: Some(*payment_preimage), local_amount: htlc.amount_msat / 1000});
17991799
//TODO: with option_simplified_commitment track outpoint too
18001800
log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid);
18011801
res.push(htlc_success_tx);

‎lightning/src/ln/onchaintx.rs

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -392,16 +392,16 @@ impl OnchainTxHandler {
392392

393393
let new_timer = Self::get_height_timer(height, cached_claim_datas.soonest_timelock);
394394
let mut inputs_witnesses_weight = 0;
395-
let mut amt = 0;
395+
let mut total_amount = 0;
396396
for per_outp_material in cached_claim_datas.per_input_material.values() {
397397
match per_outp_material {
398-
&InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
398+
&InputMaterial::Revoked { ref script, ref is_htlc, ref revoked_amount, .. } => {
399399
inputs_witnesses_weight += Self::get_witnesses_weight(if !is_htlc { &[InputDescriptors::RevokedOutput] } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::OfferedHTLC) { &[InputDescriptors::RevokedOfferedHTLC] } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::AcceptedHTLC) { &[InputDescriptors::RevokedReceivedHTLC] } else { &[] });
400-
amt += *amount;
400+
total_amount += *revoked_amount;
401401
},
402-
&InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
402+
&InputMaterial::RemoteHTLC { ref preimage, ref remote_amount, .. } => {
403403
inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] });
404-
amt += *amount;
404+
total_amount += *remote_amount;
405405
},
406406
&InputMaterial::LocalHTLC { .. } => { return None; }
407407
}
@@ -411,27 +411,27 @@ impl OnchainTxHandler {
411411
let mut new_feerate;
412412
// If old feerate is 0, first iteration of this claim, use normal fee calculation
413413
if cached_claim_datas.feerate_previous != 0 {
414-
if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
414+
if let Some((new_fee, feerate)) = RBF_bump!(total_amount, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
415415
// If new computed fee is superior at the whole claimable amount burn all in fees
416-
if new_fee > amt {
416+
if new_fee > total_amount {
417417
bumped_tx.output[0].value = 0;
418418
} else {
419-
bumped_tx.output[0].value = amt - new_fee;
419+
bumped_tx.output[0].value = total_amount - new_fee;
420420
}
421421
new_feerate = feerate;
422422
} else { return None; }
423423
} else {
424-
if subtract_high_prio_fee!(self, fee_estimator, amt, predicted_weight, new_feerate) {
425-
bumped_tx.output[0].value = amt;
424+
if subtract_high_prio_fee!(self, fee_estimator, total_amount, predicted_weight, new_feerate) {
425+
bumped_tx.output[0].value = total_amount;
426426
} else { return None; }
427427
}
428428
assert!(new_feerate != 0);
429429

430430
for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
431431
match per_outp_material {
432-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount } => {
432+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount } => {
433433
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
434-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
434+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *revoked_amount)[..]);
435435
let sig = self.secp_ctx.sign(&sighash, &key);
436436
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
437437
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
@@ -443,10 +443,10 @@ impl OnchainTxHandler {
443443
bumped_tx.input[i].witness.push(script.clone().into_bytes());
444444
log_trace!(self, "Going to broadcast Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}...", bumped_tx.txid(), if !is_htlc { "to_local" } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::OfferedHTLC) { "offered" } else if HTLCType::scriptlen_to_htlctype(script.len()) == Some(HTLCType::AcceptedHTLC) { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
445445
},
446-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
446+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
447447
if !preimage.is_some() { bumped_tx.lock_time = *locktime }; // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
448448
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
449-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
449+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *remote_amount)[..]);
450450
let sig = self.secp_ctx.sign(&sighash, &key);
451451
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
452452
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);

0 commit comments

Comments
 (0)