Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

draft: forward blinded payments #17

Closed
wants to merge 35 commits into from
Closed

draft: forward blinded payments #17

wants to merge 35 commits into from

Conversation

carlaKC
Copy link
Owner

@carlaKC carlaKC commented Nov 21, 2022

WIP

@github-actions
Copy link

Pull reviewers stats

Stats of the last 30 days for lnd:

User Total reviews Time to review Total comments

@carlaKC carlaKC force-pushed the b12-routeblinding branch 2 times, most recently from 2d82211 to fb3b6cb Compare December 1, 2022 19:36
@carlaKC carlaKC force-pushed the b12-routeblinding branch 3 times, most recently from ce09f3e to 0c44cac Compare December 15, 2022 15:53
@carlaKC carlaKC force-pushed the b12-routeblinding branch 2 times, most recently from 67a4b2e to e27889f Compare December 22, 2022 20:40
Copy link

@calvinrzachman calvinrzachman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool to make a first review pass for route blinding! Nice finesse in hiding the handling of blind hops behind hop.DecodeHopIterators() and the hop.Iterator interface. These were already responsible for decrypting HTLC onions in batches and providing a “hop iterator” from which we obtain all information needed to forward, so it really does make sense to also put the route blinding payload handling there as well. This keeps the channel link free from having to worry about whether the hops it processes are normal or blind. I think that's why we prefer this way at least 😅


return &sphinxHopIterator{
ogPacket: ogPacket,
processedPacket: packet,
blindingKit: blindingKit,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The sphinx hop iterator is responsible for parsing hop payloads. It only makes sense that it also be responsible for parsing route blinding/blind hop payloads, so we thread through the blinding kit. The hop payload we return via the HopPayload() method of the hop.Iterator interface will have the correct "forwarding information" regardless of whether we are processing a normal or blind hop.

// it is from an update_add_htlc tlv (ie,
// not in the paylaod because we have not
// decoded it yet.
false,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we hardcode isIntroduction: false, is this parameter of any use?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO(12/22/22): Recall, this is what we may use later to differentiate between intro/intermediate when handling errors!

// We have restrictions on the types of TLVs that we allow in
// intermediate and final hops. Set a map of allowed TLVs and run a
// check for any other non-nil values in our parsed map.
allowedTLVs := map[tlv.Type]struct{}{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have allowed and required TLVs (ex: intermediate hop must have either next SCID or next node, and details on how the payment is to be forwarded). I can look into adding this as part of my final hop + validation responsibilities!

OutgoingCTLV: cltv,
}
} else {
forwarding, err := blindingKit.forwardingInfo(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We no longer assemble the forwarding information in NewHopPayloadFromReader() using the TLV fields of the top level onion payload. The forwarding info is now computed dynamically based upon whether the hop is normal or blinded. This is why we endow the “blinding kit” with the ability to compute the forwarding info (via function field). It is here where the route blinding payload is decrypted, parsed and used to populate the forwarding information field of the hop.Payload{}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this call overwrite the "active blinding point" with a blinding point override, if one is set in the route blinding TLV payload? No, rather it uses the active blinding point, which comes either from the onion TLV payload or TLV extension to UpdateAddHTLC, to compute the ephemeral blinding point for the next node and stores that inside the returned "forwarding information".

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this call overwrite the "active blinding point" with a blinding point override, if one is set in the route blinding TLV payload?

I did think about adding a setBlindingPoint method on the blindingKit, but I didn't need the blindingKit after this so left it.

}

blinding := s.BlindingKey.blindingPoint.SerializeCompressed()
if _, err := w.Write(blinding); err != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we are not a normal sphinx hop, then we will persist the route blinding point to disk! This will help us demonstrate that we can forward errors back up stream (encrypting with proper blinding point) after restart!

// Use the total aggregate relay parameters for the entire blinded
// route as the policy for the hint from our introduction node. This
// will ensure that pathfinding provides sufficient fees/delay for the
// blinded portion to the introduction node.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so rather than construct the full route piece-meal as the concatenation of two distinct route.Route{} objects, one from <src, intro> + <intro, recp>, and having to post process the concatenation such that the amounts/time-locks match up, we treat the entire blinded route as a single "hop hint" and the existing pathfinding can handle this correctly without much modification. Is that the idea?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we treat the entire blinded route as a single "hop hint" and the existing pathfinding can handle this correctly without much modification. Is that the idea?

Basically, but it's a chain of hop hints rather than a single hint.
Eg for some blinded path: Intro -> B1 -> B2
We have hints: (Intro -> B1 / B1 -> B2)

Comment on lines +102 to +122
FeeBaseMSat: lnwire.MilliSatoshi(
b.RelayInfo.BaseFee,
),
FeeProportionalMillionths: lnwire.MilliSatoshi(
b.RelayInfo.FeeRate,
),

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do we separate the cumulative/aggregate fee for the entire blinded route into base and rate components? Isn't the sender fine to operate knowing a single number for the cumulative fee along the route and a single number for the cumulative timelock along the route?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See MPP previous MPP comment.

Afaik we're going to be given both with the blinded route - the current iterations of the offers PR has the following fields in the B12 invoice:

1. subtype: `blinded_payinfo`
2. data:
   * [`u32`:`fee_base_msat`]
   * [`u32`:`fee_proportional_millionths`]

Comment on lines +3332 to +3343
uint64 aggregate_base_fee_msat = 1;

// The aggregate proportional fee for hops in the blinded portion of the
// route, expressed in micro-satoshis (parts per million).
uint64 aggregate_proportional_fee_ppm = 2;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to provide information on cumulative fees for a blinded route at this level of granularity (ie: separating into base & rate components)? Or would the sender be fine with just a single number representing all of the fees, base and rate, claimed by hops in the blinded portion of a route?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comments (or let me know if I'm missing the question completely!)

htlcswitch/hop/iterator.go Show resolved Hide resolved
htlcswitch/hop/iterator.go Show resolved Hide resolved
@carlaKC carlaKC force-pushed the b12-routeblinding branch 5 times, most recently from 8368b93 to c5edd0e Compare January 5, 2023 21:25
@AndySchroder
Copy link

Wondering, how does this PR relate to lightningnetwork#7267 ? Are they complementary or separate attempts?

Also, how is route blinding going to be usable from LND? I'm assuming you are going to release this without the rest of bolt 12, so will it be inside a normal bolt 11 invoice and then the blinded route part of the route hints? If so, will it be automatically compatible with everything that does BOLT11 invoices, like ZEUS, LNURL, etc.?

@carlaKC
Copy link
Owner Author

carlaKC commented Jan 23, 2023

Wondering, how does this PR relate to lightningnetwork#7267 ? Are they complementary or separate attempts?

This PR depends on 7267, and will be opened against real LND soon ™️

Also, how is route blinding going to be usable from LND?

With 7267, you will be able to use LND's APIs to pay a bolt 12 invoice with a blinded path - though LND thinks that it's part of a regular bolt 11 invoice with route hints. Specifically you would be able to implement the full bolt 12 flow external to LND (as it's not on the roadmap for LL right now) - a half baked example is here (though the repo is stagnant rn waiting for these changes in LND).

I'm assuming you are going to release this without the rest of bolt 12, so will it be inside a normal bolt 11 invoice and then the blinded route part of the route hints?

There won't be any chagnes to bolt 11 invoices/compatibility. There are a few advantages though:

  1. Allows determined users to pay bolt 12 offers using LNDs APIs
  2. Adding support for forwarding blinded payments improves the privacy set for nodes using them, and reliability because more of the network can relay them (~80% is LND at present)
  3. Is an incremental step towards bolt 12 in LND - though not currently on the roadmap for LL at present, even if it was it would need to be cut up somehow regardless.

@AndySchroder
Copy link

Okay, so if the payee wants blinded paths, does the payer need https://github.com/carlaKC/boltnd if they have LND with lightningnetwork#7267 and #17 merged or just the payee, or neither? Wondering if I can just use bolt 11 invoices only to get blinded paths and use it with all the existing infrastructure that does bolt 11. Or, do we require bolt 12 invoices to do blinded paths? Seems like the best way to do blinded paths would be first to get it working with all the bolt 11 infrastructure and then migrate over to bolt 12 invoices slowly. It seems to me like the blinded paths are a greater benefit since a lot of the bolt 12 invoice stuff can be accomplished with LN-URL in the interim.

@carlaKC carlaKC force-pushed the b12-routeblinding branch 5 times, most recently from a294835 to 248ee16 Compare January 27, 2023 22:37
This commit introduces the concept of a blinded payment, which
consists of a blinded path, aggregate parameters and minimal
constraints for the path and basic/sane validation.

Payment relay info and payment constraints are placed in the
record package, because when the time comes to add support for
forwarding of blinded payments, we'll extract these structs
as TLVs in our encrypted onion blob. For now, this is not
important (it's just an easy way to represent this data),
but Aggregate(Relay/Constraints) aliases are used in the
routing package to emphasize that these a blinded payment uses
aggregate versions of the per-hop struct.
This commit adds the encrypted_data and blinding_point tlvs to
the known set of even tlvs for the onion payload. These TLVs are
added in two places (the onion payload and hop struct) because
lnd uses the same set of TLV types for both structs (and they
inherently represent the same thing).

Note: in some places, unit tests intentionally mimic the style
of older tests, so as to be more consistently readable.
This commit introduces a single struct to hold all of the parameters
that are passed to FindRoute. This cleans up an already overloaded
function signature and prepares us for handling requests with blinded
routes where we need to perform some additional processing on our
para (such as extracting the target node from the blinded path).
Add the option to include a blinded route in a route request (exclusive
to including hop hints, because it's incongruous to include both), and
express the route as a chain of hop hints.

Using a chain of hints over a single hint to represent the whole path
allows us to re-use our route construction to fill in a lot of the
path on our behalf.
When we introduce blinded routes, some of our hops are expected
to have zero amounts to forward in their hop payload. This commit
updates our hop fee logic to attribute the full blinded route's
fees to the introduction node. We can't actually know where/how
these fees are distributed, so we collect them all at the
introduction node.
With the addition of blinded routes, we now need to account for the
possibility that intermediate nodes payloads will not have an amount
and expiry set because that information is provided by the recipient
encrypted data blob. This commit updates our payload packing to only
optionally include those fields.
@carlaKC carlaKC force-pushed the b12-routeblinding branch 2 times, most recently from 6126d91 to 2e677dc Compare February 1, 2023 16:39
carlaKC and others added 19 commits February 1, 2023 11:49
We'll need to pack feature vectors for route blinding, so we pull
the encoding/decoding out into separate functions (currently
contained in ChannelType). Though it's more lines of code, we keep
most of the ChannelType assertions so that we strictly enforce
use of the alias.
This commit adds encoding and decoding for blinded route data blobs.
TLV fields such as path_id (which are only used for the final hop)
are omitted to minimize the change size.
Add blinding points to update_add_htlc. This TLV will be set for
nodes that are relaying payments in blinded routes that are _not_
the introduction node.
If there is a blinding point present in update_add_htlc's TLVs, add
it to our payment descriptor struct so that it can be passed through
to onion decryption. This code path will be used for nodes that are
relaying payments inside of the blinded route (ie, all nodes except
the introduction node).

When we restore HTLCs from disk, we do not have the data provided in
UpdateAddHTLC's TLVs available. This is okay, because once we store
the HTLC as part of our commit state, either have the full wire message
stored (for acked but not signed htlcs) or a forwarding package with
all forwarding information has been produced (for fully locked in
htlcs).
This commit adds a workaround for the fact that we don't have our extra
htlc data (ie, that provided in tlvs) stored in channeldb.HTLC. It
ensures that we have the blinding point populated on restart for the
following set of circumstances:
* The incoming HTLC is irrevocably committed to in our local commitment,
  ie: we have received a CommitSig and sent a RevokeAndAck for a
  commitment that includes the incoming HTLC.
* The incoming HTLC is still pending on the sender, ie: we have not yet
  sent the remote party a CommitSig covering the incoming HTLC.

If we restart at this point, the htlc will be stored as an incoming htlc
on our local commitment (with no blinding point) and the full log update
will be saved as a LogUpdate in our remote pending updates (because we
have not yet provided the remote party with a signature).

We restore our remoteUpdateLog from the local commit's incoming htlcs,
so before this change these htlcs would be loaded into our in-memory log
without their blinding point set (and operation would incorrectly resume
as usual without it). This commit updates our logic to restore blinding
points (if set) to the htlcs in the remoteUpdateLog.
When we have payments inside of a blinded route, we need to know
the incoming amount to be able to back-calculate the amount that
we need to forward using the forwarding parameters provided in the
blinded route encrypted data. This commit adds the payment amount
to our DecodeHopIteratorRequest so that it can be threaded down to
payment forwarding information creation in later commits.
Co-authored-by: Calvin Zachman <calvin.zachman@protonmail.com>
Co-authored-by: Calvin Zachman <calvin.zachman@protonmail.com>
This commit introduces a blinding kits which abstracts over the
operations required to decrypt, deserialize and reconstruct forwarding
data from an encrypted blob of data included for nodes in blinded
routes.

The concept of a BlindingKey is separated into a separate struct so
that it can be used independently of the kit (which is specifically used
to process encrypted blobs). This abstraction will be used later to help
determine how we handle errors.
When we have a HTLC that is part of a blinded route, we need to include
the next ephemeral blinding point in UpdateAddHtlc for the next hop. The
way that we handle the addition of this key is the same for introduction
nodes and relaying nodes within the route.
If we received a payload with a blinding point set, our forwarding
information should be set from the information in our encrypted blob.
This behavior is the same for introduction and relying nodes in a
blinded route.
Note: the itest is broken up into multiple commits to make it
more readable, they can be squashed post-review.
We don't support receiving blinded in this PR - just intercept and
settle instead.
@carlaKC
Copy link
Owner Author

carlaKC commented Feb 1, 2023

Okay, so if the payee wants blinded paths, does the payer need https://github.com/carlaKC/boltnd if they have LND with lightningnetwork#7267 and #17 merged or just the payee, or neither?

To pay you'd need lightningnetwork#7267 merged + boltnd.

This PR allows LND nodes to forward blinded payments (which is nice but not required to be able to pay them).

Wondering if I can just use bolt 11 invoices only to get blinded paths and use it with all the existing infrastructure that does bolt 11.

Somebody would need to champion a spec change to add blinded paths to add the field to bolt 11 / deploy that in wallets etc. You'd also need to be able to generated blinded paths.

Seems like the best way to do blinded paths would be first to get it working with all the bolt 11 infrastructure and then migrate over to bolt 12 invoices slowly.

Pretty much every other impl has bolt 12 fully implemented at this point, so they can just start experimenting with them now. But this is why I think LND forwarding blinded payments is important. Since ~80% of the network is LND, you don't really gain much privacy if you can only select from ~20% of the network.

@carlaKC
Copy link
Owner Author

carlaKC commented Feb 1, 2023

Closing this in favor of opening on main LND repo - replacing with a better named branch branch 7298-forwardblindedroutes so this one will fall out of date.

@carlaKC carlaKC closed this Feb 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants