dcrstakepool
dcrstakepool copied to clipboard
decred
deprecate `votingwalletextpub` dcrstakepool config option
The votingwalletextpub is only used for generating a ticket address to pair with a user's wallet address to generate a multisig. #451 moved the multisig creation process to stakepoold but the ticket address is still generated by dcrstakepool using the votingwalletextpub config value. I believe the entire op can be moved to stakepoold (generate the per-user ticket address in stakepoold) and thus retire the votingwalletextpub config option.
This would render part of the work done in #422 obsolete.
Also, with reference to #451, there would be no need to perform the CreateMultisig rpc call on all stakepoold servers if a check is done at dcrstakepool startup to ensure that the votingwalletextpub is the same across all stakepoold instances. This check can be performed without needing to save the votingwalletextpub value somewhere; but rather allow the value to be retrieved for use in stakepoold when the CreateMultisig rpc method is invoked.
With the above note in mind, #422 could then focus on ensuring that the dcrwallet instances running on all stakepoold backends are properly configured (by checking that the votingwalletextpub value is the same across all stakepoold instances). #422 would also continue to check that the coldwalletextpubkey does not belong to any of the stakepoold dcrwallet instances.
I may be reading this wrong, but #422 will deprecate votingwalletextpub.
I may be reading this wrong, but #422 will deprecate votingwalletextpub.
Oops, I meant #422 in my issue desc actually, not #407. Will correct.
Yes, it does deprecate the option from dcrstakepool config but does not completely retire it from dcrstakepool. The approach in #422 so far is to issue rpc calls to stakepoold to retrieve the voting wallet pub key. I'm suggssting we shouldn't even need to do that.
#422 is already a work in the desired direction, hence I meant to reference it as what could be modified to apply the suggestion above. I mis-referenced #407.
I feel like there was a reason @jholdstock wanted to do the multisig on all the wallets. So the voting wallets watched addresses are incremented I think. The xpub is also used when a new user signs up, before they make the multisig our address for them is saved in the db. I guess that could be changed tho.
Ah, I see. What you say about the wallet watched addresses increment makes some sense. I'll look at the code again though to confirm if that's necessary.
I'm not sure the ticket address from the voting wallet is saved before the multisig is created. In any case, we can/should modify the CreateMultisig rpc method implementation to return the ticket address for the user as well, so it can be saved in the database after the multisig is created. Also, the address generation, saving to db and multisig creation is really only done when the user submits his wallet address, not necessarily when a user signs up. When a user does submit his wallet address, the intent is to generate and save the multisig.
Yeah, I was wrong about the xpub being used on sign up. I don't think it matters until the multisig is made.
If there's no need for it, why save it? And I don't see a need for it as it can easily be included in the CreateMultisig stakepoold rpc method. If your point about incrementing watched addresses on all backend wallets holds true, then generating the ticket address when CreateMultisig is invoked on all backend stakepoold instances is even more profitable.
https://matrix.to/#/!HEeJkbPRpAqgAwhXWO:decred.org/$156561569214852DcEGd:decred.org?via=decred.org&via=matrix.org&via=zettaport.com
Calling CreateMultisig on all wallets is useful because it ensures all wallets register all of the multi-sig pubkeys
That makes sense @jholdstock. The goal of this issue then would be to have all stakepoold instances generate the ticket address that is paired with the user's address for creating the multisig, instead of generating it on dcrstakepool then passing the generated address to stakepoold CreateMultisig rpc. Thus, we can retire the votingwalletextpub config option in dcrstakepool.