Skip to content

Commit ceede42

Browse files
jkczyzjkczyz
committed
Check pending funding when validating update_add_htlc
If there are any pending splices when an update_add_htlc message is received, it must be validated against each pending FundingScope. Otherwise, the HTLC could be invalid once the splice is locked.
1 parent 5688166 commit ceede42

File tree

1 file changed

+49
-34
lines changed

1 file changed

+49
-34
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 49 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5749,34 +5749,19 @@ impl<SP: Deref> FundedChannel<SP> where
57495749
Ok(self.get_announcement_sigs(node_signer, chain_hash, user_config, best_block.height, logger))
57505750
}
57515751

5752-
pub fn update_add_htlc<F: Deref>(
5753-
&mut self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator<F>,
5754-
) -> Result<(), ChannelError> where F::Target: FeeEstimator {
5755-
if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() {
5756-
return Err(ChannelError::WarnAndDisconnect("Got add HTLC message while quiescent".to_owned()));
5757-
}
5758-
if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
5759-
return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned()));
5760-
}
5761-
// If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
5762-
if self.context.channel_state.is_remote_shutdown_sent() {
5763-
return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned()));
5764-
}
5765-
if self.context.channel_state.is_peer_disconnected() {
5766-
return Err(ChannelError::close("Peer sent update_add_htlc when we needed a channel_reestablish".to_owned()));
5767-
}
5768-
if msg.amount_msat > self.funding.get_value_satoshis() * 1000 {
5752+
fn validate_update_add_htlc<F: Deref>(
5753+
&self, funding: &FundingScope, msg: &msgs::UpdateAddHTLC,
5754+
fee_estimator: &LowerBoundedFeeEstimator<F>,
5755+
) -> Result<(), ChannelError>
5756+
where
5757+
F::Target: FeeEstimator,
5758+
{
5759+
if msg.amount_msat > funding.get_value_satoshis() * 1000 {
57695760
return Err(ChannelError::close("Remote side tried to send more than the total value of the channel".to_owned()));
57705761
}
5771-
if msg.amount_msat == 0 {
5772-
return Err(ChannelError::close("Remote side tried to send a 0-msat HTLC".to_owned()));
5773-
}
5774-
if msg.amount_msat < self.context.holder_htlc_minimum_msat {
5775-
return Err(ChannelError::close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.context.holder_htlc_minimum_msat, msg.amount_msat)));
5776-
}
57775762

57785763
let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
5779-
let htlc_stats = self.context.get_pending_htlc_stats(&self.funding, None, dust_exposure_limiting_feerate);
5764+
let htlc_stats = self.context.get_pending_htlc_stats(funding, None, dust_exposure_limiting_feerate);
57805765
if htlc_stats.pending_inbound_htlcs + 1 > self.context.holder_max_accepted_htlcs as usize {
57815766
return Err(ChannelError::close(format!("Remote tried to push more than our max accepted HTLCs ({})", self.context.holder_max_accepted_htlcs)));
57825767
}
@@ -5806,53 +5791,83 @@ impl<SP: Deref> FundedChannel<SP> where
58065791
}
58075792

58085793
let pending_value_to_self_msat =
5809-
self.funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat;
5794+
funding.value_to_self_msat + htlc_stats.pending_inbound_htlcs_value_msat - removed_outbound_total_msat;
58105795
let pending_remote_value_msat =
5811-
self.funding.get_value_satoshis() * 1000 - pending_value_to_self_msat;
5796+
funding.get_value_satoshis() * 1000 - pending_value_to_self_msat;
58125797
if pending_remote_value_msat < msg.amount_msat {
58135798
return Err(ChannelError::close("Remote HTLC add would overdraw remaining funds".to_owned()));
58145799
}
58155800

58165801
// Check that the remote can afford to pay for this HTLC on-chain at the current
58175802
// feerate_per_kw, while maintaining their channel reserve (as required by the spec).
58185803
{
5819-
let remote_commit_tx_fee_msat = if self.funding.is_outbound() { 0 } else {
5804+
let remote_commit_tx_fee_msat = if funding.is_outbound() { 0 } else {
58205805
let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
5821-
self.context.next_remote_commit_tx_fee_msat(&self.funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations
5806+
self.context.next_remote_commit_tx_fee_msat(funding, Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations
58225807
};
5823-
let anchor_outputs_value_msat = if !self.funding.is_outbound() && self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
5808+
let anchor_outputs_value_msat = if !funding.is_outbound() && funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
58245809
ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
58255810
} else {
58265811
0
58275812
};
58285813
if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(anchor_outputs_value_msat) < remote_commit_tx_fee_msat {
58295814
return Err(ChannelError::close("Remote HTLC add would not leave enough to pay for fees".to_owned()));
58305815
};
5831-
if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < self.funding.holder_selected_channel_reserve_satoshis * 1000 {
5816+
if pending_remote_value_msat.saturating_sub(msg.amount_msat).saturating_sub(remote_commit_tx_fee_msat).saturating_sub(anchor_outputs_value_msat) < funding.holder_selected_channel_reserve_satoshis * 1000 {
58325817
return Err(ChannelError::close("Remote HTLC add would put them under remote reserve value".to_owned()));
58335818
}
58345819
}
58355820

5836-
let anchor_outputs_value_msat = if self.funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
5821+
let anchor_outputs_value_msat = if funding.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
58375822
ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
58385823
} else {
58395824
0
58405825
};
5841-
if self.funding.is_outbound() {
5826+
if funding.is_outbound() {
58425827
// Check that they won't violate our local required channel reserve by adding this HTLC.
58435828
let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
5844-
let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(&self.funding, htlc_candidate, None);
5845-
if self.funding.value_to_self_msat < self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat {
5829+
let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(funding, htlc_candidate, None);
5830+
if funding.value_to_self_msat < funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat {
58465831
return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
58475832
}
58485833
}
5834+
5835+
Ok(())
5836+
}
5837+
5838+
pub fn update_add_htlc<F: Deref>(
5839+
&mut self, msg: &msgs::UpdateAddHTLC, fee_estimator: &LowerBoundedFeeEstimator<F>,
5840+
) -> Result<(), ChannelError> where F::Target: FeeEstimator {
5841+
if self.context.channel_state.is_remote_stfu_sent() || self.context.channel_state.is_quiescent() {
5842+
return Err(ChannelError::WarnAndDisconnect("Got add HTLC message while quiescent".to_owned()));
5843+
}
5844+
if !matches!(self.context.channel_state, ChannelState::ChannelReady(_)) {
5845+
return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned()));
5846+
}
5847+
// If the remote has sent a shutdown prior to adding this HTLC, then they are in violation of the spec.
5848+
if self.context.channel_state.is_remote_shutdown_sent() {
5849+
return Err(ChannelError::close("Got add HTLC message when channel was not in an operational state".to_owned()));
5850+
}
5851+
if self.context.channel_state.is_peer_disconnected() {
5852+
return Err(ChannelError::close("Peer sent update_add_htlc when we needed a channel_reestablish".to_owned()));
5853+
}
5854+
if msg.amount_msat == 0 {
5855+
return Err(ChannelError::close("Remote side tried to send a 0-msat HTLC".to_owned()));
5856+
}
5857+
if msg.amount_msat < self.context.holder_htlc_minimum_msat {
5858+
return Err(ChannelError::close(format!("Remote side tried to send less than our minimum HTLC value. Lower limit: ({}). Actual: ({})", self.context.holder_htlc_minimum_msat, msg.amount_msat)));
5859+
}
58495860
if self.context.next_counterparty_htlc_id != msg.htlc_id {
58505861
return Err(ChannelError::close(format!("Remote skipped HTLC ID (skipped ID: {})", self.context.next_counterparty_htlc_id)));
58515862
}
58525863
if msg.cltv_expiry >= 500000000 {
58535864
return Err(ChannelError::close("Remote provided CLTV expiry in seconds instead of block height".to_owned()));
58545865
}
58555866

5867+
core::iter::once(&self.funding)
5868+
.chain(self.pending_funding.iter())
5869+
.try_for_each(|funding| self.validate_update_add_htlc(funding, msg, fee_estimator))?;
5870+
58565871
// Now update local state:
58575872
self.context.next_counterparty_htlc_id += 1;
58585873
self.context.pending_inbound_htlcs.push(InboundHTLCOutput {

0 commit comments

Comments
 (0)