amp: introduce child preimage and hash derivation #4162

cfromknecht · 2020-04-07T19:25:57Z

WIP since spec is still in flux, primarily looking for feedback on the interface.

Roasbeef

Solid start! Still need to wrap my head around some of the nuance w.r.t splits/children, but I dig the overall abstraction level.

amp/child.go

Roasbeef · 2020-04-14T01:54:08Z

amp/share_desc.go

+	// max depth 32. Each bit, starting with the least significant,
+	// represents a left (0) or right (1) traversal. The traversal ends
+	// after processing Depth bits.
+	Path uint32


What are the advantages of this approached compared to a "linear" scheme? By linear here I mean just fetching random values and XOR'ing them incrementally with the actual root seed.

In this case, all the leaves will XOR to uncover the actual root leaf?

Or I guess the value here is in deterministic share derivation with an elkrem-like structure?

by linear scheme i'm assuming you mean something like:

share1 = r1

share2 = r2

...

share_n = root ^ r1 ^ ... ^ r_{n-1}

the primary issue is that the shares can succeed/fail independently. say you've sent out n shares, share_n succeeds but share_2 fails. you can no longer update share_n to include another random value.

the construction is much simpler when there is no "distinguished" share, and just use recursion on state that is local to that shard (you only need the parent value to split). in fact, the only thing you can do in the example of above is to split share_2 into share_2l and share_2r, such that share_2 = share_2l ^ share2r. this process can continue no matter how many time one needs to split.

amp/share_desc.go

amp/sharing.go

amp/derivation_test.go

joostjager

I am re-thinking the rationale for this type of multi-shard payments. What is exactly the benefit over sending multiple keysend payments that pay to the same payment address? (Possibly extended with a set id to make recurrent payments possible.)

With multiple keysends, the receiver can pull an incomplete set. But why is it important to prevent that? With xor-amp, it is still possible for the receiver to pull an incomplete set once all shards have arrived.

Something that is a difference, is that the receiver cannot pull if the set never becomes complete. But isn't this a feature that is only relevant in case the network is unreliable and incomplete sets happen frequently enough? Is that worth adding a new payment type? Even if incomplete sets happen often, why would the receiver pull the incomplete set?

amp/child.go

joostjager · 2020-06-11T12:15:05Z

Parking this idea here:

Allow an external share to be part of the AMP set. So once all the htlcs have arrived, the receiver still needs to obtain one more share to find out the root secret. This last share could for example be the preimage of an on-chain reveal.

Roasbeef

Ready to land IMO so we can move forward with the other components, just one question to solidify my understanding of one of the design decisions re determinism or not.

amp/sharer.go

Roasbeef · 2021-01-20T22:21:48Z

amp/sharer.go

+	return children
+}
+
+// split splits a share into two random values, that when XOR'd reproduce the


Love this, super simple yet extremely flexible!

Roasbeef · 2021-01-20T22:23:29Z

amp/sharer.go

+func split(parent *Share) (Share, Share, error) {
+	// Generate a random share for the left child.
+	var left Share
+	if _, err := rand.Read(left[:]); err != nil {


Just to double check, the rationale for not instead using the root share as the seed to a CSPRNG here is that we don't retry failed payments on start up, so it doesn't matter that these splits are deterministic?

Yes, we currently don't resume MPP payments on restarts and instead just wait for them to be cancelled. The plan is to keep this for AMP as well, which means we don't need to be able to rederive any of the secret values.

FWIW the splits could still use an RNG if we choose to store them all, the deterministic derivation was just an attempt at minimizing the storage requirements and minimize how much randomness we consume.

amp/derivation_test.go

guggero

Looks good to me, great work 💯
Excited to see movement on the AMP front.

Very clever with the binary tree construction, also reads very well with the abstractions!

I have two questions, mostly to get some more context on the scheme (non-blocking to this PR):

Let's say we created 4 shares by splitting the root and then both left and right again and assigning them the indices 0 to 4 (0=left_left, 1=left_right, 2=right_left, 3=right_right). Now share 2 fails and we need to split further. How would we continue with assigning the indices? Would we just use a counter and for example assign 4 and 5 to the new children of 2 (and discard index 2 itself)?
How does the receiver know whether they have received all shares? Just by continually checking if XORing all shares together and plugging it into SHA results in the preimage/payment hash?

amp/child.go

guggero · 2021-01-25T08:40:57Z

amp/child.go

+// DeriveChild computes the child preimage and child hash for a given (root,
+// share, index) tuple. The derivation is defined as:
+//
+//   child_preimage = SHA256(root || share || be32(index)),


nit/reminder: the inclusion of the share in the child preimage is not yet mentioned/updated in lightning-rfc#658.

yeah the AMP rfc is out of date with this PR, hoping to get to that this week

amp/sharer.go

amp/derivation_test.go

cfromknecht · 2021-01-26T20:36:19Z

Looks good to me, great work 💯
Excited to see movement on the AMP front.

Very clever with the binary tree construction, also reads very well with the abstractions!

🙏🙏🙏

Let's say we created 4 shares by splitting the root and then both left and right again and assigning them the indices 0 to 4 (0=left_left, 1=left_right, 2=right_left, 3=right_right). Now share 2 fails and we need to split further. How would we continue with assigning the indices? Would we just use a counter and for example assign 4 and 5 to the new children of 2 (and discard index 2 itself)?

The child_index is not related to the position within the tree or total number of HTLCs sent, it's primary purpose is to be able to rerandomize the payment hash/preimage in the event an HTLC with a particular share fails and we don't want to split yet.

This can be done either by incrementing or, preferably, by picking values at random. An example, here just starting at 0 and incrementing for each fail, would be:

(0, A^B) route1 --> FAIL
(1, A^B) route2 --> FAIL
split into (A, B)
(0, A) route3 --> SUCCESS
(0, B) route4 --> FAIL
(1, B) route5 --> FAIL
(2, B) route5 --> SUCCESS

How does the receiver know whether they have received all shares? Just by continually checking if XORing all shares together and plugging it into SHA results in the preimage/payment hash?

In addition an amp_record the sender also includes an mpp_record which includes total_amt_msat and payment_addr. Once total_amt_msat has been received in HTLC set identified by (payment_addr, set_id), the receiver XORs all the shares using ReconstructChildren and checks whether or that the child hash/preimages satisfy all of the accepted HTLCs in the (payment_addr, set_id) HTLC set. If so, it settles the HTLC set and otherwise fails it.

cfromknecht · 2021-01-26T20:39:31Z

Allow an external share to be part of the AMP set. So once all the htlcs have arrived, the receiver still needs to obtain one more share to find out the root secret. This last share could for example be the preimage of an on-chain reveal.

@joostjager great idea, I can make an issue for HODL-AMP!

Roasbeef

LGTM 🐲

guggero · 2021-01-28T07:48:27Z

The child_index is not related to the position within the tree or total number of HTLCs sent, it's primary purpose is to be able to rerandomize the payment hash/preimage in the event an HTLC with a particular share fails and we don't want to split yet.

Makes sense, thanks for the explanations, @cfromknecht!

Roasbeef added this to the 0.11.0 milestone Apr 8, 2020

Roasbeef added amp crypto Related to the cryptography underlying LND payments Related to invoices/payments v0.11 labels Apr 8, 2020

Roasbeef requested review from Roasbeef and joostjager April 14, 2020 02:00

Roasbeef reviewed Apr 14, 2020

View reviewed changes

joostjager reviewed Apr 15, 2020

View reviewed changes

amp/child.go Show resolved Hide resolved

cfromknecht added v0.12 and removed v0.11 labels Jun 23, 2020

cfromknecht removed this from the 0.11.0 milestone Jun 23, 2020

Roasbeef added this to the 0.12.0 milestone Aug 12, 2020

Roasbeef modified the milestones: 0.12.0, 0.13.0 Oct 1, 2020

Roasbeef requested review from carlaKC and joostjager and removed request for joostjager January 13, 2021 00:25

cfromknecht changed the title ~~[WIP] amp: introduce child preimage and hash derivation~~ amp: introduce child preimage and hash derivation Jan 13, 2021

Roasbeef requested review from guggero and removed request for carlaKC January 13, 2021 05:01

cfromknecht force-pushed the amp-keys branch from a093d3f to e8078f9 Compare January 15, 2021 22:52

cfromknecht requested a review from Roasbeef January 15, 2021 23:11

Roasbeef reviewed Jan 20, 2021

View reviewed changes

cfromknecht force-pushed the amp-keys branch from e8078f9 to 49cba6f Compare January 22, 2021 20:04

cfromknecht requested a review from Roasbeef January 22, 2021 20:04

guggero approved these changes Jan 25, 2021

View reviewed changes

cfromknecht added 3 commits January 26, 2021 12:43

amp/child: add Child deriviation

0144bdb

amp/sharer: add recursive sharing logic

cb47d0c

amp/derivation_test: add tests

0ef04a7

cfromknecht force-pushed the amp-keys branch from 49cba6f to 0ef04a7 Compare January 26, 2021 20:45

cfromknecht added v0.13 and removed v0.12 labels Jan 28, 2021

Roasbeef approved these changes Jan 28, 2021

View reviewed changes

Roasbeef merged commit d10d1cb into lightningnetwork:master Jan 28, 2021

cfromknecht deleted the amp-keys branch January 28, 2021 00:25

cfromknecht mentioned this pull request Apr 29, 2021

routing+invoices: implement AMP secret sharing and combination #3939

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

amp: introduce child preimage and hash derivation #4162

amp: introduce child preimage and hash derivation #4162

cfromknecht commented Apr 7, 2020 •

edited

Loading

Roasbeef left a comment

Roasbeef Apr 14, 2020

Roasbeef Apr 14, 2020

cfromknecht Jan 15, 2021

joostjager left a comment

joostjager commented Jun 11, 2020

Roasbeef left a comment

Roasbeef Jan 20, 2021

Roasbeef Jan 20, 2021

cfromknecht Jan 22, 2021

guggero left a comment

guggero Jan 25, 2021

cfromknecht Jan 26, 2021 •

edited

Loading

cfromknecht commented Jan 26, 2021 •

edited

Loading

cfromknecht commented Jan 26, 2021

Roasbeef left a comment

guggero commented Jan 28, 2021

amp: introduce child preimage and hash derivation #4162

amp: introduce child preimage and hash derivation #4162

Conversation

cfromknecht commented Apr 7, 2020 • edited Loading

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef Apr 14, 2020

Choose a reason for hiding this comment

Roasbeef Apr 14, 2020

Choose a reason for hiding this comment

cfromknecht Jan 15, 2021

Choose a reason for hiding this comment

joostjager left a comment

Choose a reason for hiding this comment

joostjager commented Jun 11, 2020

Roasbeef left a comment

Choose a reason for hiding this comment

Roasbeef Jan 20, 2021

Choose a reason for hiding this comment

Roasbeef Jan 20, 2021

Choose a reason for hiding this comment

cfromknecht Jan 22, 2021

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

guggero Jan 25, 2021

Choose a reason for hiding this comment

cfromknecht Jan 26, 2021 • edited Loading

Choose a reason for hiding this comment

cfromknecht commented Jan 26, 2021 • edited Loading

cfromknecht commented Jan 26, 2021

Roasbeef left a comment

Choose a reason for hiding this comment

guggero commented Jan 28, 2021

cfromknecht commented Apr 7, 2020 •

edited

Loading

cfromknecht Jan 26, 2021 •

edited

Loading

cfromknecht commented Jan 26, 2021 •

edited

Loading