Skip to content

Commit 90cd9f4

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 f9d7581 commit 90cd9f4

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
@@ -456,27 +456,27 @@ pub(crate) enum InputMaterial {
456456
pubkey: Option<PublicKey>,
457457
key: SecretKey,
458458
is_htlc: bool,
459-
amount: u64,
459+
revoked_amount: u64,
460460
},
461461
RemoteHTLC {
462462
script: Script,
463463
key: SecretKey,
464464
preimage: Option<PaymentPreimage>,
465-
amount: u64,
465+
remote_amount: u64,
466466
locktime: u32,
467467
},
468468
LocalHTLC {
469469
script: Script,
470470
sigs: (Signature, Signature),
471471
preimage: Option<PaymentPreimage>,
472-
amount: u64,
472+
local_amount: u64,
473473
}
474474
}
475475

476476
impl Writeable for InputMaterial {
477477
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
478478
match self {
479-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount} => {
479+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount} => {
480480
writer.write_all(&[0; 1])?;
481481
script.write(writer)?;
482482
pubkey.write(writer)?;
@@ -486,23 +486,23 @@ impl Writeable for InputMaterial {
486486
} else {
487487
writer.write_all(&[1; 1])?;
488488
}
489-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
489+
writer.write_all(&byte_utils::be64_to_array(*revoked_amount))?;
490490
},
491-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
491+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
492492
writer.write_all(&[1; 1])?;
493493
script.write(writer)?;
494494
key.write(writer)?;
495495
preimage.write(writer)?;
496-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
496+
writer.write_all(&byte_utils::be64_to_array(*remote_amount))?;
497497
writer.write_all(&byte_utils::be32_to_array(*locktime))?;
498498
},
499-
&InputMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref amount } => {
499+
&InputMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref local_amount } => {
500500
writer.write_all(&[2; 1])?;
501501
script.write(writer)?;
502502
sigs.0.write(writer)?;
503503
sigs.1.write(writer)?;
504504
preimage.write(writer)?;
505-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
505+
writer.write_all(&byte_utils::be64_to_array(*local_amount))?;
506506
}
507507
}
508508
Ok(())
@@ -521,26 +521,26 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
521521
1 => false,
522522
_ => return Err(DecodeError::InvalidValue),
523523
};
524-
let amount = Readable::read(reader)?;
524+
let revoked_amount = Readable::read(reader)?;
525525
InputMaterial::Revoked {
526526
script,
527527
pubkey,
528528
key,
529529
is_htlc,
530-
amount
530+
revoked_amount
531531
}
532532
},
533533
1 => {
534534
let script = Readable::read(reader)?;
535535
let key = Readable::read(reader)?;
536536
let preimage = Readable::read(reader)?;
537-
let amount = Readable::read(reader)?;
537+
let remote_amount = Readable::read(reader)?;
538538
let locktime = Readable::read(reader)?;
539539
InputMaterial::RemoteHTLC {
540540
script,
541541
key,
542542
preimage,
543-
amount,
543+
remote_amount,
544544
locktime
545545
}
546546
},
@@ -549,12 +549,12 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
549549
let their_sig = Readable::read(reader)?;
550550
let our_sig = Readable::read(reader)?;
551551
let preimage = Readable::read(reader)?;
552-
let amount = Readable::read(reader)?;
552+
let local_amount = Readable::read(reader)?;
553553
InputMaterial::LocalHTLC {
554554
script,
555555
sigs: (their_sig, our_sig),
556556
preimage,
557-
amount
557+
local_amount
558558
}
559559
}
560560
_ => return Err(DecodeError::InvalidValue),
@@ -1461,7 +1461,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
14611461
// First, process non-htlc outputs (to_local & to_remote)
14621462
for (idx, outp) in tx.output.iter().enumerate() {
14631463
if outp.script_pubkey == revokeable_p2wsh {
1464-
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value };
1464+
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, revoked_amount: outp.value };
14651465
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});
14661466
} else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
14671467
spendable_descriptor = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
@@ -1482,7 +1482,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
14821482
tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
14831483
return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_descriptor); // Corrupted per_commitment_data, fuck this user
14841484
}
1485-
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 };
1485+
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 };
14861486
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
14871487
}
14881488
}
@@ -1646,7 +1646,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
16461646
let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
16471647
let aggregable = if !htlc.offered { false } else { true };
16481648
if preimage.is_some() || !htlc.offered {
1649-
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
1649+
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, remote_amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
16501650
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
16511651
}
16521652
}
@@ -1702,7 +1702,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17021702
let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
17031703

17041704
log_trace!(self, "Remote HTLC broadcast {}:{}", htlc_txid, 0);
1705-
let witness_data = InputMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: tx.output[0].value };
1705+
let witness_data = InputMaterial::Revoked { script: redeemscript, pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, revoked_amount: tx.output[0].value };
17061706
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 });
17071707
let mut claimable_outpoints = HashMap::with_capacity(1);
17081708
claimable_outpoints.insert(htlc_txid, outpoints);
@@ -1752,7 +1752,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17521752

17531753
add_dynamic_output!(htlc_timeout_tx, 0);
17541754
let mut per_input_material = HashMap::with_capacity(1);
1755-
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});
1755+
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});
17561756
//TODO: with option_simplified_commitment track outpoint too
17571757
log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid);
17581758
res.push(htlc_timeout_tx);
@@ -1768,7 +1768,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17681768

17691769
add_dynamic_output!(htlc_success_tx, 0);
17701770
let mut per_input_material = HashMap::with_capacity(1);
1771-
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});
1771+
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});
17721772
//TODO: with option_simplified_commitment track outpoint too
17731773
log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_success_tx.input[0].previous_output.vout, htlc_success_tx.input[0].previous_output.txid);
17741774
res.push(htlc_success_tx);

lightning/src/ln/onchaintx.rs

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

390390
let new_timer = Self::get_height_timer(height, cached_claim_datas.soonest_timelock);
391391
let mut inputs_witnesses_weight = 0;
392-
let mut amt = 0;
392+
let mut total_amount = 0;
393393
for per_outp_material in cached_claim_datas.per_input_material.values() {
394394
match per_outp_material {
395-
&InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
395+
&InputMaterial::Revoked { ref script, ref is_htlc, ref revoked_amount, .. } => {
396396
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 { &[] });
397-
amt += *amount;
397+
total_amount += *revoked_amount;
398398
},
399-
&InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
399+
&InputMaterial::RemoteHTLC { ref preimage, ref remote_amount, .. } => {
400400
inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] });
401-
amt += *amount;
401+
total_amount += *remote_amount;
402402
},
403403
&InputMaterial::LocalHTLC { .. } => { return None; }
404404
}
@@ -408,27 +408,27 @@ impl OnchainTxHandler {
408408
let mut new_feerate;
409409
// If old feerate is 0, first iteration of this claim, use normal fee calculation
410410
if cached_claim_datas.feerate_previous != 0 {
411-
if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
411+
if let Some((new_fee, feerate)) = RBF_bump!(total_amount, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
412412
// If new computed fee is superior at the whole claimable amount burn all in fees
413-
if new_fee > amt {
413+
if new_fee > total_amount {
414414
bumped_tx.output[0].value = 0;
415415
} else {
416-
bumped_tx.output[0].value = amt - new_fee;
416+
bumped_tx.output[0].value = total_amount - new_fee;
417417
}
418418
new_feerate = feerate;
419419
} else { return None; }
420420
} else {
421-
if subtract_high_prio_fee!(self, fee_estimator, amt, predicted_weight, new_feerate) {
422-
bumped_tx.output[0].value = amt;
421+
if subtract_high_prio_fee!(self, fee_estimator, total_amount, predicted_weight, new_feerate) {
422+
bumped_tx.output[0].value = total_amount;
423423
} else { return None; }
424424
}
425425
assert!(new_feerate != 0);
426426

427427
for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
428428
match per_outp_material {
429-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount } => {
429+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount } => {
430430
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
431-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
431+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *revoked_amount)[..]);
432432
let sig = self.secp_ctx.sign(&sighash, &key);
433433
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
434434
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
@@ -440,10 +440,10 @@ impl OnchainTxHandler {
440440
bumped_tx.input[i].witness.push(script.clone().into_bytes());
441441
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);
442442
},
443-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
443+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
444444
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
445445
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
446-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
446+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *remote_amount)[..]);
447447
let sig = self.secp_ctx.sign(&sighash, &key);
448448
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
449449
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);

0 commit comments

Comments
 (0)