Skip to content

Commit 0c86bbf

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 bfdd9d1 commit 0c86bbf

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),
@@ -1463,7 +1463,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
14631463
// First, process non-htlc outputs (to_local & to_remote)
14641464
for (idx, outp) in tx.output.iter().enumerate() {
14651465
if outp.script_pubkey == revokeable_p2wsh {
1466-
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value };
1466+
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, revoked_amount: outp.value };
14671467
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});
14681468
} else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
14691469
spendable_descriptor = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
@@ -1484,7 +1484,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
14841484
tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
14851485
return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_descriptor); // Corrupted per_commitment_data, fuck this user
14861486
}
1487-
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 };
1487+
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 };
14881488
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
14891489
}
14901490
}
@@ -1648,7 +1648,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
16481648
let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
16491649
let aggregable = if !htlc.offered { false } else { true };
16501650
if preimage.is_some() || !htlc.offered {
1651-
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
1651+
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, remote_amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
16521652
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
16531653
}
16541654
}
@@ -1704,7 +1704,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
17041704
let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
17051705

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

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

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