Implement and document Channel/ChannelManager (de)serialization by TheBlueMatt · Pull Request #223 · lightningdevkit/rust-lightning

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt · 2018-10-25T19:37:48Z

Based on #217 and #222.

I'm sure there are bugs hiding in various state transitions and interaction with serialize/deserialize, and definitely need to think harder about potential for funds loss on deserializing old states with pending HTLC closure due to conflicts between deserialized ChannelManager/ChannelMonitors, but need to get the big API changes landed here first.

yuntai · 2018-10-26T16:03:58Z

think there is a bug in one condition checking in fn send_commitment().

https://github.com/rust-bitcoin/rust-lightning/pull/223/files#diff-40049ab371072bb899cde6c5f42bcae6R3131

was not sure comment on the closed pr get notified.

TheBlueMatt · 2018-10-26T17:58:52Z

Hmm, github doesn't auto-open files so your like is actually busted. Can you comment on the line you think is wrong?

yuntai · 2018-10-27T01:50:10Z

#213 (comment)

this link works?

TheBlueMatt · 2018-10-27T05:46:05Z

Oops, indeed, will fix in #226.

Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Drop channel_close_key from ChannelKeys

…nnel Drop channel_monitor_claim_key from ChannelKeys

This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

This slightly changes the serialization format, but we're still early enough that that's OK.

Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

TheBlueMatt added this to the 0.0.6 milestone Oct 25, 2018

TheBlueMatt force-pushed the 2018-10-chanmanager-serialize branch 5 times, most recently from 87b62e0 to a56e717Compare October 26, 2018 02:12

TheBlueMatt force-pushed the 2018-10-chanmanager-serialize branch 4 times, most recently from da37a9b to 7fd3f11Compare October 26, 2018 21:24

Antoine Riardand others added 16 commits October 27, 2018 09:34

Generate Events from ChannelMonitor to indicate spendable ouputs
8c235d9
Extend KeyStorage with delayed_payment_base_key and per_commitment_point to derive local_delayed private key

Add a KeysInterface which provides keys from user
d33cb3c
Move ChannelKeys into keysinterface for generate a set of it from master_seed and change return type to panic on bogus data

Implement KeysInterface for KeysManager util
6d5dc6c

Integrate KeysInterface to ChannelManager with Arc
e361fed

Integrate shutdown_pubkey from KeysInterface in ChannelManager/Channel
32a9378
Drop channel_close_key from ChannelKeys

Integrate destination_script from KeysInterface in ChannelManager/Cha…
5180686
…nnel Drop channel_monitor_claim_key from ChannelKeys

Split Event, move MessageSendEvent push() inside channel_state lock
e397cb9

Send AcceptChannel responses out-of-band to ensure ordered delivery
608d517

Send funding_signed messages out-of-band to ensure ordered delivery
c962a27

Send announcement_signatures msgs out-of-band for ordered delivery
e382a7b

Send shutdown/closing_signed msgs out-of-band for ordered delivery
812f255

Send RAA/CS messages out-of-band to ensure ordered delivery
4342114

Respond to channel_reestablish out-of-band for ordered delivery
e2de49d

Send channel_reestablish out-of-band to ensure ordered deliver
249aa77

Avoid reentrancy of send_data from PeerHandler::read_bytes.
294ad32
This greatly simplifies clients of PeerHandler, and because almost all response messages have already been moved to process_events this doesn't change much effeciency-wise.

Fix and test update_add_htlc but disconnect pre-commitment_signed
c0c139c

TheBlueMatt added 10 commits October 27, 2018 09:42

Store [u8; 32]s instead of SharedSecrets (for deserialization)
bb43b98

Add a second Readable trait for state args, clean macros slightly
062a02d

Give ChannelMonitor a logger via new ReadableArgs trait
47fe673

Add a BIG lock to ChannelManager
74cec62
During normal operation we should never need to take this, so we use a RwLock that allows normal parallelism until we want to serialize out our ChannelManager, at which point we can take the write-mode lock.

impl Readable/Writable for Route
4eafd37

impl some additional (de)serializers, including a u48 wrapper type
f049011

Redo ChannelMonitor deserialization to avoid read_to_end()
612e280
This slightly changes the serialization format, but we're still early enough that that's OK.

Track last_block_hash in ChannelMonitor and expose it on deser
56513f2
Also make block_connected take a &mut self to ensure serialized state will always be self-consistent.

Track ChannelMonitor-watched-outpoints (+ remove now-uesless Mutex)
6f08779

Store+expose bits of Channel[Monitor] to figure out local state
b2bd64d

TheBlueMatt force-pushed the 2018-10-chanmanager-serialize branch from 7fd3f11 to 3b18677Compare October 27, 2018 13:44

TheBlueMatt added 2 commits October 27, 2018 09:58

Implement and document Channel/ChannelManager (de)serialization
a2fb3cc

Add very basic test of ChannelManager serialization round-trip
7f91572

TheBlueMatt force-pushed the 2018-10-chanmanager-serialize branch from 3b18677 to 7f91572Compare October 27, 2018 13:58

TheBlueMatt merged commit 8cc3be9 into lightningdevkit:masterOct 27, 2018

TheBlueMatt mentioned this pull request Oct 28, 2018
Implement serialization/deserialization of Channel/ChannelManager #55
Closed

TheBlueMatt mentioned this pull request Sep 27, 2020
Drop the redundant/broken ChannelMonitor::get_monitored_outpoints#722
Merged