Skip to content

Commit

Permalink
Merge pull request lightningdevkit#2753 from TheBlueMatt/2023-11-inbo…
Browse files Browse the repository at this point in the history
…und-preimages

Provide inbound HTLC preimages to the `EcdsaChannelSigner`
  • Loading branch information
wpaulino authored Dec 4, 2023
2 parents 7f96446 + 262072d commit 12c2086
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 30 deletions.
30 changes: 20 additions & 10 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,8 @@ struct CommitmentStats<'a> {
htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
local_balance_msat: u64, // local balance before fees but considering dust limits
remote_balance_msat: u64, // remote balance before fees but considering dust limits
preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
}

/// Used when calculating whether we or the remote can afford an additional HTLC.
Expand Down Expand Up @@ -1428,6 +1429,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
}
}

let mut inbound_htlc_preimages: Vec<PaymentPreimage> = Vec::new();

for ref htlc in self.pending_inbound_htlcs.iter() {
let (include, state_name) = match htlc.state {
InboundHTLCState::RemoteAnnounced(_) => (!generated_by_local, "RemoteAnnounced"),
Expand All @@ -1445,7 +1448,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
match &htlc.state {
&InboundHTLCState::LocalRemoved(ref reason) => {
if generated_by_local {
if let &InboundHTLCRemovalReason::Fulfill(_) = reason {
if let &InboundHTLCRemovalReason::Fulfill(preimage) = reason {
inbound_htlc_preimages.push(preimage);
value_to_self_msat_offset += htlc.amount_msat as i64;
}
}
Expand All @@ -1455,7 +1459,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
}
}

let mut preimages: Vec<PaymentPreimage> = Vec::new();

let mut outbound_htlc_preimages: Vec<PaymentPreimage> = Vec::new();

for ref htlc in self.pending_outbound_htlcs.iter() {
let (include, state_name) = match htlc.state {
Expand All @@ -1474,7 +1479,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
};

if let Some(preimage) = preimage_opt {
preimages.push(preimage);
outbound_htlc_preimages.push(preimage);
}

if include {
Expand Down Expand Up @@ -1580,7 +1585,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
htlcs_included,
local_balance_msat: value_to_self_msat as u64,
remote_balance_msat: value_to_remote_msat as u64,
preimages
inbound_htlc_preimages,
outbound_htlc_preimages,
}
}

Expand Down Expand Up @@ -2178,7 +2184,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
let signature = match &self.holder_signer {
// TODO (taproot|arik): move match into calling method for Taproot
ChannelSignerType::Ecdsa(ecdsa) => {
ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), Vec::new(), &self.secp_ctx)
.map(|(sig, _)| sig).ok()?
},
// TODO (taproot|arik)
Expand Down Expand Up @@ -2216,7 +2222,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
match &self.holder_signer {
// TODO (arik): move match into calling method for Taproot
ChannelSignerType::Ecdsa(ecdsa) => {
let funding_signed = ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), &self.secp_ctx)
let funding_signed = ecdsa.sign_counterparty_commitment(&counterparty_initial_commitment_tx, Vec::new(), Vec::new(), &self.secp_ctx)
.map(|(signature, _)| msgs::FundingSigned {
channel_id: self.channel_id(),
signature,
Expand Down Expand Up @@ -3247,7 +3253,7 @@ impl<SP: Deref> Channel<SP> where
self.context.counterparty_funding_pubkey()
);

self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.preimages)
self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.outbound_htlc_preimages)
.map_err(|_| ChannelError::Close("Failed to validate our commitment".to_owned()))?;

// Update state now that we've passed all the can-fail calls...
Expand Down Expand Up @@ -5780,8 +5786,12 @@ impl<SP: Deref> Channel<SP> where
htlcs.push(htlc);
}

let res = ecdsa.sign_counterparty_commitment(&commitment_stats.tx, commitment_stats.preimages, &self.context.secp_ctx)
.map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?;
let res = ecdsa.sign_counterparty_commitment(
&commitment_stats.tx,
commitment_stats.inbound_htlc_preimages,
commitment_stats.outbound_htlc_preimages,
&self.context.secp_ctx,
).map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?;
signature = res.0;
htlc_signatures = res.1;

Expand Down
4 changes: 2 additions & 2 deletions lightning/src/ln/functional_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ fn test_update_fee_that_funder_cannot_afford() {
&mut htlcs,
&local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable()
);
local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap()
local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), Vec::new(), &secp_ctx).unwrap()
};

let commit_signed_msg = msgs::CommitmentSigned {
Expand Down Expand Up @@ -1494,7 +1494,7 @@ fn test_fee_spike_violation_fails_htlc() {
&mut vec![(accepted_htlc_info, ())],
&local_chan.context.channel_transaction_parameters.as_counterparty_broadcastable()
);
local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), &secp_ctx).unwrap()
local_chan_signer.as_ecdsa().unwrap().sign_counterparty_commitment(&commitment_tx, Vec::new(), Vec::new(), &secp_ctx).unwrap()
};

let commit_signed_msg = msgs::CommitmentSigned {
Expand Down
10 changes: 6 additions & 4 deletions lightning/src/sign/ecdsa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,18 @@ pub trait EcdsaChannelSigner: ChannelSigner {
/// Policy checks should be implemented in this function, including checking the amount
/// sent to us and checking the HTLCs.
///
/// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
/// A validating signer should ensure that an HTLC output is removed only when the matching
/// preimage is provided, or when the value to holder is restored.
/// The preimages of outbound and inbound HTLCs that were fulfilled since the last commitment
/// are provided. A validating signer should ensure that an outbound HTLC output is removed
/// only when the matching preimage is provided and after the corresponding inbound HTLC has
/// been removed for forwarded payments.
///
/// Note that all the relevant preimages will be provided, but there may also be additional
/// irrelevant or duplicate preimages.
//
// TODO: Document the things someone using this interface should enforce before signing.
fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction,
preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>
inbound_htlc_preimages: Vec<PaymentPreimage>,
outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<(Signature, Vec<Signature>), ()>;
/// Creates a signature for a holder's commitment transaction.
///
Expand Down
10 changes: 5 additions & 5 deletions lightning/src/sign/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,14 +594,14 @@ pub trait ChannelSigner {
/// Policy checks should be implemented in this function, including checking the amount
/// sent to us and checking the HTLCs.
///
/// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
/// The preimages of outbound HTLCs that were fulfilled since the last commitment are provided.
/// A validating signer should ensure that an HTLC output is removed only when the matching
/// preimage is provided, or when the value to holder is restored.
///
/// Note that all the relevant preimages will be provided, but there may also be additional
/// irrelevant or duplicate preimages.
fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction,
preimages: Vec<PaymentPreimage>) -> Result<(), ()>;
outbound_htlc_preimages: Vec<PaymentPreimage>) -> Result<(), ()>;

/// Validate the counterparty's revocation.
///
Expand Down Expand Up @@ -1086,7 +1086,7 @@ impl ChannelSigner for InMemorySigner {
chan_utils::build_commitment_secret(&self.commitment_seed, idx)
}

fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction, _preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
fn validate_holder_commitment(&self, _holder_tx: &HolderCommitmentTransaction, _outbound_htlc_preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
Ok(())
}

Expand All @@ -1112,7 +1112,7 @@ impl ChannelSigner for InMemorySigner {
const MISSING_PARAMS_ERR: &'static str = "ChannelSigner::provide_channel_parameters must be called before signing operations";

impl EcdsaChannelSigner for InMemorySigner {
fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, _preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, _inbound_htlc_preimages: Vec<PaymentPreimage>, _outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
let trusted_tx = commitment_tx.trust();
let keys = trusted_tx.keys();

Expand Down Expand Up @@ -1260,7 +1260,7 @@ impl TaprootChannelSigner for InMemorySigner {
todo!()
}

fn partially_sign_counterparty_commitment(&self, counterparty_nonce: PublicNonce, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<All>) -> Result<(PartialSignatureWithNonce, Vec<schnorr::Signature>), ()> {
fn partially_sign_counterparty_commitment(&self, counterparty_nonce: PublicNonce, commitment_tx: &CommitmentTransaction, inbound_htlc_preimages: Vec<PaymentPreimage>, outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<All>) -> Result<(PartialSignatureWithNonce, Vec<schnorr::Signature>), ()> {
todo!()
}

Expand Down
12 changes: 7 additions & 5 deletions lightning/src/sign/taproot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,19 @@ pub trait TaprootChannelSigner: ChannelSigner {
/// Policy checks should be implemented in this function, including checking the amount
/// sent to us and checking the HTLCs.
///
/// The preimages of outgoing HTLCs that were fulfilled since the last commitment are provided.
/// A validating signer should ensure that an HTLC output is removed only when the matching
/// preimage is provided, or when the value to holder is restored.
/// The preimages of outbound and inbound HTLCs that were fulfilled since the last commitment
/// are provided. A validating signer should ensure that an outbound HTLC output is removed
/// only when the matching preimage is provided and after the corresponding inbound HTLC has
/// been removed for forwarded payments.
///
/// Note that all the relevant preimages will be provided, but there may also be additional
/// irrelevant or duplicate preimages.
//
// TODO: Document the things someone using this interface should enforce before signing.
fn partially_sign_counterparty_commitment(&self, counterparty_nonce: PublicNonce,
commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>,
secp_ctx: &Secp256k1<secp256k1::All>,
commitment_tx: &CommitmentTransaction,
inbound_htlc_preimages: Vec<PaymentPreimage>,
outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>,
) -> Result<(PartialSignatureWithNonce, Vec<Signature>), ()>;

/// Creates a signature for a holder's commitment transaction.
Expand Down
8 changes: 4 additions & 4 deletions lightning/src/util/test_channel_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ impl ChannelSigner for TestChannelSigner {
self.inner.release_commitment_secret(idx)
}

fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, _preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
fn validate_holder_commitment(&self, holder_tx: &HolderCommitmentTransaction, _outbound_htlc_preimages: Vec<PaymentPreimage>) -> Result<(), ()> {
let mut state = self.state.lock().unwrap();
let idx = holder_tx.commitment_number();
assert!(idx == state.last_holder_commitment || idx == state.last_holder_commitment - 1, "expecting to validate the current or next holder commitment - trying {}, current {}", idx, state.last_holder_commitment);
Expand Down Expand Up @@ -166,7 +166,7 @@ impl ChannelSigner for TestChannelSigner {
}

impl EcdsaChannelSigner for TestChannelSigner {
fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
fn sign_counterparty_commitment(&self, commitment_tx: &CommitmentTransaction, inbound_htlc_preimages: Vec<PaymentPreimage>, outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<(Signature, Vec<Signature>), ()> {
self.verify_counterparty_commitment_tx(commitment_tx, secp_ctx);

{
Expand All @@ -185,7 +185,7 @@ impl EcdsaChannelSigner for TestChannelSigner {
state.last_counterparty_commitment = cmp::min(last_commitment_number, actual_commitment_number)
}

Ok(self.inner.sign_counterparty_commitment(commitment_tx, preimages, secp_ctx).unwrap())
Ok(self.inner.sign_counterparty_commitment(commitment_tx, inbound_htlc_preimages, outbound_htlc_preimages, secp_ctx).unwrap())
}

fn sign_holder_commitment(&self, commitment_tx: &HolderCommitmentTransaction, secp_ctx: &Secp256k1<secp256k1::All>) -> Result<Signature, ()> {
Expand Down Expand Up @@ -288,7 +288,7 @@ impl TaprootChannelSigner for TestChannelSigner {
todo!()
}

fn partially_sign_counterparty_commitment(&self, counterparty_nonce: PublicNonce, commitment_tx: &CommitmentTransaction, preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<All>) -> Result<(PartialSignatureWithNonce, Vec<secp256k1::schnorr::Signature>), ()> {
fn partially_sign_counterparty_commitment(&self, counterparty_nonce: PublicNonce, commitment_tx: &CommitmentTransaction, inbound_htlc_preimages: Vec<PaymentPreimage>, outbound_htlc_preimages: Vec<PaymentPreimage>, secp_ctx: &Secp256k1<All>) -> Result<(PartialSignatureWithNonce, Vec<secp256k1::schnorr::Signature>), ()> {
todo!()
}

Expand Down

0 comments on commit 12c2086

Please sign in to comment.