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

new account regn protocol: preregister/payfee #1017

Closed
wants to merge 1 commit into from

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Mar 15, 2021

WARNING: The scope of this PR is extensive. There are a number of TODOs, but it is functional on the positive paths and most of the easy-to-hit edge cases. Ignore the unit tests (for the most part) as there is a lot of updating to do. Account import/export is presently broken because of client DB changes. And generally it is fairly rough.

The primary goal of this PR is to replace the current registration protocol (register/notifyfee) with a new one that involves transmitting the raw fee payment transaction to the server, and only then creating an account. This involves the following new routes: 'preregister' client-originating request to obtain fee payment details given an (optional) specified asset, 'payfee' client-originating request to provide the fee payment transaction, and 'feepaid' (optional) server-originating notification to inform the connected client that the fee txn is confirmed and their account is thus "confirmed". The 'connect' response is also modified to convey "confirmed" status as well.

A secondary goal of this work is to design for fee payments with various assets configurable by the server operator.

Presently I am able to complete registration in the most likely scenarios: no connection or process interruptions, restart dexc and then mine the fee txn, and shutdown dexc and restart it after confirming the fee txn. Each scenario has a different path for ensuring completion of the account confirmation process. There is work toward allowing recovery of failed 'payfee' request that may or may not have created an account server-side, but this is untested. Also, the server needs work to reattempt account confirmation for pending fee transactions on restart; presently it forgets to check the confirmations of pending fee txns.

How to review this PR in it's current state:

  1. Start with server/auth/registrar.go and read the docs for how 'preregister' and 'payfee' are meant to work. The 'preregister' response from the server is signed so that the fee address, amount, and asset details are provably assigned by the server. A successful 'payfee' request creates the account when the txn is validated and accepted by the network. Importantly, this means that "account found" actually means "paid" now, unlike when the legacy 'register' request merely associated an account ID with a fee address to be used. However, the account is not "confirmed" until the tx reaches the required number of confirmations.
  2. See ParseFeeTx in server/asset/dcr/dcr.go for details on what constitutes a valid fee payment transaction. Note that a fee transaction commits to a specific account ID, and as such the "fee address" need not be unique to the account. This commitment also prevents others from claiming the fee payment as their own if it is broadcasted before the 'payfee' request.
  3. See (*ExchangeWallet).MakeRegFeeTx in client/asset/dcr/dcr.go for how the client constructs a Decred fee payment transaction to satisfy ParseFeeTx given the instructions from the 'preregister' response.
  4. Note the new address pool concept used in server/auth, and how server/asset/dcr/addrpool.go implements it for DCR fee payments. Notably, the auth manager may hand out the same address to multiple registrants if there are enough simultaneous pending 'preregister' requests not yet met with 'payfee' requests. Possibility for address reuse is in the design.
  5. See the new (*Core).Register method in client/core/core.go. Skip the "account already exists" handling at first and note the beginnings of support for fee payment in multiple assets as specified by the server's config (feeAsset). Note that the new account data including the raw fee tx is stored in the client's DB after 'preregister' and generation of the fee transaction, but before 'payfee' and broadcasting of the transaction. PayFeeSig is stored after successful 'payfee' request via c.db.AccountPaid, while Confirmed is stored via c.db.ConfirmAccount after required confirmations are reached and a 'feepaid' notification is received or the server's 'connect' response indicates the account is confirmed.
  6. Note that after a successful 'payfee' request but before reaching "confirmed" status, the account ID may 'connect' (authDEX), but not trade, similar to suspended accounts.

Side note about account ID commitment in fee transaction: In a mesh configuration, this may be helpful to tie a transaction to a certain account, although the consensus for defining acceptable amount and payment address is still unclear. I can imagine including the server's 'preregister' signature, but in a mesh anyone can run a server so what exactly does that prove to the other nodes in the mesh? This needs more thought, but the general approach of providing the raw transaction via 'payfee' to actually create an account entry is the goal of this work.

@chappjc chappjc added this to the 0.3 milestone Mar 27, 2021
@chappjc
Copy link
Member Author

chappjc commented Jun 2, 2021

This will be revived in a very different form after #1015 is merged.
The address pool will be dropped and a tl fb scheme will be implemented.

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.

None yet

1 participant