Skip to content

Commit fc33132

Browse files
committed
Panic if signing fails in OnChainTx
Signatures in OnChainTx must not fail, or we stand to lose funds
1 parent 9398b78 commit fc33132

File tree

1 file changed

+26
-38
lines changed

1 file changed

+26
-38
lines changed

lightning/src/ln/onchaintx.rs

Lines changed: 26 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -594,17 +594,15 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
594594
chan_utils::get_revokeable_redeemscript(&chan_keys.revocation_key, *on_counterparty_tx_csv, &chan_keys.broadcaster_delayed_payment_key)
595595
};
596596

597-
if let Ok(sig) = self.key_storage.sign_justice_transaction(&bumped_tx, i, *amount, &per_commitment_key, htlc, &self.secp_ctx) {
598-
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
599-
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
600-
if htlc.is_some() {
601-
bumped_tx.input[i].witness.push(chan_keys.revocation_key.clone().serialize().to_vec());
602-
} else {
603-
bumped_tx.input[i].witness.push(vec!(1));
604-
}
605-
bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
606-
} else { return None; }
607-
//TODO: panic ?
597+
let sig= self.key_storage.sign_justice_transaction(&bumped_tx, i, *amount, &per_commitment_key, htlc, &self.secp_ctx).expect("sign justice tx");
598+
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
599+
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
600+
if htlc.is_some() {
601+
bumped_tx.input[i].witness.push(chan_keys.revocation_key.clone().serialize().to_vec());
602+
} else {
603+
bumped_tx.input[i].witness.push(vec!(1));
604+
}
605+
bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
608606

609607
log_trace!(logger, "Going to broadcast Penalty Transaction {} claiming revoked {} output {} from {} with new feerate {}...", bumped_tx.txid(), if *input_descriptor == InputDescriptors::RevokedOutput { "to_holder" } else if *input_descriptor == InputDescriptors::RevokedOfferedHTLC { "offered" } else if *input_descriptor == InputDescriptors::RevokedReceivedHTLC { "received" } else { "" }, outp.vout, outp.txid, new_feerate);
610608
}
@@ -614,17 +612,16 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
614612
let witness_script = chan_utils::get_htlc_redeemscript_with_explicit_keys(&htlc, &chan_keys.broadcaster_htlc_key, &chan_keys.countersignatory_htlc_key, &chan_keys.revocation_key);
615613

616614
if !preimage.is_some() { bumped_tx.lock_time = htlc.cltv_expiry }; // Right now we don't aggregate time-locked transaction, if we do we should set lock_time before to avoid breaking hash computation
617-
if let Ok(sig) = self.key_storage.sign_counterparty_htlc_transaction(&bumped_tx, i, &htlc.amount_msat / 1000, &per_commitment_point, htlc, &self.secp_ctx) {
618-
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
619-
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
620-
if let &Some(preimage) = preimage {
621-
bumped_tx.input[i].witness.push(preimage.0.to_vec());
622-
} else {
623-
// Due to BIP146 (MINIMALIF) this must be a zero-length element to relay.
624-
bumped_tx.input[i].witness.push(vec![]);
625-
}
626-
bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
615+
let sig = self.key_storage.sign_counterparty_htlc_transaction(&bumped_tx, i, &htlc.amount_msat / 1000, &per_commitment_point, htlc, &self.secp_ctx).expect("sign counterparty HTLC tx");
616+
bumped_tx.input[i].witness.push(sig.serialize_der().to_vec());
617+
bumped_tx.input[i].witness[0].push(SigHashType::All as u8);
618+
if let &Some(preimage) = preimage {
619+
bumped_tx.input[i].witness.push(preimage.0.to_vec());
620+
} else {
621+
// Due to BIP146 (MINIMALIF) this must be a zero-length element to relay.
622+
bumped_tx.input[i].witness.push(vec![]);
627623
}
624+
bumped_tx.input[i].witness.push(witness_script.clone().into_bytes());
628625
log_trace!(logger, "Going to broadcast Claim Transaction {} claiming counterparty {} htlc output {} from {} with new feerate {}...", bumped_tx.txid(), if preimage.is_some() { "offered" } else { "received" }, outp.vout, outp.txid, new_feerate);
629626
}
630627
},
@@ -896,9 +893,8 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
896893

897894
fn sign_prev_holder_htlcs(&mut self) {
898895
if let Some(ref holder_commitment) = self.prev_holder_commitment {
899-
if let Ok((_sig, sigs)) = self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
900-
self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
901-
}
896+
let (_sig, sigs) = self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx).expect("sign holder commitment");
897+
self.prev_holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, sigs));
902898
}
903899
}
904900

@@ -918,13 +914,9 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
918914
// to monitor before.
919915
pub(crate) fn get_fully_signed_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
920916
if let Some(ref mut holder_commitment) = self.holder_commitment {
921-
match self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
922-
Ok((sig, htlc_sigs)) => {
923-
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, htlc_sigs));
924-
Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
925-
},
926-
Err(_) => return None,
927-
}
917+
let (sig, htlc_sigs) = self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx).expect("signing holder commitment");
918+
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, htlc_sigs));
919+
Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
928920
} else {
929921
None
930922
}
@@ -933,13 +925,9 @@ impl<ChanSigner: ChannelKeys> OnchainTxHandler<ChanSigner> {
933925
#[cfg(any(test, feature="unsafe_revoked_tx_signing"))]
934926
pub(crate) fn get_fully_signed_copy_holder_tx(&mut self, funding_redeemscript: &Script) -> Option<Transaction> {
935927
if let Some(ref mut holder_commitment) = self.holder_commitment {
936-
match self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx) {
937-
Ok((sig, htlc_sigs)) => {
938-
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, htlc_sigs));
939-
Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
940-
},
941-
Err(_) => return None,
942-
}
928+
let (sig, htlc_sigs) = self.key_storage.sign_holder_commitment(holder_commitment, &self.secp_ctx).expect("sign holder commitment");
929+
self.holder_htlc_sigs = Some(Self::extract_holder_sigs(holder_commitment, htlc_sigs));
930+
Some(holder_commitment.add_holder_sig(funding_redeemscript, sig))
943931
} else {
944932
None
945933
}

0 commit comments

Comments
 (0)