RequestSendFundsOp should only contain next nodes on route

Summary

Currently RequestSendFundsOp always contains the full route from the buyer to the seller. To (somewhat) protect the buyer's privacy, we propose to remove the nodes from the beginning of the route. Those nodes are not required for the correct operation of the protocol, and they expose the identity of the buyer.

Example: Consider the following network layout:

B -- C -- D -- E

Currently, a RequestSendFundsOp message from B to E contains the route: B -- C -- D -- E. We denote the contents of the route in the RequestSendFundsOp message received by the following nodes:

B: B -- C -- D -- E.
C: B -- C -- D -- E
D: B -- C -- D -- E.

In other words: when the message is forwarded from B to C, it still contains the full B -- C -- D -- E. Next, when C forwards the message to D, it still contains the full route: B -- C -- D -- E. Finally, when the message arrives at D, is still contains the full route: B -- C -- D -- E. This reveals to E the public key of the buyer: B, hence violating the privacy of B.

Instead, we propose that when a node receives a RequestSendFundsOp message, the message will contain as a route only the next nodes, and not include the node itself and the previous nodes. Continuing the example,

B will receive a RequestSendFundsOp message with the route C -- D -- E.
C will receive a RequestSendFundsOp message with the route D -- E.
D will receive a RequestSendFundsOp message with the route E.

Privacy implications

This modification should provide some sort of privacy for the buyer (But not for the seller). Note that this will not provide "perfect" privacy for the buyer, for the following reasons:

The Index server might know the identity of the buyer.
A few colluding nodes might be able to find out the identity of the buyer.

That said, the proposed modification is a "quick win" for increasing the buyer privacy without too much work.

In the future, if this becomes important enough, we might be able to consider techniques like onion routing, where every node is given only the identity of the next node to forward a RequestSendFundsOp, in encrypted form.

Instructions

The proposed modification does not require changing the structure of RequestSendFundsOp or any serialization code.

The major changes required are:

offst-proto signature buffers
offst-funder logic (The part that deals with incoming and outgoing Request messages).

Part (1) Currently the ResponseSendFundsOp contains the following capnp description:

struct ResponseSendFundsOp {
        requestId @0: Uid;
        destHashedLock @1: HashedLock;
        randNonce @2: RandNonce;
        signature @3: Signature;
        # Signature{key=destinationKey}(
        #   sha512/256("FUNDS_RESPONSE") ||
        #   sha512/256(requestId || sha512/256(route) || randNonce) ||
        #   srcHashedLock || 
        #   destHashedLock || 
        #   destPayment ||
        #   totalDestPayment ||
        #   invoiceId
        # )
        # ...
}

Consider the signature description above (in comment). The signature currently contains sha512/256(route). This part needs to be removed, as with the proposed modification nodes later on the route will not be able to calculate this value. Therefore the route should not be included at all inside the signature. The new signature should be:

   # Signature{key=destinationKey}(
        #   sha512/256("FUNDS_RESPONSE") ||
        #   sha512/256(requestId || randNonce) ||
        #   srcHashedLock || 
        #   destHashedLock || 
        #   destPayment ||
        #   totalDestPayment ||
        #   invoiceId
        # )
        # ...

This new signature form should be updated in all the comments that contain signature description:

funder.capnp in offst-proto
doc/docs/atomic_proposal.md
fuder/signature_buff.rs in offst-proto.

The code that computes this signature could be found in funder/signature_buff.rs in offst-proto. Make sure to modify all the places that calculate the signature. I think that the code in signature_buff.rs contains some kind of code duplication for the computation of the signature. If you can refactor this part while you are there it could be an extra win.

After this part is finished, cargo test should work correctly. At this point the full route is still forwarded in the tests, but the signature does not include the full route any more.

Part (2) offst-funder logic

I think that the files that require modification are:

funder/src/mutual_credit/incoming.rs (process_request_send_funds()).
(I'm almost sure outgoing.rs does not require any modification.)
funder/src/handler/handle_friend.rs (Cutting one node from the route)
funder/src/handler/handle_control.rs (Cutting one node from the route)

Stuff that should be changed:

During verification: Instead of searching ourselves and our friend on the route, assume that we are the first on the route (But not included), and the friend showed up earlier (also not included). For example, if the full route is B -- C -- D -- E -- F and we are C, we should only see D -- E -- F.
Removing the first node from the route at handle_friend.rs. This should be done at the same place where RequestSendFundsOp.left_fees is decreased, at the end of handle_request_send_funds().
Removing the first node from the route given at control_create_transaction_inner() in handle_control.rs right before we push the request into the pending user requests queue. We need to do this because now the rest of the funder code now expects routes that do not begin with our own public key. Instead, the route should begin with the public key of the next node.

Other notes

The Funder's code is a bit strange, because any mutation done on the Funder data must be atomic. This is intentional.
There is reasonably good test coverage for the code mentioned above, so the tests should help you be sure you are on the right track. Integration tests are really slow, and you don't need to run them all the time. You can run only the relevant tests for a specific crate, like this: crate test -p offst-funder.
Be sure to send me a message if things get more complicated than the scope described in this issue.

freedomlayer / offset