onion messages

[accumulator]

This PR is ready for review.

• added CLI commands
  • get_blinded_path_via to construct a blinded path
    • only with direct peers as introduction point currently
      • longer routes should be trivial to add
    • optionally add dummy hops to obfuscate true blinded path length
  • send_onion_message to send a text onionmsg_tlv.message payload to a node_id or blinded path
    • receiving electrum node will log these on info level
• route finding over onion message capable nodes using channels as edges
  • tested against c-lightning
  • not tested against electrum peers, as electrum does not publish to the channel graph
  • known issue: valid paths with offline hops are a problem
• direct connect to destination peer/introduction point if no path can be found over channel graph
• OnionMessageManager class implementing queues for rate limiting
  • request-reply queue for matching replies to requests,
  • retries in case no reply within x seconds etc. (not needed for text onionmsg_tlv.message payloads, which are fire-and-forget, but eventually bolt12 will need this for e.g. 'invoice_request')
    • known issue: not trying alternate routes yet
  • forwarding queue, for limiting amount and pacing of onion message forwards
• serialization and deserialization of subtype declarations in wire definitions

Testing

• Testing done with sending onion messages across electrum instances Alice <-> Bob <-> Carol <-> Dave using node_ids and blinded paths as destination.
  • no route finding, see below
  • typical test:
    1. alice get_blinded_path_via $(bob nodeid)
    2. carol send_onion_message <blinded path> hello
• Testing done with sending onion messages and invoice_requests to c-lightning instances Alice <-> cln01 <-> cln02 <-> cln03 <-> Bob
  • with routing finding (so offer with introduction point not a direct peer)
  • reply matching with request (using bolt12 branch to issue invoice_request from offer)

known issues

• route finding over channel graph is unreliable, as nodes can be offline and no alternative routes are tried and no direct peer connection is attempted once a valid but unusable path is found.
  • it should be possible to determine from gossip if an edge is usable
• direct peer connect to dest/introduction point is more reliable, but zero privacy if not using TOR.
• retries in case of request-reply is not using alternative routes yet

[accumulator]

this branch is now onion_messages only, all bolt12 stuff has been moved to the bolt12 branch, which builds on top of this branch/PR

[ecdsa]

A general note on formatting: if we use multiple lines for the parameters passed to a function, we should avoid indenting them at the level of the opening parenthesis, but use an extra line instead. Example:

-                blinded_path = OnionWireSerializer._read_complex_field(fd=blinded_path_fd,
-                                                                       field_type='blinded_path',
-                                                                       count=1)
+                blinded_path = OnionWireSerializer._read_complex_field(
+                    fd=blinded_path_fd,
+                    field_type='blinded_path',
+                    count=1)

Why? because changing the method name will trigger reindentation of all the lines in the first case, not in the second case. This results in larger diffs. See for example the commit where submit_reqrpy is renamed to submit_requestreply

[accumulator]

The current version of get_shared_secrets_along_route seems unnecessarily complicated. We should be very careful about introducing complexity, as it may become difficult to get rid of it down the road.

Yes I don't like it either.

I suggest to rewrite get_shared_secrets_along_route as follows (this is a diff against master, not against this PR):

 def get_shared_secrets_along_route(payment_path_pubkeys: Sequence[bytes],
-                                   session_key: bytes) -> Sequence[bytes]:
+                                   session_key: bytes) -> Tuple[Sequence[bytes], Sequence[bytes]]:
     num_hops = len(payment_path_pubkeys)
     hop_shared_secrets = num_hops * [b'']
+    hop_blinded_node_ids = num_hops * [b'']
     ephemeral_key = session_key
     # compute shared key for each hop
     for i in range(0, num_hops):
         hop_shared_secrets[i] = get_ecdh(ephemeral_key, payment_path_pubkeys[i])
+        hop_blinded_node_ids[i] = get_blinded_node_id(payment_path_pubkeys[i], hop_shared_secrets[i])
         ephemeral_pubkey = ecc.ECPrivkey(ephemeral_key).get_public_key_bytes()
         blinding_factor = sha256(ephemeral_pubkey + hop_shared_secrets[i])
         blinding_factor_int = int.from_bytes(blinding_factor, byteorder="big")
         ephemeral_key_int = int.from_bytes(ephemeral_key, byteorder="big")
         ephemeral_key_int = ephemeral_key_int * blinding_factor_int % ecc.CURVE_ORDER
         ephemeral_key = ephemeral_key_int.to_bytes(32, byteorder="big")
-    return hop_shared_secrets
+    return hop_shared_secrets, hop_blinded_node_ids

This is a lot simpler than the current PR, and I believe this is sufficient. If we need to create a blinded path with an override (for the moment this is only needed in the unit tests AFAICT), then it is easy to do that by calling the above method twice, and concatenate the results:

        hop_shared_secrets1, blinded_node_ids1 = get_shared_secrets_along_route([ALICE_PUBKEY], BLINDING_SECRET)                                                                              
        hop_shared_secrets2, blinded_node_ids2 = get_shared_secrets_along_route([BOB_PUBKEY, CAROL_PUBKEY, DAVE_PUBKEY], BLINDING_OVERRIDE_SECRET)                                            
        hop_shared_secrets = hop_shared_secrets1 + hop_shared_secrets2                                                                                                                        
        blinded_node_ids = blinded_node_ids1 + blinded_node_ids2

Yes this is much better

[accumulator]

Another thing that needs to be improved is the unit tests provided in this PR. We should refrain from copy-pasting long hexadecimal strings in our code. Since there is a json file provided by the RFC, why not use that file directly? Here is a branch where I started to do that: https://github.com/spesmilo/electrum/tree/onion_messages_unittest

merged.

[ecdsa]

Hi, I know that I have already asked this, but could you rebase this branch on current master?

It is currently based on 8f7c11f, which does not pass regtest.
In fact, on that commit, regtest just times out after 1 hour.
As a result, everytime you push in this branch, regtests are failing and unnecessarily consuming cirrus compute credits.

[ecdsa]

TestOnionMessageManager is unacceptably slow (98 seconds here)
When we test timeouts, we do not use the same time constants as on mainnet.
Please have a look at MPP_EXPIRY in test_lnpeer.py, line 149

[ecdsa]

I do not think this pull request should be merged in its current state.

The class OnionMessageManager contains unneeded data structures and event handlers.
I managed to clean up/simplify some of the code in the following branch:

https://github.com/spesmilo/electrum/tree/onion_messages_cleanup

Please use that branch for further pull requests