Update rust-lightning dependency #164
Conversation
luckysori
force-pushed
the
fix/ldk-116-upgrade-and-channel-keys-id
branch
2 times, most recently
from
927ee99 to
d1e08a0
Compare
luckysori
changed the base branch from
feature/ln-dlc-channels
to
chore/update-to-tip
luckysori
force-pushed
the
chore/update-to-tip
branch
from
d39cbf8 to
6be0604
Compare
luckysori
force-pushed
the
fix/ldk-116-upgrade-and-channel-keys-id
branch
from
d1e08a0 to
4e104b4
Compare
luckysori
changed the base branch from
chore/update-to-tip
to
feature/ln-dlc-channels
luckysori
force-pushed
the
fix/ldk-116-upgrade-and-channel-keys-id
branch
from
4e104b4 to
15bb7cb
Compare
|
@Tibo-lg: This PR syncs up our branch with |
| @@ -316,6 +316,11 @@ where | |||
| SubChannelState::OffChainClosed => { | |||
| s.is_offer = true; | |||
| s.update_idx -= 1; | |||
|
|
|||
| // We do this to give older channels a chance to upgrade to the new version which | |||
There was a problem hiding this comment.
Honestly I'm not a big fan of this. It feels hacky and error prone. I'd like to have a way to handle upgrading at de-serialization time. Not sure what the best way is to be honest, but for example LDK uses the ReadableArgs trait for when deserialization requires external data.
There was a problem hiding this comment.
Yeah, it was supposed to be hacky in this case.
We don't have to merge this here anyway. I will just update the PR and we can just keep depending on this patch in 10101.
There was a problem hiding this comment.
If you think it's time to take backward compatibility issues seriously, it's probably worth taking the time to find a proper solution (though it's also hard to say how many more breaking changes will be required to reach a stable implementation).
There was a problem hiding this comment.
I agree with you on needing to figure out how to do this properly as soon as possible. But we can probably live with this quick fix for now.
luckysori
force-pushed
the
fix/ldk-116-upgrade-and-channel-keys-id
branch
from
15bb7cb to
34db518
Compare
dlc-messages/src/ser_impls.rs
Outdated
| @@ -453,7 +453,14 @@ pub fn read_option_cb<R: ::std::io::Read, T, F>( | |||
| where | |||
| F: Fn(&mut R) -> Result<T, DecodeError>, | |||
| { | |||
| let prefix: u8 = Readable::read(reader)?; | |||
| let prefix: u8 = match Readable::read(reader) { | |||
There was a problem hiding this comment.
I wonder if it'd be better to have a different option type that allows the field not being present?
There was a problem hiding this comment.
Maybe! I thought this patch wouldn't be controversial, but it's fair to say that we don't need it since we dropped the other one. I'll remove this one too!
luckysori
force-pushed
the
fix/ldk-116-upgrade-and-channel-keys-id
branch
from
34db518 to
2400398
Compare
luckysori
force-pushed
the
fix/ldk-116-upgrade-and-channel-keys-id
branch
from
2400398 to
d335349
Compare
The backwards-compatibility problem is associated with the recent upgrade to
rust-lightning:0.0.116.