Skip to content

Client side account recovery #335

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.

Sign up for GitHub

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

Jump to bottom
Merged
guggero merged 11 commits into from
Jan 28, 2022
Merged

Client side account recovery #335

guggero merged 11 commits into from
Jan 28, 2022

Conversation

@positiveblue
Copy link
Contributor

@positiveblue positiveblue commented Jan 10, 2022

Enable full client-side recovery of account state.

Using the same LND node as a backend, the process looks like following:

  • The user provides a target (number of accounts to be recovered)
  • The pool client is able to derive a number of potential accounts using the auctioneer public key (hardcoded) and the derived secret (the user is using the same LND node as before)
  • We then need to find the initial state for the accounts that were created. For that, we explore the account space (expiry height and batchKey) and look for a valid account script in each of the LND node transactions.
  • Once all the initial states have been found, we need to get the accounts to their latest state. For that, we need to find account modifications (updates) that trigger an on-chain tx. Again, this process involves searching for the right expiry height.

After the process is complete, the user is able to close the left expired accounts without any server assistance.

@positiveblue positiveblue force-pushed the acc-recovery branch 3 times, most recently from 0ee0ed9 to 9de0a03 Compare January 11, 2022 02:51
@positiveblue positiveblue requested a review from guggero January 11, 2022 03:14
guggero
guggero reviewed Jan 11, 2022
View reviewed changes
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Nice work on this! Exciting to see the last milestone for leaving the alpha stage being so close to completion 🎉

Main comments are about unit test cases and ease of use in regtest.

@positiveblue positiveblue requested a review from guggero January 12, 2022 06:14
@positiveblue positiveblue force-pushed the acc-recovery branch 2 times, most recently from ea7d71b to 006c11c Compare January 12, 2022 23:29
guggero
guggero reviewed Jan 13, 2022
View reviewed changes
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Getting pretty close! Mostly down to style and UX comments now.

Will run a set of manual tests in the next round.

@lightninglabs-deploy
Copy link
Collaborator

lightninglabs-deploy commented Jan 20, 2022

@positiveblue, remember to re-request review from reviewers when ready

@guggero guggero self-requested a review January 21, 2022 09:13
@guggero
Copy link
Contributor

guggero commented Jan 21, 2022

I tested this on regtest today.
I created two accounts, the second one was involved in a batch, then I did a deposit and a withdrawal, both times extending the expiry by 1440 blocks.

The recovery is still running after almost 9 hours, and it looks like it's working on the last transaction:

2022-01-21 14:44:19.681 [DBG] ACCT: found accout with trader key 0308d7767416cbc8f0d8b287fbec4f4f7ab20795241fd609fc12ce30554a37d2ed
2022-01-21 14:46:26.426 [DBG] ACCT: found accout with trader key 020d51947a486b49137b839f8abcdab92d4c6cb958f4a5f55fa8366c9c5ceb9042
2022-01-21 14:50:39.885 [DBG] ACCT: found accout with trader key 020d51947a486b49137b839f8abcdab92d4c6cb958f4a5f55fa8366c9c5ceb9042
2022-01-21 14:52:46.219 [DBG] ACCT: found accout with trader key 020d51947a486b49137b839f8abcdab92d4c6cb958f4a5f55fa8366c9c5ceb9042

Any idea what could be the problem? Doesn't look like this is going to finish.

@positiveblue positiveblue force-pushed the acc-recovery branch 3 times, most recently from ef586e1 to 9ec50cf Compare January 26, 2022 07:33
@positiveblue
poolrpc: add fields to Recovery Requests
34d0365
@positiveblue
rpcserver: split server vs client recovery process
f74b90d
@positiveblue
cmd/pool: user server assisted recovery by default
c8a1aa9
@positiveblue
account: add target to GenerateRecoveryKeys
da2108c 
The default value for server assisted is 500 but the user is able to
give us a hint of how many accounts we are looking for.
@positiveblue
account/recovery: add struct for auctioneer data
7f849b3 
Add a new type (`auctioneerData`) containing the auctioneer data for
each evironment (mainnet, testnet and regtest).
@positiveblue positiveblue force-pushed the acc-recovery branch 2 times, most recently from 7016d18 to 505f541 Compare January 26, 2022 15:59
guggero
guggero reviewed Jan 27, 2022
View reviewed changes
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

Very nice work on this 🎉
We're very very close now, only the TODOs about the account being closed need to be addressed, the rest looks good.
I was able to recover and close my regtest account with this 💯

@positiveblue
poolscript: add RecoveryHelper
7ecc726 
RecoveryHelper is a type that helps speed up account recovery by
caching the tweaked trader and auctioneer keys for faster script
lookups.
@positiveblue positiveblue requested a review from guggero January 28, 2022 07:41
guggero
guggero approved these changes Jan 28, 2022
View reviewed changes
Copy link
Contributor

@guggero guggero left a comment

Choose a reason for hiding this comment

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

tACK, great work on this 💯 I'm very happy about how this turned out!

Pushed a small commit to fix a log message (because chainhash.Hash has a String() method, using %x causes double hex encoding).

@guggero guggero merged commit df3b819 into master Jan 28, 2022
@guggero guggero deleted the acc-recovery branch January 28, 2022 11:29
positiveblue pushed a commit to positiveblue/pool that referenced this pull request Oct 11, 2022
@guggero
Merge pull request lightninglabs#335 from lightninglabs/lnd-14
6615709 
mod: update lnd and pool to latest versions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@guggero guggero guggero approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@positiveblue @lightninglabs-deploy @guggero