Skip to content

Commit 25e2b4b

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 f3bbec3 commit 25e2b4b

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
@@ -392,27 +392,27 @@ pub(crate) enum InputMaterial {
392392
pubkey: Option<PublicKey>,
393393
key: SecretKey,
394394
is_htlc: bool,
395-
amount: u64,
395+
revoked_amount: u64,
396396
},
397397
RemoteHTLC {
398398
script: Script,
399399
key: SecretKey,
400400
preimage: Option<PaymentPreimage>,
401-
amount: u64,
401+
remote_amount: u64,
402402
locktime: u32,
403403
},
404404
LocalHTLC {
405405
script: Script,
406406
sigs: (Signature, Signature),
407407
preimage: Option<PaymentPreimage>,
408-
amount: u64,
408+
local_amount: u64,
409409
}
410410
}
411411

412412
impl Writeable for InputMaterial {
413413
fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
414414
match self {
415-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount} => {
415+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount} => {
416416
writer.write_all(&[0; 1])?;
417417
script.write(writer)?;
418418
pubkey.write(writer)?;
@@ -422,23 +422,23 @@ impl Writeable for InputMaterial {
422422
} else {
423423
writer.write_all(&[1; 1])?;
424424
}
425-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
425+
writer.write_all(&byte_utils::be64_to_array(*revoked_amount))?;
426426
},
427-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
427+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
428428
writer.write_all(&[1; 1])?;
429429
script.write(writer)?;
430430
key.write(writer)?;
431431
preimage.write(writer)?;
432-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
432+
writer.write_all(&byte_utils::be64_to_array(*remote_amount))?;
433433
writer.write_all(&byte_utils::be32_to_array(*locktime))?;
434434
},
435-
&InputMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref amount } => {
435+
&InputMaterial::LocalHTLC { ref script, ref sigs, ref preimage, ref local_amount } => {
436436
writer.write_all(&[2; 1])?;
437437
script.write(writer)?;
438438
sigs.0.write(writer)?;
439439
sigs.1.write(writer)?;
440440
preimage.write(writer)?;
441-
writer.write_all(&byte_utils::be64_to_array(*amount))?;
441+
writer.write_all(&byte_utils::be64_to_array(*local_amount))?;
442442
}
443443
}
444444
Ok(())
@@ -457,26 +457,26 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
457457
1 => false,
458458
_ => return Err(DecodeError::InvalidValue),
459459
};
460-
let amount = Readable::read(reader)?;
460+
let revoked_amount = Readable::read(reader)?;
461461
InputMaterial::Revoked {
462462
script,
463463
pubkey,
464464
key,
465465
is_htlc,
466-
amount
466+
revoked_amount
467467
}
468468
},
469469
1 => {
470470
let script = Readable::read(reader)?;
471471
let key = Readable::read(reader)?;
472472
let preimage = Readable::read(reader)?;
473-
let amount = Readable::read(reader)?;
473+
let remote_amount = Readable::read(reader)?;
474474
let locktime = Readable::read(reader)?;
475475
InputMaterial::RemoteHTLC {
476476
script,
477477
key,
478478
preimage,
479-
amount,
479+
remote_amount,
480480
locktime
481481
}
482482
},
@@ -485,12 +485,12 @@ impl<R: ::std::io::Read> Readable<R> for InputMaterial {
485485
let their_sig = Readable::read(reader)?;
486486
let our_sig = Readable::read(reader)?;
487487
let preimage = Readable::read(reader)?;
488-
let amount = Readable::read(reader)?;
488+
let local_amount = Readable::read(reader)?;
489489
InputMaterial::LocalHTLC {
490490
script,
491491
sigs: (their_sig, our_sig),
492492
preimage,
493-
amount
493+
local_amount
494494
}
495495
}
496496
_ => return Err(DecodeError::InvalidValue),
@@ -1308,7 +1308,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
13081308
// First, process non-htlc outputs (to_local & to_remote)
13091309
for (idx, outp) in tx.output.iter().enumerate() {
13101310
if outp.script_pubkey == revokeable_p2wsh {
1311-
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, amount: outp.value };
1311+
let witness_data = InputMaterial::Revoked { script: revokeable_redeemscript.clone(), pubkey: Some(revocation_pubkey), key: revocation_key, is_htlc: false, revoked_amount: outp.value };
13121312
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});
13131313
} else if Some(&outp.script_pubkey) == local_payment_p2wpkh.as_ref() {
13141314
spendable_descriptor = Some(SpendableOutputDescriptor::DynamicOutputP2WPKH {
@@ -1329,7 +1329,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
13291329
tx.output[transaction_output_index as usize].script_pubkey != expected_script.to_v0_p2wsh() {
13301330
return (claimable_outpoints, (commitment_txid, watch_outputs), spendable_descriptor); // Corrupted per_commitment_data, fuck this user
13311331
}
1332-
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 };
1332+
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 };
13331333
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable: true, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
13341334
}
13351335
}
@@ -1493,7 +1493,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
14931493
let preimage = if htlc.offered { if let Some(p) = self.payment_preimages.get(&htlc.payment_hash) { Some(*p) } else { None } } else { None };
14941494
let aggregable = if !htlc.offered { false } else { true };
14951495
if preimage.is_some() || !htlc.offered {
1496-
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
1496+
let witness_data = InputMaterial::RemoteHTLC { script: expected_script, key: htlc_privkey, preimage, remote_amount: htlc.amount_msat / 1000, locktime: htlc.cltv_expiry };
14971497
outpoints.push(ClaimRequest { absolute_timelock: htlc.cltv_expiry, aggregable, outpoint: BitcoinOutPoint { txid: commitment_txid, vout: transaction_output_index }, witness_data });
14981498
}
14991499
}
@@ -1549,7 +1549,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
15491549
let htlc_txid = tx.txid(); //TODO: This is gonna be a performance bottleneck for watchtowers!
15501550

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

16001600
add_dynamic_output!(htlc_timeout_tx, 0);
16011601
let mut per_input_material = HashMap::with_capacity(1);
1602-
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});
1602+
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});
16031603
//TODO: with option_simplified_commitment track outpoint too
16041604
log_trace!(self, "Outpoint {}:{} is being being claimed", htlc_timeout_tx.input[0].previous_output.vout, htlc_timeout_tx.input[0].previous_output.txid);
16051605
res.push(htlc_timeout_tx);
@@ -1615,7 +1615,7 @@ impl<ChanSigner: ChannelKeys> ChannelMonitor<ChanSigner> {
16151615

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

lightning/src/ln/onchaintx.rs

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

389389
let new_timer = Self::get_height_timer(height, cached_claim_datas.soonest_timelock);
390390
let mut inputs_witnesses_weight = 0;
391-
let mut amt = 0;
391+
let mut total_amount = 0;
392392
for per_outp_material in cached_claim_datas.per_input_material.values() {
393393
match per_outp_material {
394-
&InputMaterial::Revoked { ref script, ref is_htlc, ref amount, .. } => {
394+
&InputMaterial::Revoked { ref script, ref is_htlc, ref revoked_amount, .. } => {
395395
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 { &[] });
396-
amt += *amount;
396+
total_amount += *revoked_amount;
397397
},
398-
&InputMaterial::RemoteHTLC { ref preimage, ref amount, .. } => {
398+
&InputMaterial::RemoteHTLC { ref preimage, ref remote_amount, .. } => {
399399
inputs_witnesses_weight += Self::get_witnesses_weight(if preimage.is_some() { &[InputDescriptors::OfferedHTLC] } else { &[InputDescriptors::ReceivedHTLC] });
400-
amt += *amount;
400+
total_amount += *remote_amount;
401401
},
402402
&InputMaterial::LocalHTLC { .. } => { return None; }
403403
}
@@ -407,27 +407,27 @@ impl OnchainTxHandler {
407407
let mut new_feerate;
408408
// If old feerate is 0, first iteration of this claim, use normal fee calculation
409409
if cached_claim_datas.feerate_previous != 0 {
410-
if let Some((new_fee, feerate)) = RBF_bump!(amt, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
410+
if let Some((new_fee, feerate)) = RBF_bump!(total_amount, cached_claim_datas.feerate_previous, fee_estimator, predicted_weight as u64) {
411411
// If new computed fee is superior at the whole claimable amount burn all in fees
412-
if new_fee > amt {
412+
if new_fee > total_amount {
413413
bumped_tx.output[0].value = 0;
414414
} else {
415-
bumped_tx.output[0].value = amt - new_fee;
415+
bumped_tx.output[0].value = total_amount - new_fee;
416416
}
417417
new_feerate = feerate;
418418
} else { return None; }
419419
} else {
420-
if subtract_high_prio_fee!(self, fee_estimator, amt, predicted_weight, new_feerate) {
421-
bumped_tx.output[0].value = amt;
420+
if subtract_high_prio_fee!(self, fee_estimator, total_amount, predicted_weight, new_feerate) {
421+
bumped_tx.output[0].value = total_amount;
422422
} else { return None; }
423423
}
424424
assert!(new_feerate != 0);
425425

426426
for (i, (outp, per_outp_material)) in cached_claim_datas.per_input_material.iter().enumerate() {
427427
match per_outp_material {
428-
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref amount } => {
428+
&InputMaterial::Revoked { ref script, ref pubkey, ref key, ref is_htlc, ref revoked_amount } => {
429429
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
430-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
430+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *revoked_amount)[..]);
431431
let sig = self.secp_ctx.sign(&sighash, &key);
432432
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
433433
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
@@ -439,10 +439,10 @@ impl OnchainTxHandler {
439439
bumped_tx.input[i].witness.push(script.clone().into_bytes());
440440
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);
441441
},
442-
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref amount, ref locktime } => {
442+
&InputMaterial::RemoteHTLC { ref script, ref key, ref preimage, ref remote_amount, ref locktime } => {
443443
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
444444
let sighash_parts = bip143::SighashComponents::new(&bumped_tx);
445-
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *amount)[..]);
445+
let sighash = hash_to_message!(&sighash_parts.sighash_all(&bumped_tx.input[i], &script, *remote_amount)[..]);
446446
let sig = self.secp_ctx.sign(&sighash, &key);
447447
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
448448
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);

0 commit comments

Comments
 (0)