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

Implement tspend policy operations APIs #234

Open
matheusd opened this issue Jan 26, 2021 · 6 comments
Open

Implement tspend policy operations APIs #234

matheusd opened this issue Jan 26, 2021 · 6 comments

Comments

@matheusd
Copy link
Member

Policy choice for tspends can be set on dcrwallet for both a specific pi key (that is, approving or disapproving all tspends signed by some given key) or by individual tspend hash.

This needs to be exposed so that stakeholders can select their preference for when their vote is cast.

At a minimum, users need to be able to set and retrieve the vote choices for a given ticket.

@jholdstock
Copy link
Member

jholdstock commented May 14, 2021

Add optional params to /payFee request

The client is currently setting their consensus vote choices when a ticket is initially registered with the /payFee request. We should add new, optional params to also set tspendPolicy and piKeyPolicy. eg.

...
"tspendPolicy": { "first_tspend_hash": "yes", "second_tspend_hash": "no" },
"piKeyPolicy": { "first_pi_key": "yes", "second_pi_key": "no" },
...

New endpoints for updating tspendPolicy/piKeyPolicy

  • /api/v3/settspendpolicy
  • /api/v3/setpikeypolicy

Using the same request encoding specified above, and the existing mechanisms for ticket authentication and storing request history for accountability.

Additional thoughts

  • Add ticketHash param to treasury RPC calls dcrwallet#2051 needed first
  • Valid values for vote choices are "invalid", "abstain", "yes", "no". Reject anything else (we can probably depend on dcrwallet to validate this, and just return the wallet error if it is descriptive enough)
  • Ensure tspend/pikey policy are being set in all places where consensus vote choices are currently being set, ie:
    • Voting wallet consistency checking
    • In the background handler, when a ticket fee tx is confirmed
  • We don't need to bump API version here because all new fields/endpoints are optional.

@davecgh
Copy link
Member

davecgh commented May 14, 2021

The mostly seems fine, but I don't see a way to disallow a specific pi key (for example, if it gets compromised, etc). Simply not specifying one would result in an abstain as opposed to explicitly voting no.

@jholdstock
Copy link
Member

jholdstock commented May 15, 2021

Typo - I fixed the piKeyPolicy example above

@jholdstock
Copy link
Member

I've started implementing this and rather than creating new endpoints for /setspendpolicy and /settreasurypolicy, I think it makes more sense to add new optional fields to the existing /setvotechoices endpoint.

The name setvotechoices is already generic enough to cover both consensus voting and treasury spend voting. Internally to vspd, it will ease the process of recording vote choice changes. From a wider perspective, it will reduce the amount of network calls required to update choices, which in turn may reduce the amount of times accounts need to be unlocked in dcrwallet to sign requests.

@jholdstock
Copy link
Member

Still waiting for a way to bulk retrieve tspend/treasury policies from dcrwallet for the purpose of consistency checking.

Also identified that tspend and treasury policies could be a DoS vector for VSP servers. If vspd accepts any hash and/or any treasury key, a malicious user could spam loads of them and fill the vspd/wallet DBs.

@jholdstock
Copy link
Member

Still waiting for a way to bulk retrieve tspend/treasury policies from dcrwallet for the purpose of consistency checking.

This is still outstanding, the last thing required before this issue is closed.

Also identified that tspend and treasury policies could be a DoS vector for VSP servers. If vspd accepts any hash and/or any treasury key, a malicious user could spam loads of them and fill the vspd/wallet DBs.

This was mitigated by subjecting tspend/treasury vote changes to the same limit as agenda vote changes. The database will only keep a record of the 10 most recent changes for each ticket.

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

No branches or pull requests

3 participants