Fix open channel management (Bolt 2)

[crisdut]

This PR completes all steps to open a channel between lnp-nodes, according to Bolt2, except a initial "refund tx" signature, because I will solve that in other PR.

Also, I will create another PR to fix the connect (bolt1) and open channel (bolt2) operations with other LN implementations because I found issues related a parsing messages.

Changes

• Add ChanelUpdate event to force accept peer change the channel id.
• Allow accept node load your keyset.
• Allow electrum worker track and untrack transactions (only to "zero conf") [1]
• Implements accept peer finish_accepted, finish_signed and finish_funded methods.
• Review propose peer and accept peer lifecycles.

Dependencies

• Fix lifecycle accept node lnp-core#25
• Add missing feature flag (bolt9) lnp-core#24

[1] The current rust-electrum-client does not get transaction with "verbose" informations. I will find a way to get the information. Details here

[codecov-commenter]

Codecov Report

Merging #69 (d5f28d6) into master (35f921e) will decrease coverage by 0.0%.
The diff coverage is 0.0%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master    #69     +/-   ##
=======================================
- Coverage     0.1%   0.1%   -0.0%     
=======================================
  Files          41     41             
  Lines        3273   3355     +82     
=======================================
  Hits            3      3             
- Misses       3270   3352     +82     

Impacted Files	Coverage Δ
src/bus/ctl.rs	0.0% <0.0%> (ø)
src/channeld/automata/accept.rs	0.0% <0.0%> (ø)
src/channeld/automata/mod.rs	0.0% <0.0%> (ø)
src/channeld/automata/propose.rs	0.0% <0.0%> (ø)
src/channeld/runtime.rs	0.0% <0.0%> (ø)
src/lnpd/runtime.rs	0.0% <0.0%> (ø)
src/routed/runtime.rs	0.0% <0.0%> (ø)
src/watchd/runtime.rs	0.0% <0.0%> (ø)
src/watchd/worker.rs	0.0% <0.0%> (ø)
src/opts.rs	0.0% <0.0%> (ø)
... and 19 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[dr-orlovsky]

@crisdut looks interesting! Can you provide some comments which functionality should it add to the node specifically?

[crisdut]

@crisdut looks interesting! Can you provide some comments which functionality should it add to the node specifically?

Hi @dr-orlovsky,

Yes, sure.

For now, I'm adding the final steps to create a channel and correctly persist the state channel. After that, I will finish the track transactions worker on watchd daemon.

Also, I started updating the workflows (here) to help us to review the bolts implementations. I will split each workflow into two flows: internal and external. The first flow is how daemons interact with others along the channel events. The other flow is how the lnp node needs to exchange messages with other peers (independently of the node implementation).

Does it make sense to you?

BTW, I saw your comment here, and I was wondering if it's worth continuing to correct the code or is it better to wait for the re-architecture?

[dr-orlovsky]

I will split each workflow into two flows: internal and external. The first flow is how daemons interact with others along the channel events. The other flow is how the lnp node needs to exchange messages with other peers (independently of the node implementation).

Not sure how this can work, since these two workflows are not independent...

BTW, I saw your comment https://github.com/RGB-WG/rgb-node/discussions/238#discussioncomment-4718147, and I was wondering if it's worth continuing to correct the code or is it better to wait for the re-architecture?

With the new architecture the things which would change are not parts of the business logic; it is just that connections will run on the same thread (while today we have a thread per each remote peer connection). Watchd and channels will remain on an independent threads. So not sure whether it worth waiting for the new architecture, I think parts which you are doing shouldn't be affected by that much.

[crisdut]

With the new architecture the things which would change are not parts of the business logic; it is just that connections will run on the same thread (while today we have a thread per each remote peer connection). Watchd and channels will remain on an independent threads. So not sure whether it worth waiting for the new architecture, I think parts which you are doing shouldn't be affected by that much.

OK sure.

This week I have worked on this issue less than I'd liked. I'm finishing testing this first part. Next week, I'll update this branch again. Thanks for review.

[crisdut]

Hi @dr-orlovsky

I finished the PR, I updated description #69 (comment)

[dr-orlovsky]

I think this is excellent work! Thank you @crisdut!

I see just a single issue which may lead to some notifications from electrum to get lost, which would be nice to fix - and we can merge after.

Also left a couple of nits which are nice-to-have. Otherwise LGTM

[dr-orlovsky]

Amplify with FlagVec fixes is release, as well as LNP Core (v0.9). I am planning to release v0.9 of LNP Node soon. Can you pls update the PR such that I can merge it before doing the release?

[dr-orlovsky]

Looks like we need another iteration due to incorrect panics - plus found a simpler way to avoid electrum message loss

[dr-orlovsky]

ACK d5f28d6