Skip to content

Commit 0eaf329

Browse files
committed
Make impl_writeable_tlv_based_enum* actually upgradable
In cc78b77 it was discovered that `impl_writeable_tlv_based_enum_upgradable` wasn't actually upgradable - tuple variants weren't written with length-prefixes, causing downgrades with new tuple variants to be unreadable by older clients as they wouldn't know where to stop reading. This was fixed by simply assuming that any new variants will be non-tuple variants with a length prefix, but no code write-side changes were made, allowing new code to freely continue to use the broken tuple-variant serialization. Here we address this be defining yet more serialization macros which aren't broken, and convert existing usage of the existing macros using non-length-prefixed tuple variants to renamed `*_legacy` macros. Note that this changes the serialization format of `impl_writeable_tlv_based_enum[_upgradable]` when tuple fields are written, and as such deliberately changes the call semantics for such tuples.
1 parent 40283e7 commit 0eaf329

File tree

14 files changed

+212
-44
lines changed

14 files changed

+212
-44
lines changed

lightning/src/blinded_path/payment.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -431,7 +431,7 @@ impl Readable for PaymentConstraints {
431431
}
432432
}
433433

434-
impl_writeable_tlv_based_enum!(PaymentContext,
434+
impl_writeable_tlv_based_enum_legacy!(PaymentContext,
435435
;
436436
(0, Unknown),
437437
(1, Bolt12Offer),

lightning/src/chain/channelmonitor.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ pub enum MonitorEvent {
189189
monitor_update_id: u64,
190190
},
191191
}
192-
impl_writeable_tlv_based_enum_upgradable!(MonitorEvent,
192+
impl_writeable_tlv_based_enum_upgradable_legacy!(MonitorEvent,
193193
// Note that Completed is currently never serialized to disk as it is generated only in
194194
// ChainMonitor.
195195
(0, Completed) => {

lightning/src/chain/package.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -689,7 +689,7 @@ impl PackageSolvingData {
689689
}
690690
}
691691

692-
impl_writeable_tlv_based_enum!(PackageSolvingData, ;
692+
impl_writeable_tlv_based_enum_legacy!(PackageSolvingData, ;
693693
(0, RevokedOutput),
694694
(1, RevokedHTLCOutput),
695695
(2, CounterpartyOfferedHTLCOutput),

lightning/src/events/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ impl PaymentPurpose {
171171
}
172172
}
173173

174-
impl_writeable_tlv_based_enum!(PaymentPurpose,
174+
impl_writeable_tlv_based_enum_legacy!(PaymentPurpose,
175175
(0, Bolt11InvoicePayment) => {
176176
(0, payment_preimage, option),
177177
(2, payment_secret, required),
@@ -494,7 +494,7 @@ enum InterceptNextHop {
494494
impl_writeable_tlv_based_enum!(InterceptNextHop,
495495
(0, FakeScid) => {
496496
(0, requested_next_hop_scid, required),
497-
};
497+
},
498498
);
499499

500500
/// The reason the payment failed. Used in [`Event::PaymentFailed`].
@@ -535,7 +535,7 @@ impl_writeable_tlv_based_enum!(PaymentFailureReason,
535535
(4, RetriesExhausted) => {},
536536
(6, PaymentExpired) => {},
537537
(8, RouteNotFound) => {},
538-
(10, UnexpectedError) => {}, ;
538+
(10, UnexpectedError) => {},
539539
);
540540

541541
/// An Event which you should probably take some action in response to.

lightning/src/ln/channel.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ impl_writeable_tlv_based_enum!(InboundHTLCResolution,
130130
},
131131
(2, Pending) => {
132132
(0, update_add_htlc, required),
133-
};
133+
},
134134
);
135135

136136
enum InboundHTLCState {
@@ -8340,7 +8340,7 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures)
83408340
const SERIALIZATION_VERSION: u8 = 4;
83418341
const MIN_SERIALIZATION_VERSION: u8 = 3;
83428342

8343-
impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,;
8343+
impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,;
83448344
(0, FailRelay),
83458345
(1, FailMalformed),
83468346
(2, Fulfill),

lightning/src/ln/channel_state.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ impl_writeable_tlv_based_enum_upgradable!(InboundHTLCStateDetails,
6969
(0, AwaitingRemoteRevokeToAdd) => {},
7070
(2, Committed) => {},
7171
(4, AwaitingRemoteRevokeToRemoveFulfill) => {},
72-
(6, AwaitingRemoteRevokeToRemoveFail) => {};
72+
(6, AwaitingRemoteRevokeToRemoveFail) => {},
7373
);
7474

7575
/// Exposes details around pending inbound HTLCs.
@@ -159,7 +159,7 @@ impl_writeable_tlv_based_enum_upgradable!(OutboundHTLCStateDetails,
159159
(0, AwaitingRemoteRevokeToAdd) => {},
160160
(2, Committed) => {},
161161
(4, AwaitingRemoteRevokeToRemoveSuccess) => {},
162-
(6, AwaitingRemoteRevokeToRemoveFailure) => {};
162+
(6, AwaitingRemoteRevokeToRemoveFailure) => {},
163163
);
164164

165165
/// Exposes details around pending outbound HTLCs.
@@ -701,5 +701,5 @@ impl_writeable_tlv_based_enum!(ChannelShutdownState,
701701
(2, ShutdownInitiated) => {},
702702
(4, ResolvingHTLCs) => {},
703703
(6, NegotiatingClosingFee) => {},
704-
(8, ShutdownComplete) => {}, ;
704+
(8, ShutdownComplete) => {},
705705
);

lightning/src/ln/channelmanager.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ impl_writeable_tlv_based_enum!(SentHTLCId,
473473
},
474474
(2, OutboundRoute) => {
475475
(0, session_priv, required),
476-
};
476+
},
477477
);
478478

479479

@@ -833,7 +833,7 @@ impl_writeable_tlv_based_enum!(EventCompletionAction,
833833
// Note that by the time we get past the required read above, channel_funding_outpoint will be
834834
// filled in, so we can safely unwrap it here.
835835
(3, channel_id, (default_value, ChannelId::v1_from_funding_outpoint(channel_funding_outpoint.0.unwrap()))),
836-
};
836+
}
837837
);
838838

839839
#[derive(Clone, PartialEq, Eq, Debug)]
@@ -862,7 +862,7 @@ impl RAAMonitorUpdateBlockingAction {
862862

863863
impl_writeable_tlv_based_enum!(RAAMonitorUpdateBlockingAction,
864864
(0, ForwardedPaymentInboundClaim) => { (0, channel_id, required), (2, htlc_id, required) }
865-
;);
865+
);
866866

867867

868868
/// State we hold per-peer.
@@ -10542,7 +10542,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting,
1054210542
(4, payment_data, option), // Added in 0.0.116
1054310543
(5, custom_tlvs, optional_vec),
1054410544
},
10545-
;);
10545+
);
1054610546

1054710547
impl_writeable_tlv_based!(PendingHTLCInfo, {
1054810548
(0, routing, required),
@@ -10622,14 +10622,14 @@ impl Readable for HTLCFailureMsg {
1062210622
}
1062310623
}
1062410624

10625-
impl_writeable_tlv_based_enum!(PendingHTLCStatus, ;
10625+
impl_writeable_tlv_based_enum_legacy!(PendingHTLCStatus, ;
1062610626
(0, Forward),
1062710627
(1, Fail),
1062810628
);
1062910629

1063010630
impl_writeable_tlv_based_enum!(BlindedFailure,
1063110631
(0, FromIntroductionNode) => {},
10632-
(2, FromBlindedNode) => {}, ;
10632+
(2, FromBlindedNode) => {},
1063310633
);
1063410634

1063510635
impl_writeable_tlv_based!(HTLCPreviousHopData, {

lightning/src/ln/onion_utils.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -953,7 +953,7 @@ impl_writeable_tlv_based_enum!(HTLCFailReasonRepr,
953953
(0, failure_code, required),
954954
(2, data, required_vec),
955955
},
956-
;);
956+
);
957957

958958
impl HTLCFailReason {
959959
#[rustfmt::skip]

lightning/src/ln/outbound_payment.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -300,13 +300,13 @@ pub enum Retry {
300300
}
301301

302302
#[cfg(not(feature = "std"))]
303-
impl_writeable_tlv_based_enum!(Retry,
303+
impl_writeable_tlv_based_enum_legacy!(Retry,
304304
;
305305
(0, Attempts)
306306
);
307307

308308
#[cfg(feature = "std")]
309-
impl_writeable_tlv_based_enum!(Retry,
309+
impl_writeable_tlv_based_enum_legacy!(Retry,
310310
;
311311
(0, Attempts),
312312
(2, Timeout)
@@ -397,7 +397,7 @@ pub(crate) enum StaleExpiration {
397397
AbsoluteTimeout(core::time::Duration),
398398
}
399399

400-
impl_writeable_tlv_based_enum!(StaleExpiration,
400+
impl_writeable_tlv_based_enum_legacy!(StaleExpiration,
401401
;
402402
(0, TimerTicks),
403403
(2, AbsoluteTimeout)

lightning/src/ln/script.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ impl Readable for ShutdownScript {
5353
}
5454
}
5555

56-
impl_writeable_tlv_based_enum!(ShutdownScriptImpl, ;
56+
impl_writeable_tlv_based_enum_legacy!(ShutdownScriptImpl, ;
5757
(0, Legacy),
5858
(1, Bolt2),
5959
);

lightning/src/sign/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ pub enum SpendableOutputDescriptor {
301301
StaticPaymentOutput(StaticPaymentOutputDescriptor),
302302
}
303303

304-
impl_writeable_tlv_based_enum!(SpendableOutputDescriptor,
304+
impl_writeable_tlv_based_enum_legacy!(SpendableOutputDescriptor,
305305
(0, StaticOutput) => {
306306
(0, outpoint, required),
307307
(1, channel_keys_id, option),

lightning/src/util/config.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -410,7 +410,7 @@ pub enum MaxDustHTLCExposure {
410410
FeeRateMultiplier(u64),
411411
}
412412

413-
impl_writeable_tlv_based_enum!(MaxDustHTLCExposure, ;
413+
impl_writeable_tlv_based_enum_legacy!(MaxDustHTLCExposure, ;
414414
(1, FixedLimitMsat),
415415
(3, FeeRateMultiplier),
416416
);

0 commit comments

Comments
 (0)