lightningnetwork / lightning-onion

Onion Routed Micropayments for the Lightning Network
MIT License
398 stars 127 forks source link

WIP - sphinx: replay protection and garbage collector #11

Closed Crypt-iQ closed 6 years ago

Crypt-iQ commented 7 years ago

This pull request attempts to address #304 I am in need of comments for this pull request as I'm not sure if I tackled the latter two objectives correctly.

As for the first objective, at what point does the time lock expire "too far" in the future? Should this value be set in the forwarding policy of each node? Or should it be a const?

Regarding the decaying log, I'm not sure if I batched the writes to the database correctly. The boltdb doc page says that db.Batch must be run from a goroutine and concurrent calls to db.Batch are batched together by boltdb transactions. Running db.Batch within a goroutine and actually having concurrent calls to db.Batch would require something like running everything as a goroutine here.

Regarding the garbage collector, I'm not sure what is meant by a "persistent" goroutine.

The updated lnd branch that this branch works successfully with is: https://github.com/Crypt-iQ/lnd/tree/sphinxCompat

This commit introduces persistent replay protection against prior shared secrets from prior HTLC's. A new data structure DecayedLog was created that stores all shared secrets in boltdb and it persists on startup. Additionally, it comes with a set of tests that assert its persistence guarantees. DecayedLog adheres to the newly created PersistLog interface. DecayedLog also comes with a garbage collector that removes expired shared secrets from the back-end boltdb.

Crypt-iQ commented 6 years ago

My garbage collector was previously using CLTV's incorrectly. The latest commit (2f6a89a) fixes that. Upon not retrieving anything, the Get method in DecayedLog will now return math.MaxUint32. I could instead create a new, specific error. Additionally, the tests are also now cleaned up and fixed.

The only question that remains for me is about rejecting time locks deemed too far in the future & whether I need to add a field to the ForwardingPolicy of each node.

I am new to glide, so I may have messed something up there.

Crypt-iQ commented 6 years ago

I was thinking about my garbage collector and how it sweeps up CLTV's and I now understand what is meant by a "persistent goroutine" for the garbage collector. I will update my code so that if the garbage collector is shut down and misses block notifications from the ChainNotifier, it can still delete expired CLTV's. Additionally, I can use a db.Batch call with the garbage collector instead of separating the logic between read & write.

Crypt-iQ commented 6 years ago

c83f9be makes the garbage collector persistent, cleans up tests, and calls boltdb's Batch when possible.

Crypt-iQ commented 6 years ago

Realized I was using CLTV relatively ............ so my future commit will revert this change.

Crypt-iQ commented 6 years ago