Skip to content

Commit 2370a34

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 b3652f7 commit 2370a34

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

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

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

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

‎lightning/src/ln/onchaintx.rs‎

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

391391
let new_timer = Self::get_height_timer(height, cached_claim_datas.soonest_timelock);
392392
let mut inputs_witnesses_weight = 0;
393-
let mut amt = 0;
393+
let mut total_amount = 0;
394394
for per_outp_material in cached_claim_datas.per_input_material.values() {
395395
match per_outp_material {
396-
&InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
396+
&InputMaterial::Revoked { ref script, ref is_htlc, ref revoked_amount, .. } => {
397397
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 { &[] });
398-
amt += *amount;
398+
total_amount += *revoked_amount;
399399
},
400-
&InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
400+
&InputMaterial::RemoteHTLC { ref preimage, ref remote_amount, .. } => {
401401
inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] });
402-
amt += *amount;
402+
total_amount += *remote_amount;
403403
},
404404
&InputMaterial::LocalHTLC { .. } => { return None; }
405405
}
@@ -409,27 +409,27 @@ impl OnchainTxHandler {
409409
let mut new_feerate;
410410
// If old feerate is 0, first iteration of this claim, use normal fee calculation
411411
if cached_claim_datas.feerate_previous != 0 {
412-
if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
412+
if let Some((new_fee, feerate)) = RBF_bump!(total_amount, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
413413
// If new computed fee is superior at the whole claimable amount burn all in fees
414-
if new_fee > amt {
414+
if new_fee > total_amount {
415415
bumped_tx.output[0].value = 0;
416416
} else {
417-
bumped_tx.output[0].value = amt - new_fee;
417+
bumped_tx.output[0].value = total_amount - new_fee;
418418
}
419419
new_feerate = feerate;
420420
} else { return None; }
421421
} else {
422-
if subtract_high_prio_fee!(self, fee_estimator, amt, predicted_weight, new_feerate) {
423-
bumped_tx.output[0].value = amt;
422+
if subtract_high_prio_fee!(self, fee_estimator, total_amount, predicted_weight, new_feerate) {
423+
bumped_tx.output[0].value = total_amount;
424424
} else { return None; }
425425
}
426426
assert!(new_feerate != 0);
427427

428428
for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
429429
match per_outp_material {
430-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount } => {
430+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount } => {
431431
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
432-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
432+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *revoked_amount)[..]);
433433
let sig = self.secp_ctx.sign(&sighash, &key);
434434
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
435435
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
@@ -441,10 +441,10 @@ impl OnchainTxHandler {
441441
bumped_tx.input[i].witness.push(script.clone().into_bytes());
442442
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);
443443
},
444-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
444+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
445445
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
446446
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
447-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
447+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *remote_amount)[..]);
448448
let sig = self.secp_ctx.sign(&sighash, &key);
449449
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
450450
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);

0 commit comments

Comments
 (0)