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

rpc: Add TSpendPolicies and SetTSpendPolicy requests #2172

Merged
merged 2 commits into from
Nov 28, 2022
Merged

Conversation

bgptr
Copy link
Contributor

@bgptr bgptr commented Jul 25, 2022

This will allow decrediton to now get and update the pending tspend policies.

Notes:

  • SetTSpendPolicy does not update VSPs about the vote choices, but decrediton takes care of it by calling SetVspdVoteChoices, just like when voting to the consensus changes. This applies to SetTreasuryPolicy too. Closes [treasury spending] Changing Voting for preference to Yes give error decrediton#3763
  • SetVspdVoteChoices function updates all treasuryPolicies and tspendPolicies now and not just the ones that related to a particular ticket.

+ remove the updateVSPVoteChoices function call from TreasuryPolicies and SetTSpendPolicy and in SetVspdVoteChoices function update all treasuryPolicies and tspendPolicies and not just the ones that related to a particalar ticket
Copy link
Member

@matheusd matheusd left a comment

Choose a reason for hiding this comment

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

Working as expected (as tested using grpcui and calling the individual functions). I'll test it in decrediton once we have a new tspend in either testnet or mainnet.

Comment on lines 3401 to 3402
// If a VSP host is configured in the application settings, the voting
// preferences will also be set with the VSP.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't appear to be the case. Besides, we will want to use the VSP that is associated with each live ticket to be updated, and not the one in the current application settings.

Copy link
Member

Choose a reason for hiding this comment

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

looks like this is being handled by another rpc call, so this comment can just be nuked and then this looks ok to me.

@jrick jrick merged commit 7352c1c into decred:master Nov 28, 2022
@bgptr bgptr deleted the tspend branch November 28, 2022 20:52
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.

[treasury spending] Changing Voting for preference to Yes give error
3 participants