Skip to content

Fix backwards compat for blocked_monitor_updates and finally kill vec_type #2400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 10 commits into from
Jul 11, 2023
Merged
20 changes: 10 additions & 10 deletions lightning/src/chain/channelmonitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ impl_writeable_tlv_based!(HolderSignedTx, {
(8, delayed_payment_key, required),
(10, per_commitment_point, required),
(12, feerate_per_kw, required),
(14, htlc_outputs, vec_type)
(14, htlc_outputs, required_vec)
});

impl HolderSignedTx {
Expand Down Expand Up @@ -538,15 +538,15 @@ impl ChannelMonitorUpdateStep {
impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
(0, LatestHolderCommitmentTXInfo) => {
(0, commitment_tx, required),
(1, claimed_htlcs, vec_type),
(2, htlc_outputs, vec_type),
(1, claimed_htlcs, optional_vec),
(2, htlc_outputs, required_vec),
(4, nondust_htlc_sources, optional_vec),
},
(1, LatestCounterpartyCommitmentTXInfo) => {
(0, commitment_txid, required),
(2, commitment_number, required),
(4, their_per_commitment_point, required),
(6, htlc_outputs, vec_type),
(6, htlc_outputs, required_vec),
},
(2, PaymentPreimage) => {
(0, payment_preimage, required),
Expand Down Expand Up @@ -1075,12 +1075,12 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signe

write_tlv_fields!(writer, {
(1, self.funding_spend_confirmed, option),
(3, self.htlcs_resolved_on_chain, vec_type),
(5, self.pending_monitor_events, vec_type),
(3, self.htlcs_resolved_on_chain, required_vec),
(5, self.pending_monitor_events, required_vec),
(7, self.funding_spend_seen, required),
(9, self.counterparty_node_id, option),
(11, self.confirmed_commitment_tx_counterparty_output, option),
(13, self.spendable_txids_confirmed, vec_type),
(13, self.spendable_txids_confirmed, required_vec),
(15, self.counterparty_fulfilled_htlcs, required),
});

Expand Down Expand Up @@ -4051,12 +4051,12 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
let mut counterparty_fulfilled_htlcs = Some(HashMap::new());
read_tlv_fields!(reader, {
(1, funding_spend_confirmed, option),
(3, htlcs_resolved_on_chain, vec_type),
(5, pending_monitor_events, vec_type),
(3, htlcs_resolved_on_chain, optional_vec),
(5, pending_monitor_events, optional_vec),
(7, funding_spend_seen, option),
(9, counterparty_node_id, option),
(11, confirmed_commitment_tx_counterparty_output, option),
(13, spendable_txids_confirmed, vec_type),
(13, spendable_txids_confirmed, optional_vec),
(15, counterparty_fulfilled_htlcs, option),
});

Expand Down
24 changes: 13 additions & 11 deletions lightning/src/events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -909,7 +909,7 @@ impl Writeable for Event {
(2, payment_failed_permanently, required),
(3, false, required), // all_paths_failed in LDK versions prior to 0.0.114
(4, path.blinded_tail, option),
(5, path.hops, vec_type),
(5, path.hops, required_vec),
(7, short_channel_id, option),
(9, None::<RouteParameters>, option), // retry in LDK versions prior to 0.0.115
(11, payment_id, option),
Expand Down Expand Up @@ -977,7 +977,7 @@ impl Writeable for Event {
write_tlv_fields!(writer, {
(0, payment_id, required),
(2, payment_hash, option),
(4, path.hops, vec_type),
(4, path.hops, required_vec),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, how did you find all the commits you referenced when checking if certain fields had been written as required for a while?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git blame and some elbow grease :)

(6, path.blinded_tail, option),
})
},
Expand Down Expand Up @@ -1008,7 +1008,7 @@ impl Writeable for Event {
write_tlv_fields!(writer, {
(0, payment_id, required),
(2, payment_hash, required),
(4, path.hops, vec_type),
(4, path.hops, required_vec),
(6, path.blinded_tail, option),
})
},
Expand All @@ -1017,7 +1017,7 @@ impl Writeable for Event {
write_tlv_fields!(writer, {
(0, payment_id, required),
(2, payment_hash, required),
(4, path.hops, vec_type),
(4, path.hops, required_vec),
(6, short_channel_id, option),
(8, path.blinded_tail, option),
})
Expand Down Expand Up @@ -1162,7 +1162,9 @@ impl MaybeReadable for Event {
(1, network_update, upgradable_option),
(2, payment_failed_permanently, required),
(4, blinded_tail, option),
(5, path, vec_type),
// Added as a part of LDK 0.0.101 and always filled in since.
// Defaults to an empty Vec, though likely should have been `Option`al.
(5, path, optional_vec),
(7, short_channel_id, option),
(11, payment_id, option),
(13, failure_opt, upgradable_option),
Expand Down Expand Up @@ -1279,13 +1281,13 @@ impl MaybeReadable for Event {
_init_and_read_tlv_fields!(reader, {
(0, payment_id, required),
(2, payment_hash, option),
(4, path, vec_type),
(4, path, required_vec),
(6, blinded_tail, option),
});
Ok(Some(Event::PaymentPathSuccessful {
payment_id: payment_id.0.unwrap(),
payment_hash,
path: Path { hops: path.unwrap(), blinded_tail },
path: Path { hops: path, blinded_tail },
}))
};
f()
Expand Down Expand Up @@ -1338,13 +1340,13 @@ impl MaybeReadable for Event {
_init_and_read_tlv_fields!(reader, {
(0, payment_id, required),
(2, payment_hash, required),
(4, path, vec_type),
(4, path, required_vec),
(6, blinded_tail, option),
});
Ok(Some(Event::ProbeSuccessful {
payment_id: payment_id.0.unwrap(),
payment_hash: payment_hash.0.unwrap(),
path: Path { hops: path.unwrap(), blinded_tail },
path: Path { hops: path, blinded_tail },
}))
};
f()
Expand All @@ -1354,14 +1356,14 @@ impl MaybeReadable for Event {
_init_and_read_tlv_fields!(reader, {
(0, payment_id, required),
(2, payment_hash, required),
(4, path, vec_type),
(4, path, required_vec),
(6, short_channel_id, option),
(8, blinded_tail, option),
});
Ok(Some(Event::ProbeFailed {
payment_id: payment_id.0.unwrap(),
payment_hash: payment_hash.0.unwrap(),
path: Path { hops: path.unwrap(), blinded_tail },
path: Path { hops: path, blinded_tail },
short_channel_id,
}))
};
Expand Down
20 changes: 5 additions & 15 deletions lightning/src/ln/chan_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,7 +1056,7 @@ impl_writeable_tlv_based!(HolderCommitmentTransaction, {
(0, inner, required),
(2, counterparty_sig, required),
(4, holder_sig_first, required),
(6, counterparty_htlc_sigs, vec_type),
(6, counterparty_htlc_sigs, required_vec),
});

impl HolderCommitmentTransaction {
Expand Down Expand Up @@ -1346,7 +1346,7 @@ impl Writeable for CommitmentTransaction {
(6, self.feerate_per_kw, required),
(8, self.keys, required),
(10, self.built, required),
(12, self.htlcs, vec_type),
(12, self.htlcs, required_vec),
(14, legacy_deserialization_prevention_marker, option),
(15, self.channel_type_features, required),
});
Expand All @@ -1356,24 +1356,14 @@ impl Writeable for CommitmentTransaction {

impl Readable for CommitmentTransaction {
fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
let mut commitment_number = RequiredWrapper(None);
let mut to_broadcaster_value_sat = RequiredWrapper(None);
let mut to_countersignatory_value_sat = RequiredWrapper(None);
let mut feerate_per_kw = RequiredWrapper(None);
let mut keys = RequiredWrapper(None);
let mut built = RequiredWrapper(None);
_init_tlv_field_var!(htlcs, vec_type);
let mut _legacy_deserialization_prevention_marker: Option<()> = None;
let mut channel_type_features = None;

read_tlv_fields!(reader, {
_init_and_read_tlv_fields!(reader, {
(0, commitment_number, required),
(2, to_broadcaster_value_sat, required),
(4, to_countersignatory_value_sat, required),
(6, feerate_per_kw, required),
(8, keys, required),
(10, built, required),
(12, htlcs, vec_type),
(12, htlcs, required_vec),
(14, _legacy_deserialization_prevention_marker, option),
(15, channel_type_features, option),
});
Expand All @@ -1389,7 +1379,7 @@ impl Readable for CommitmentTransaction {
feerate_per_kw: feerate_per_kw.0.unwrap(),
keys: keys.0.unwrap(),
built: built.0.unwrap(),
htlcs: _init_tlv_based_struct_field!(htlcs, vec_type),
htlcs,
channel_type_features: channel_type_features.unwrap_or(ChannelTypeFeatures::only_static_remote_key())
})
}
Expand Down
12 changes: 6 additions & 6 deletions lightning/src/ln/channel.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6781,11 +6781,11 @@ impl<Signer: WriteableEcdsaChannelSigner> Writeable for Channel<Signer> {
(5, self.context.config, required),
(6, serialized_holder_htlc_max_in_flight, option),
(7, self.context.shutdown_scriptpubkey, option),
(8, self.context.blocked_monitor_updates, vec_type),
(8, self.context.blocked_monitor_updates, optional_vec),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops :( Good thing we're getting rid of vec_type.

(9, self.context.target_closing_feerate_sats_per_kw, option),
(11, self.context.monitor_pending_finalized_fulfills, vec_type),
(11, self.context.monitor_pending_finalized_fulfills, required_vec),
(13, self.context.channel_creation_height, required),
(15, preimages, vec_type),
(15, preimages, required_vec),
(17, self.context.announcement_sigs_state, required),
(19, self.context.latest_inbound_scid_alias, option),
(21, self.context.outbound_scid_alias, required),
Expand Down Expand Up @@ -7089,11 +7089,11 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
(5, config, option), // Note that if none is provided we will *not* overwrite the existing one.
(6, holder_max_htlc_value_in_flight_msat, option),
(7, shutdown_scriptpubkey, option),
(8, blocked_monitor_updates, vec_type),
(8, blocked_monitor_updates, optional_vec),
(9, target_closing_feerate_sats_per_kw, option),
(11, monitor_pending_finalized_fulfills, vec_type),
(11, monitor_pending_finalized_fulfills, optional_vec),
(13, channel_creation_height, option),
(15, preimages_opt, vec_type),
(15, preimages_opt, optional_vec),
(17, announcement_sigs_state, option),
(19, latest_inbound_scid_alias, option),
(21, outbound_scid_alias, option),
Expand Down
17 changes: 9 additions & 8 deletions lightning/src/ln/channelmanager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ use crate::ln::features::{ChannelFeatures, ChannelTypeFeatures, InitFeatures, No
#[cfg(any(feature = "_test_utils", test))]
use crate::ln::features::InvoiceFeatures;
use crate::routing::gossip::NetworkGraph;
use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteHop, RouteParameters, Router};
use crate::routing::router::{BlindedTail, DefaultRouter, InFlightHtlcs, Path, Payee, PaymentParameters, Route, RouteParameters, Router};
use crate::routing::scoring::{ProbabilisticScorer, ProbabilisticScoringFeeParameters};
use crate::ln::msgs;
use crate::ln::onion_utils;
Expand Down Expand Up @@ -3170,6 +3170,7 @@ where
/// irrevocably committed to on our end. In such a case, do NOT retry the payment with a
/// different route unless you intend to pay twice!
///
/// [`RouteHop`]: crate::routing::router::RouteHop
/// [`Event::PaymentSent`]: events::Event::PaymentSent
/// [`Event::PaymentFailed`]: events::Event::PaymentFailed
/// [`UpdateHTLCs`]: events::MessageSendEvent::UpdateHTLCs
Expand Down Expand Up @@ -7442,7 +7443,7 @@ impl Readable for ChannelDetails {
}

impl_writeable_tlv_based!(PhantomRouteHints, {
(2, channels, vec_type),
(2, channels, required_vec),
(4, phantom_scid, required),
(6, real_node_pubkey, required),
});
Expand Down Expand Up @@ -7634,15 +7635,15 @@ impl Readable for HTLCSource {
0 => {
let mut session_priv: crate::util::ser::RequiredWrapper<SecretKey> = crate::util::ser::RequiredWrapper(None);
let mut first_hop_htlc_msat: u64 = 0;
let mut path_hops: Option<Vec<RouteHop>> = Some(Vec::new());
let mut path_hops = Vec::new();
let mut payment_id = None;
let mut payment_params: Option<PaymentParameters> = None;
let mut blinded_tail: Option<BlindedTail> = None;
read_tlv_fields!(reader, {
(0, session_priv, required),
(1, payment_id, option),
(2, first_hop_htlc_msat, required),
(4, path_hops, vec_type),
(4, path_hops, required_vec),
(5, payment_params, (option: ReadableArgs, 0)),
(6, blinded_tail, option),
});
Expand All @@ -7651,7 +7652,7 @@ impl Readable for HTLCSource {
// instead.
payment_id = Some(PaymentId(*session_priv.0.unwrap().as_ref()));
}
let path = Path { hops: path_hops.ok_or(DecodeError::InvalidValue)?, blinded_tail };
let path = Path { hops: path_hops, blinded_tail };
if path.hops.len() == 0 {
return Err(DecodeError::InvalidValue);
}
Expand Down Expand Up @@ -7686,7 +7687,7 @@ impl Writeable for HTLCSource {
(1, payment_id_opt, option),
(2, first_hop_htlc_msat, required),
// 3 was previously used to write a PaymentSecret for the payment.
(4, path.hops, vec_type),
(4, path.hops, required_vec),
(5, None::<PaymentParameters>, option), // payment_params in LDK versions prior to 0.0.115
(6, path.blinded_tail, option),
});
Expand Down Expand Up @@ -7936,7 +7937,7 @@ where
(6, monitor_update_blocked_actions_per_peer, option),
(7, self.fake_scid_rand_bytes, required),
(8, if events_not_backwards_compatible { Some(&*events) } else { None }, option),
(9, htlc_purposes, vec_type),
(9, htlc_purposes, required_vec),
(10, in_flight_monitor_updates, option),
(11, self.probing_cookie_secret, required),
(13, htlc_onion_fields, optional_vec),
Expand Down Expand Up @@ -8375,7 +8376,7 @@ where
(6, monitor_update_blocked_actions_per_peer, option),
(7, fake_scid_rand_bytes, option),
(8, events_override, option),
(9, claimable_htlc_purposes, vec_type),
(9, claimable_htlc_purposes, optional_vec),
(10, in_flight_monitor_updates, option),
(11, probing_cookie_secret, option),
(13, claimable_htlc_onion_fields, optional_vec),
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3760,7 +3760,7 @@ mod tests {
let test_bytes = vec![42u8; 1000];
if let OnionHopDataFormat::NonFinalNode { short_channel_id } = payload.format {
_encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
(1, test_bytes, vec_type),
(1, test_bytes, required_vec),
(2, HighZeroBytesDroppedBigSize(payload.amt_to_forward), required),
(4, HighZeroBytesDroppedBigSize(payload.outgoing_cltv_value), required),
(6, short_channel_id, required)
Expand Down
2 changes: 1 addition & 1 deletion lightning/src/ln/onion_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
},
(1, Reason) => {
(0, failure_code, required),
(2, data, vec_type),
(2, data, required_vec),
},
;);

Expand Down
4 changes: 2 additions & 2 deletions lightning/src/onion_message/packet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,15 @@ impl<T: CustomOnionMessageContents> Writeable for (Payload<T>, [u8; 32]) {
match &self.0 {
Payload::Forward(ForwardControlTlvs::Blinded(encrypted_bytes)) => {
_encode_varint_length_prefixed_tlv!(w, {
(4, *encrypted_bytes, vec_type)
(4, *encrypted_bytes, required_vec)
})
},
Payload::Receive {
control_tlvs: ReceiveControlTlvs::Blinded(encrypted_bytes), reply_path, message,
} => {
_encode_varint_length_prefixed_tlv!(w, {
(2, reply_path, option),
(4, *encrypted_bytes, vec_type),
(4, *encrypted_bytes, required_vec),
(message.tlv_type(), message, required)
})
},
Expand Down
Loading