Skip to content

Commit 801b58d

Browse files
jkczyzjkczyz
committed
Revert "Move ChannelContext::minimum_depth to FundingScope"
This reverts commit 4b4ce81.
1 parent 4b4ce81 commit 801b58d

File tree

3 files changed

+30
-45
lines changed

3 files changed

+30
-45
lines changed

‎lightning/src/ln/channel.rs

Lines changed: 25 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1689,9 +1689,6 @@ pub(super) struct FundingScope {
16891689
funding_tx_confirmed_in: Option<BlockHash>,
16901690
funding_tx_confirmation_height: u32,
16911691
short_channel_id: Option<u64>,
1692-
1693-
/// The number of confirmation required before sending `channel_ready` or `splice_locked`.
1694-
minimum_depth: Option<u32>,
16951692
}
16961693

16971694
impl Writeable for FundingScope {
@@ -1705,7 +1702,6 @@ impl Writeable for FundingScope {
17051702
(11, self.funding_tx_confirmed_in, option),
17061703
(13, self.funding_tx_confirmation_height, required),
17071704
(15, self.short_channel_id, option),
1708-
(17, self.minimum_depth, option),
17091705
});
17101706
Ok(())
17111707
}
@@ -1721,7 +1717,6 @@ impl Readable for FundingScope {
17211717
let mut funding_tx_confirmed_in = None;
17221718
let mut funding_tx_confirmation_height = RequiredWrapper(None);
17231719
let mut short_channel_id = None;
1724-
let mut minimum_depth = None;
17251720

17261721
read_tlv_fields!(reader, {
17271722
(1, value_to_self_msat, required),
@@ -1732,7 +1727,6 @@ impl Readable for FundingScope {
17321727
(11, funding_tx_confirmed_in, option),
17331728
(13, funding_tx_confirmation_height, required),
17341729
(15, short_channel_id, option),
1735-
(17, minimum_depth, option),
17361730
});
17371731

17381732
Ok(Self {
@@ -1748,7 +1742,6 @@ impl Readable for FundingScope {
17481742
funding_tx_confirmed_in,
17491743
funding_tx_confirmation_height: funding_tx_confirmation_height.0.unwrap(),
17501744
short_channel_id,
1751-
minimum_depth,
17521745
#[cfg(any(test, fuzzing))]
17531746
next_local_commitment_tx_fee_info_cached: Mutex::new(None),
17541747
#[cfg(any(test, fuzzing))]
@@ -1854,10 +1847,6 @@ impl FundingScope {
18541847
pub fn get_short_channel_id(&self) -> Option<u64> {
18551848
self.short_channel_id
18561849
}
1857-
1858-
pub fn minimum_depth(&self) -> Option<u32> {
1859-
self.minimum_depth
1860-
}
18611850
}
18621851

18631852
/// Info about a pending splice, used in the pre-splice channel
@@ -2046,7 +2035,7 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
20462035
#[cfg(not(any(test, feature="_test_utils")))]
20472036
counterparty_max_accepted_htlcs: u16,
20482037
holder_max_accepted_htlcs: u16,
2049-
negotiated_minimum_depth: Option<u32>,
2038+
minimum_depth: Option<u32>,
20502039

20512040
counterparty_forwarding_info: Option<CounterpartyForwardingInfo>,
20522041

@@ -2828,7 +2817,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
28282817
funding_tx_confirmed_in: None,
28292818
funding_tx_confirmation_height: 0,
28302819
short_channel_id: None,
2831-
minimum_depth,
28322820
};
28332821
let channel_context = ChannelContext {
28342822
user_id,
@@ -2903,7 +2891,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
29032891
holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat },
29042892
counterparty_max_accepted_htlcs: open_channel_fields.max_accepted_htlcs,
29052893
holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)),
2906-
negotiated_minimum_depth: minimum_depth,
2894+
minimum_depth,
29072895

29082896
counterparty_forwarding_info: None,
29092897

@@ -3065,7 +3053,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
30653053
funding_tx_confirmed_in: None,
30663054
funding_tx_confirmation_height: 0,
30673055
short_channel_id: None,
3068-
minimum_depth: None, // Filled in in accept_channel
30693056
};
30703057
let channel_context = Self {
30713058
user_id,
@@ -3140,7 +3127,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
31403127
holder_htlc_minimum_msat: if config.channel_handshake_config.our_htlc_minimum_msat == 0 { 1 } else { config.channel_handshake_config.our_htlc_minimum_msat },
31413128
counterparty_max_accepted_htlcs: 0,
31423129
holder_max_accepted_htlcs: cmp::min(config.channel_handshake_config.our_max_accepted_htlcs, max_htlcs(&channel_type)),
3143-
negotiated_minimum_depth: None, // Filled in in accept_channel
3130+
minimum_depth: None, // Filled in in accept_channel
31443131

31453132
counterparty_forwarding_info: None,
31463133

@@ -3326,6 +3313,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
33263313
self.temporary_channel_id
33273314
}
33283315

3316+
pub fn minimum_depth(&self) -> Option<u32> {
3317+
self.minimum_depth
3318+
}
3319+
33293320
/// Gets the "user_id" value passed into the construction of this channel. It has no special
33303321
/// meaning and exists only to allow users to have a persistent identifier of a channel.
33313322
pub fn get_user_id(&self) -> u128 {
@@ -3467,11 +3458,10 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
34673458
self.counterparty_max_accepted_htlcs = common_fields.max_accepted_htlcs;
34683459

34693460
if peer_limits.trust_own_funding_0conf {
3470-
funding.minimum_depth = Some(common_fields.minimum_depth);
3461+
self.minimum_depth = Some(common_fields.minimum_depth);
34713462
} else {
3472-
funding.minimum_depth = Some(cmp::max(1, common_fields.minimum_depth));
3463+
self.minimum_depth = Some(cmp::max(1, common_fields.minimum_depth));
34733464
}
3474-
self.negotiated_minimum_depth = funding.minimum_depth;
34753465

34763466
let counterparty_pubkeys = ChannelPublicKeys {
34773467
funding_pubkey: common_fields.funding_pubkey,
@@ -6771,7 +6761,7 @@ impl<SP: Deref> FundedChannel<SP> where
67716761
// the funding transaction confirmed before the monitor was persisted, or
67726762
// * a 0-conf channel and intended to send the channel_ready before any broadcast at all.
67736763
let channel_ready = if self.context.monitor_pending_channel_ready {
6774-
assert!(!self.funding.is_outbound() || self.funding.minimum_depth == Some(0),
6764+
assert!(!self.funding.is_outbound() || self.context.minimum_depth == Some(0),
67756765
"Funding transaction broadcast by the local client before it should have - LDK didn't do it!");
67766766
self.context.monitor_pending_channel_ready = false;
67776767
self.get_channel_ready(logger)
@@ -8020,7 +8010,7 @@ impl<SP: Deref> FundedChannel<SP> where
80208010
) {
80218011
// If we're not a 0conf channel, we'll be waiting on a monitor update with only
80228012
// AwaitingChannelReady set, though our peer could have sent their channel_ready.
8023-
debug_assert!(self.funding.minimum_depth.unwrap_or(1) > 0);
8013+
debug_assert!(self.context.minimum_depth.unwrap_or(1) > 0);
80248014
return true;
80258015
}
80268016
if self.holder_commitment_point.transaction_number() == INITIAL_COMMITMENT_NUMBER - 1 &&
@@ -8094,7 +8084,7 @@ impl<SP: Deref> FundedChannel<SP> where
80948084
// Called:
80958085
// * always when a new block/transactions are confirmed with the new height
80968086
// * when funding is signed with a height of 0
8097-
if self.funding.funding_tx_confirmation_height == 0 && self.funding.minimum_depth != Some(0) {
8087+
if self.funding.funding_tx_confirmation_height == 0 && self.context.minimum_depth != Some(0) {
80988088
return None;
80998089
}
81008090

@@ -8103,7 +8093,7 @@ impl<SP: Deref> FundedChannel<SP> where
81038093
self.funding.funding_tx_confirmation_height = 0;
81048094
}
81058095

8106-
if funding_tx_confirmations < self.funding.minimum_depth.unwrap_or(0) as i64 {
8096+
if funding_tx_confirmations < self.context.minimum_depth.unwrap_or(0) as i64 {
81078097
return None;
81088098
}
81098099

@@ -8229,9 +8219,9 @@ impl<SP: Deref> FundedChannel<SP> where
82298219
// If this is a coinbase transaction and not a 0-conf channel
82308220
// we should update our min_depth to 100 to handle coinbase maturity
82318221
if tx.is_coinbase() &&
8232-
self.funding.minimum_depth.unwrap_or(0) > 0 &&
8233-
self.funding.minimum_depth.unwrap_or(0) < COINBASE_MATURITY {
8234-
self.funding.minimum_depth = Some(COINBASE_MATURITY);
8222+
self.context.minimum_depth.unwrap_or(0) > 0 &&
8223+
self.context.minimum_depth.unwrap_or(0) < COINBASE_MATURITY {
8224+
self.context.minimum_depth = Some(COINBASE_MATURITY);
82358225
}
82368226
}
82378227
// If we allow 1-conf funding, we may need to check for channel_ready here and
@@ -8332,7 +8322,7 @@ impl<SP: Deref> FundedChannel<SP> where
83328322
// to.
83338323
if funding_tx_confirmations == 0 && self.funding.funding_tx_confirmed_in.is_some() {
83348324
let err_reason = format!("Funding transaction was un-confirmed. Locked at {} confs, now have {} confs.",
8335-
self.funding.minimum_depth.unwrap(), funding_tx_confirmations);
8325+
self.context.minimum_depth.unwrap(), funding_tx_confirmations);
83368326
return Err(ClosureReason::ProcessingError { err: err_reason });
83378327
}
83388328
} else if !self.funding.is_outbound() && self.funding.funding_tx_confirmed_in.is_none() &&
@@ -9618,9 +9608,9 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
96189608
// If the funding transaction is a coinbase transaction, we need to set the minimum depth to 100.
96199609
// We can skip this if it is a zero-conf channel.
96209610
if funding_transaction.is_coinbase() &&
9621-
self.funding.minimum_depth.unwrap_or(0) > 0 &&
9622-
self.funding.minimum_depth.unwrap_or(0) < COINBASE_MATURITY {
9623-
self.funding.minimum_depth = Some(COINBASE_MATURITY);
9611+
self.context.minimum_depth.unwrap_or(0) > 0 &&
9612+
self.context.minimum_depth.unwrap_or(0) < COINBASE_MATURITY {
9613+
self.context.minimum_depth = Some(COINBASE_MATURITY);
96249614
}
96259615

96269616
debug_assert!(self.funding.funding_transaction.is_none());
@@ -9949,7 +9939,7 @@ impl<SP: Deref> InboundV1Channel<SP> where SP::Target: SignerProvider {
99499939
dust_limit_satoshis: self.context.holder_dust_limit_satoshis,
99509940
max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat,
99519941
htlc_minimum_msat: self.context.holder_htlc_minimum_msat,
9952-
minimum_depth: self.funding.minimum_depth.unwrap(),
9942+
minimum_depth: self.context.minimum_depth.unwrap(),
99539943
to_self_delay: self.funding.get_holder_selected_contest_delay(),
99549944
max_accepted_htlcs: self.context.holder_max_accepted_htlcs,
99559945
funding_pubkey: keys.funding_pubkey,
@@ -10361,7 +10351,7 @@ impl<SP: Deref> PendingV2Channel<SP> where SP::Target: SignerProvider {
1036110351
dust_limit_satoshis: self.context.holder_dust_limit_satoshis,
1036210352
max_htlc_value_in_flight_msat: self.context.holder_max_htlc_value_in_flight_msat,
1036310353
htlc_minimum_msat: self.context.holder_htlc_minimum_msat,
10364-
minimum_depth: self.funding.minimum_depth.unwrap(),
10354+
minimum_depth: self.context.minimum_depth.unwrap(),
1036510355
to_self_delay: self.funding.get_holder_selected_contest_delay(),
1036610356
max_accepted_htlcs: self.context.holder_max_accepted_htlcs,
1036710357
funding_pubkey: keys.funding_pubkey,
@@ -10729,7 +10719,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1072910719
self.context.counterparty_max_accepted_htlcs.write(writer)?;
1073010720

1073110721
// Note that this field is ignored by 0.0.99+ as the TLV Optional variant is used instead.
10732-
self.context.negotiated_minimum_depth.unwrap_or(0).write(writer)?;
10722+
self.context.minimum_depth.unwrap_or(0).write(writer)?;
1073310723

1073410724
match &self.context.counterparty_forwarding_info {
1073510725
Some(info) => {
@@ -10804,7 +10794,7 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1080410794
// here. On the read side, old versions will simply ignore the odd-type entries here,
1080510795
// and new versions map the default values to None and allow the TLV entries here to
1080610796
// override that.
10807-
(1, self.funding.minimum_depth, option),
10797+
(1, self.context.minimum_depth, option),
1080810798
(2, chan_type, option),
1080910799
(3, self.funding.counterparty_selected_channel_reserve_satoshis, option),
1081010800
(4, serialized_holder_selected_reserve, option),
@@ -10840,7 +10830,6 @@ impl<SP: Deref> Writeable for FundedChannel<SP> where SP::Target: SignerProvider
1084010830
(54, self.pending_funding, optional_vec), // Added in 0.2
1084110831
(55, removed_htlc_failure_attribution_data, optional_vec), // Added in 0.2
1084210832
(57, holding_cell_failure_attribution_data, optional_vec), // Added in 0.2
10843-
(59, self.context.negotiated_minimum_depth, option), // Added in 0.2
1084410833
});
1084510834

1084610835
Ok(())
@@ -11139,7 +11128,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1113911128
let mut is_manual_broadcast = None;
1114011129

1114111130
let mut pending_funding = Some(Vec::new());
11142-
let mut negotiated_minimum_depth: Option<u32> = None;
1114311131

1114411132
read_tlv_fields!(reader, {
1114511133
(0, announcement_sigs, option),
@@ -11179,7 +11167,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1117911167
(54, pending_funding, optional_vec), // Added in 0.2
1118011168
(55, removed_htlc_failure_attribution_data, optional_vec),
1118111169
(57, holding_cell_failure_attribution_data, optional_vec),
11182-
(59, negotiated_minimum_depth, option), // Added in 0.2
1118311170
});
1118411171

1118511172
let holder_signer = signer_provider.derive_channel_signer(channel_keys_id);
@@ -11355,7 +11342,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1135511342
funding_tx_confirmed_in,
1135611343
funding_tx_confirmation_height,
1135711344
short_channel_id,
11358-
minimum_depth,
1135911345
},
1136011346
pending_funding: pending_funding.unwrap(),
1136111347
context: ChannelContext {
@@ -11428,8 +11414,7 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, &'c Channel
1142811414
counterparty_htlc_minimum_msat,
1142911415
holder_htlc_minimum_msat,
1143011416
counterparty_max_accepted_htlcs,
11431-
// TODO: But what if minimum_depth (TLV 1) had been overridden with COINBASE_MATURITY?
11432-
negotiated_minimum_depth: negotiated_minimum_depth.or(minimum_depth),
11417+
minimum_depth,
1143311418

1143411419
counterparty_forwarding_info,
1143511420

‎lightning/src/ln/channel_state.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -531,7 +531,7 @@ impl ChannelDetails {
531531
next_outbound_htlc_limit_msat: balance.next_outbound_htlc_limit_msat,
532532
next_outbound_htlc_minimum_msat: balance.next_outbound_htlc_minimum_msat,
533533
user_channel_id: context.get_user_id(),
534-
confirmations_required: funding.minimum_depth(),
534+
confirmations_required: context.minimum_depth(),
535535
confirmations: Some(funding.get_funding_tx_confirmations(best_block_height)),
536536
force_close_spend_delay: funding.get_counterparty_selected_contest_delay(),
537537
is_outbound: funding.is_outbound(),

‎lightning/src/ln/channelmanager.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3026,7 +3026,7 @@ macro_rules! locked_close_channel {
30263026
// into the map (which prevents the `PeerState` from being cleaned up) for channels that
30273027
// never even got confirmations (which would open us up to DoS attacks).
30283028
let update_id = $channel_context.get_latest_monitor_update_id();
3029-
if $channel_funding.get_funding_tx_confirmation_height().is_some() || $channel_funding.minimum_depth() == Some(0) || update_id > 1 {
3029+
if $channel_funding.get_funding_tx_confirmation_height().is_some() || $channel_context.minimum_depth() == Some(0) || update_id > 1 {
30303030
let chan_id = $channel_context.channel_id();
30313031
$peer_state.closed_channel_monitor_update_ids.insert(chan_id, update_id);
30323032
}
@@ -7947,7 +7947,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
79477947

79487948
if accept_0conf {
79497949
// This should have been correctly configured by the call to Inbound(V1/V2)Channel::new.
7950-
debug_assert!(channel.funding().minimum_depth().unwrap() == 0);
7950+
debug_assert!(channel.context().minimum_depth().unwrap() == 0);
79517951
} else if channel.funding().get_channel_type().requires_zero_conf() {
79527952
let send_msg_err_event = MessageSendEvent::HandleError {
79537953
node_id: channel.context().get_counterparty_node_id(),
@@ -8023,7 +8023,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
80238023
Some(funded_chan) => {
80248024
// This covers non-zero-conf inbound `Channel`s that we are currently monitoring, but those
80258025
// which have not yet had any confirmations on-chain.
8026-
if !funded_chan.funding.is_outbound() && funded_chan.funding.minimum_depth().unwrap_or(1) != 0 &&
8026+
if !funded_chan.funding.is_outbound() && funded_chan.context.minimum_depth().unwrap_or(1) != 0 &&
80278027
funded_chan.funding.get_funding_tx_confirmations(best_block_height) == 0
80288028
{
80298029
num_unfunded_channels += 1;
@@ -8036,7 +8036,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
80368036
}
80378037

80388038
// 0conf channels are not considered unfunded.
8039-
if chan.funding().minimum_depth().unwrap_or(1) == 0 {
8039+
if chan.context().minimum_depth().unwrap_or(1) == 0 {
80408040
continue;
80418041
}
80428042

0 commit comments

Comments
 (0)