-
Notifications
You must be signed in to change notification settings - Fork 75
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
add /purchaseticket API endpoint for accountless ticket purchase #515
add /purchaseticket API endpoint for accountless ticket purchase #515
Conversation
19cbba1
to
2819207
Compare
4934d04
to
145a270
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's kind of hard to get the entire workflow into my head.
It might help if you could open an issue explaining everything, maybe make some sort of visual aid? Decrediton will have to use this api, maybe get their opinion?
One concern I have is that it leaves cli users out of the loop. What about them? I can't say that this closes #274, or it's partner #291 without accounting for them.
And some things to consider going forward are that this area may change in a big way. We want to start producing a new redeem script for every ticket, and a new fee address.
The fee addresses has an issue going #504
The redeem script not yet. However the consensus is that we need the client to be able to reproduce the script. So, in the end the pool will need to import some more keys for every user, in order to make unique script hashes that are reproduce-able by the user.
After that the pool fee will need to be paid out-of-band, in order to get coinjoin working according to jrick.
But for now this does look like a good system to me. Even after we get more privacy, a txid will still be a txid and the reward address will still be visible. Right? I still need to actually test this and the other, but if they work, I think this is progress in the right direction.
controllers/main.go
Outdated
err = controller.StakepooldUpdateUsers(dbMap) | ||
if err != nil { | ||
log.Warnf("failure to update users: %v", err) | ||
if purchaseInfo := controller.generateTicketPurchaseInfo(dbMap, user, userPubKeyAddr); purchaseInfo != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer this fail on error like most other stuff, but maybe just me.
controllers/main.go
Outdated
return controller.Address(c, r) | ||
} | ||
func (controller *MainController) generateTicketPurchaseInfo(dbMap *gorp.DbMap, user *models.User, | ||
userPubKeyAddr string) *poolapi.PurchaseInfo { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And would prefer this also returned an error, like must other stuff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to your previous comment, will implement. What I did was mostly a refactor of existing code and I noticed that some errors were not being logged, but returning an error will cause all errors to be logged - which is better I think.
Decrediton can use the API for sure, it's pretty similar to what Decrediton currently does, only that an API key isn't necessary. As described here decred/decrediton#280, to connect a vsp, a user provides the vsp host address and API Key. When a user submits these,
It's worth noting that Decrediton has to authorize each of the above API using a valid API key; and to get this API key, the user first needs to create an account on the vsp frontend using an email address. With this new API endpoint, Decrediton would no longer need an API key, just the vsp host address. Without requiring an API key, a user absolutely does not need to create an account from the vsp frontend and can purchase tickets directly from Decrediton. Additionally, Decrediton does not need to make multiple API calls to get the information it needs to purchase tickets. This new API endpoint, both submits the user's pubkey address and returns the data needed by a client app such as Decrediton to complete the ticket purchase. Also, with this new API endpoint, Decrediton can purchase multiple tickets without reusing voting or vsp fee addresses. Each purchase can begin with generating a pubkey address from the user's wallet, submitting that address to this new API endpoint which would return a new set of voting and vsp fee address that can be used for ticket purchase. Of course, Decrediton can, if it prefers, chose to save the first voting + vsp fee addresses and reuse those for subsequent ticket purchases.
That's correct. Wasn't sure if it is necessary to provide support for cli users, if that is the case, such provision can be made. Cli support is currently possible because of the vsp frontend where users can submit their pubkey address and retrieve their multisig script+voting address+fee address.
Can't say this resolves #291 as #291 appears to refer to vsp login from the website. It does close #274 since with this PR, tickets can be purchased without using an email. #514 makes it possible to set voting preferences for tickets purchased this way also without requiring email. Email becomes optional - users can still chose to create accounts from the vsp frontend by providing an email address or use a client app like Decrediton to directly access vsp features without creating an account on the vsp frontend.
I agree with that last bit. This PR in no way prevents further work especially wrt adding the ability to (re)produce the redeem script on both the client and vsp end and improvements relating to payments of vsp fees. What this PR does is simply make it possible for users to interact with/use vsp features without NEEDING to use an email address to create a user account on the vsp site. |
03b155e
to
d8700a7
Compare
I've made a consumer of this api endpoint: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a couple comments, but this is working great for me.
Email: userPubKeyAddr, | ||
EmailVerified: 0, | ||
VoteBits: 1, | ||
VoteBitsVersion: int64(controller.voteVersion), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps it would be best to put in some random password? Just to be safe and sure noone can access this account through http...
(1) Will (2) Is (3) How voting prefs are set and changed? Can you provide or point to examples of all requests and responses? (4) In the current system the communication with the VSP is done once via the browser, and later the user just broadcasts VSP ticket transactions and the VSP detects them. In the new system, each new pubkeyaddr requires communicating with the VSP. Forcing users to use the website is bad anyway (I'd like to do everything without browsers), but if new address is used for each tickets, using the website simply becomes infeasible. This means changes to dcrwallet or a separate staking program is required for CLI users. I would prefer the latter to avoid introducing VSP protocols into dcrwallet. (5) In case new address is used for each ticket purchase, redeem script returned for each ticket needs to be backed up (!) by the client and imported into his wallet. This will require automation and new features on the client side - either changes to dcrwallet (including commands to export and import an ever growing bundle of redeem scripts) or a standalone program. Server side also needs tools to backup/restore all the redeem scripts. Without these, the new APIs will be unusable (in no address reuse mode). Is that correct?
Absolutely necessary. The approach taken everywhere else in Decred is that CLI support is added first, and then some of that is reused in GUIs. Also note that "real" CLI support means all important operations can be done without ever using the ugly web browsers.
Per the above, ideally it refers to all kinds of access including non-website. |
The whole problem with redeem scripts outlined in (5) goes away if decred/dcrwallet#1613 is implemented. |
d8700a7
to
ba640f1
Compare
Long time no see! Thank you for continuing this! I have started adding tests to main.go. It would be great if you could add tests for generateTicketPurchaseInfo. It should be done on top of #602 There you should have all the tools you need to set up a mock stakepoold and sql. I also still think you need to throw a random password in there for the new accountless users. |
One concern. Right now new users have the hurdle of a captcha, and then email verification, that supposedly prevents automation of creating new users. With this change, what is to prevent a hostile user from creating new users until we hit max users? Suddenly hitting max users would be pretty catastrophic for the stakepool owner, and the attacker could continue if they up the max. Eventually the database would take up lots of space, and poolfee and voting addresses could be exhausted. Either we need a puzzle here, or not save these users until they buy a ticket, which creates difficulties, because then how do we watch for tickets unless we watch even bogus users? Or we need to be informed of ticket purchases by these users, then confirm that they have indeed bought said ticket before we add and watch them. Even if they are all good users, this may bring us to max users much faster if users make a new account for every ticket. |
How does this work compare to #574 ? Is there any cooperation? (I'm concerned about wasted efforts) |
This is a PR and #574 is an issue |
0a0b1f3
to
60e30c7
Compare
Closed in favour of #625 |
This introduces a new API endpoint to make ticket purchase possible without user signup/login.
This PR paired with #514 resolves #274 (also resolves decredcommunity/issues#100) - emails become optional and vsp services such as purchasing tickets (implemented here) and setting voting preferences (implemented in #514) can be used without creating user accounts on the vsp.
Accountless ticket purchase is done by making a POST request to
api/{v1 or v2}/purchaseticket
, passingUserPubKeyAddr
in post body. This doc describes how to generate apubkeyaddr
usingdcrwallet
anddcrctl
.Calling the new
/purchaseticket
endpoint with a validpubkeyaddr
returns the following (sample) data. Calling the endpoint with the samepubkeyaddr
multiple times returns the same data. The message field in the response will readAPIPurchaseTicket: purchaseinfo retrieved for existing userPubKeyAddr
.The redeem script returned with the API response should be imported into the purchasing wallet so you can recover your funds and vote in the unlikely event of a VSP failure.
The ticket address, fee address and fee percentage should be used to create and broadcast the ticket purchase transaction. Read how to do this with dcrwallet+dcrctl.
If you subsequently restore your wallet from seed, redeem scripts for "vsp tickets" would need to be (re)imported. Currently, a user can login to the vsp server to retrieve the redeem script. Since tickets purchase using this new API do not have user account login, redeem scripts can be retrieved by issuing an API call to
/getpurchaseinfo
using the ticket authentication scheme introduced in #514.