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

BSIP47 - Vote Proxies for Different Referendum Categories and explicit voting operation #114

Merged
merged 8 commits into from
Sep 19, 2019

Conversation

xeroc
Copy link
Member

@xeroc xeroc commented Oct 15, 2018

This BSIP comes together with a pull request for a proposed implementation.

It is sponsored by Blockchain Projects BV

@xeroc
Copy link
Member Author

xeroc commented Oct 15, 2018

Questions:

  • Should this operation fail if a proxy is defined?
  • Should setting proxy-account be part of this operation, or not?

@pmconrad
Copy link
Contributor

I think this should better be implemented as extensions on the existing account_update_operation.

Adding a new operation type for functionality that already exists in some other form only leads to confusion and clutters up the operation namespace.

@pmconrad
Copy link
Contributor

Btw, to my knowledge number 47 (and possibly more) has already been assigned to the proposed STEALTH BSIPs. Please coordinate with core team. @ryanRfox ?

@cogutvalera
Copy link
Member

I think this should better be implemented as extensions on the existing account_update_operation.

Adding a new operation type for functionality that already exists in some other form only leads to confusion and clutters up the operation namespace.

Absolutely agree with Peter. IMHO extension makes more sense here for additional feature.

@xeroc
Copy link
Member Author

xeroc commented Oct 16, 2018

Btw, to my knowledge number 47 (and possibly more) has already been assigned to the proposed STEALTH BSIPs. Please coordinate with core team. @ryanRfox ?

I will

I think this should better be implemented as extensions on the existing account_update_operation.

The point here is that this operation separates operational permissions (voting) from administrative permissions (changing keys).
This would, together with BSIP40, even allow to automate voting (e.g. for witnesses) without risking access to the account, or the funds in it.
An extension could surely be used to allow adding/removing of votes without replacing the entire options attribute, but does not decouple operational from administrative permission.

@pmconrad
Copy link
Contributor

The distinction sounds artificial to me. I don't see why that would be an advantage.

BSIP-40 states that active authorities can not only be defined for specific operations, but also that the operation parameters can be restricted. If this is implemented properly, it will be possible to create a "voting key" using account_update_operation that cannot modify authorities.

@sschiessl-bcp
Copy link
Collaborator

sschiessl-bcp commented Oct 17, 2018

The distinction sounds artificial to me. I don't see why that would be an advantage.

It is certainly a facilitation for the user since it allows delta voting (easier to check). Advantage compared to account_update is the massive decrease in payload. Account update option are large and must always be fully given, hard to check and lots of payload.

I understand the argument to avoid duplication, but then I'd rather cut the voting out of account_update. This would be a compatibility nightmare and should be over at least two protocol upgrade with both alongside.

Is the account update options an optional field?

@pmconrad
Copy link
Contributor

Advantage compared to account_update is the massive decrease in payload.

Of course. My point is, the same can be achieved using extensions, without introducing a new operation.

Is the account update options an optional field?

Yes.

@xeroc
Copy link
Member Author

xeroc commented Nov 13, 2018

I agree with Peter, we should migrate the code over to an extension field of account_update_operation. Let's not pollute the operations space

paging @Dimfred

@pmconrad
Copy link
Contributor

@Dimfred @xeroc @sschiessl-bcp are you still working on this?

@Dimfred
Copy link

Dimfred commented Apr 24, 2019

Yes we were. We haven't reviewed it internally yet. I first would have to check the code again and talk to @xeroc and @sschiessl-bcp. But generelly yes.

@pmconrad
Copy link
Contributor

I'm only talking about this BSIP here, not about the implementation.

If you update the PR please remove the change on README.md to avoid the conflict.

@Dimfred
Copy link

Dimfred commented Apr 24, 2019

Ah okay I see. Thought the BSIP is allready out there. Yes Sure we will.

@nathanielhourt
Copy link
Contributor

nathanielhourt commented Jun 18, 2019

Related/possibly redundant with #159, which includes these changes, but proposes them as an extension to account_update_operation

@MichelSantos
Copy link
Contributor

MichelSantos commented Jul 27, 2019

@xeroc @Dimfred I merged the two proposals in such a way that the motivation comes from the multiple proxy proposal (#177), and the technical specifications largely come from the explicit voting proposal. This is one of many possible mergers so please feel free to reject or revise as necessary.

BSIP:       47
Title:      Vote Proxies for Different Referendum Categories
Authors:    Fabian Schuh <https://github.com/xeroc>
            Dmitrij Vinokour <https://github.com/Dimfred>
            Alex Megalokonomos <https://github.com/clockworkgr>
            Michel Santos <https://github.com/MichelSantos>
Status:     Draft
Type:       Protocol
Created:    2018-09-20
Discussion: https://github.com/bitshares/bsips/pull/114

Abstract

Voting power by core token holders may be assigned to different proxies in each of the BitShares referendum categories: witnesses, committee members, and worker proposals.

Motivation

Some proxies can be more or less knowledgeable, wise, and trusted by token holders to vote on certain referendum categories than others. It is therefore desirable to empower token holders to select different proxies for each of the referendum categories.

If the ability to select multiple proxies or to directly vote through a single new operation were made possible, this new operation be used in conjunction with BSIP40 which will allow an account holder to assign a specific key for voting. This combination could simplify voting for many account holders.

Rationale

Holders of the core token of BitShares (BTS) have always had the ability to directly vote on three referendum categories: witnesses, committee members, and worker proposals. They have also had the option of delegating their voting power to another account (a "proxy") to vote on any of these referendum categories.

Rather than delegating voting power to a single proxy for each of three referendum, a core token holder might prefer to select different proxies for each of the referendum categories.

This more granular approach empowers core token holders with more options. A BTS token holder may choose to directly vote on any referendum category including the option to abstain. A BTS token holder may choose to select proxies for each of the referendum categories. Each of the referendum categories may have distinct proxies or may have common proxies. Some referendum categories may have assigned proxies while others may be unassigned. Any referendum category that does not have an assigned proxy permits the voter to vote directly on any referendum in that category. But if a confused core token holder votes for a referendum in a category while also delegating that category to a proxy, the token holder's direct vote shall be ignored.

The introduction of a new operation (account_update_votes_operation) with additional changes to the core shall also enable an account holder to select proxies for different referendum categories, and to vote for referendums without needing to use the existing account_update_operation. The existing operation requires the account holder to pass all possible account settings to avoid overriding existing settings. This is inconvinient and requires a higher payload to be stored on the blockchain which results in the account holder paying a higher fee. The proposed operation shall only update the portion of the account data that is related to voting and should also reduce the fee.

The new operation only needs to include

  • The voting account
  • The desired proxy, if any, for the committee referendum category
  • The desired proxy, if any, for the witness referendum category
  • The desired proxy, if any, for the worker referendum category
  • The quantity of desired witnesses
  • The quantity of desired committee members
  • The list of referendums that are supported

Specifications

Whenever a proxy for a category is assigned, any direct votes by a core token holder within that category shall be ignored. Whenever a proxy for a category is unassigned, any direct votes by a core token holder for that category shall be counted during the vote tallying process of the blockchain. This shall require an upgrade to the protocol.

GRAPHENE_PROXY_PER_CATEGORY_ACCOUNT (New)

A new special account GRAPHENE_PROXY_PER_CATEGORY_ACCOUNT with ID 1.2.6 is defined and added to the database at protocol change activation time.

account_options extension (Existing)

Fields:

The account_options receives three new extensions:

  • optional<account_id_type> committee_voting_account
  • optional<account_id_type> witness_voting_account
  • optional<account_id_type> worker_voting_account

Validation:

  • These extensions must not be present before the protocol change activation date, neither in transactions nor in proposals.
  • If any extension is present, it must contain the ID of an existing account.
  • If any extension is present, account_options.voting_account == GRAPHENE_PROXY_PER_CATEGORY_ACCOUNT

account_object(Existing)

Note: this is an implementation detail and meant to be a hint for later development. The actual implementation may differ.

The account object shall receive these new fields:

  • flat_set<vote_id_type> committee_votes
  • flat_set<vote_id_type> witness_votes
  • flat_set<vote_id_type> worker_votes

account_create_operation and account_update_operation shall be modified to create these sets by filtering the account_options.votes field appropriately. account_update_votes_operation shall also create these sets by filtering the votes_to_add and votes_to_remove fields appropriately.

account_update_votes_operation (New)

The following is an example implementation.

struct account_update_votes_operation : public base_operation
{
	asset fee;

	/// The account to update
	account_id_type account;

	/// New account options
	flat_set<vote_id_type>    votes_to_add;
	flat_set<vote_id_type>    votes_to_remove;

	// A new voting account to set
	optional<account_id_type> committee_voting_account
	optional<account_id_type> witness_voting_account
	optional<account_id_type> worker_voting_account

	// A new number of witness_votes to set
	optional<uint16_t> num_witness;

	// A new number of committee_votes to set
	optional<uint16_t> num_committee;

	// For future extensions (see account_update_operation)
	extensions_type extensions;
};

Changes to direct votes can be specified by means of a list of vote identifiers to be added or removed. This has multiple advantages:

  • Reduces the size of the operation compared to replacing the entire slate
  • Allows to easily compare votes before and after the operation

Validation checks:

  • account must be a valid account ID, must exist, must authorize the operation, must pay fee
  • committee_voting_account, if present, must be a valid account ID, must exist
  • witness_voting_account, if present, must be a valid account ID, must exist
  • worker_voting_account, if present, must be a valid account ID, must exist
  • Validate the existence of each vote to add and remove
  • num_witness, if present, must be greater than or equal to ≥ 0
  • num_committee, if present, must be greater than or equal to ≥ 0

Evaluation:

  • Update the account's account_options.committee_votes, account_options.witness_votes, and account_options.worker_votes by filtering the votes_to_add and votes_to_remove field appropriately
  • Update the account_options.num_witness if witness_voting_account = GRAPHENE_PROXY_TO_SELF_ACCOUNT and if num_witness is present
  • Update the account_options.num_committee if committee_voting_account = GRAPHENE_PROXY_TO_SELF_ACCOUNT and if num_committee is present
  • Update the account's last vote time to the head block time

account_create_operation (Existing)

This operation shall also be updated for backwards compatibility. If account_options.voting_account == GRAPHENE_PROXY_PER_CATEGORY_ACCOUNT, then the account's per-category voting proxies are set to the specified extension if present, or to GRAPHENE_PROXY_TO_SELF_ACCOUNT if absent.
Otherwise, the voting proxy for all three categories is set to account_options.voting_account.

account_update_operation(Existing)

This operation shall also be updated for backwards compatibility. If account_options.voting_account == GRAPHENE_PROXY_PER_CATEGORY_ACCOUNT, then the account's per-category voting proxies are set to the specified extension if present, or remain unchanged if absent.
Otherwise, the voting proxy for all three categories is set to account_options.voting_account.

Vote Tallying

The vote tallying algorithm shall be modified to select the opinion_account per voting category, and use the current account's voting stake only for the opinion account's votes in the respective category.

Other BSIPs that may scale the voting weight of an account, such as by vote decay or by vote staking, can remain compatible with this BSIP by assigning the scaled voting weight to the different voting proxies that may or may not be selected by an account.

Discussion

Performance Impact

There will be a performance impact by account_create_operation, account_update_operation, and account_update_votes_operation due to the filtering if implemented as suggested. This impact should be small overall, because these operations are comparatively rare.

There will also be some overhead for vote tallying. Vote tallying happens only during the maintenance interval so the operational impact should be negligible as well.

Note that the cumulative effect of both may be noticable during chain replay.

Summary for Shareholders

This proposal introduces a means for token holders to select different proxies in each of the BitShares referendum categories: witnesses, committee members, and worker proposals. The proposed mechanism in combination with BSIP40 will permit core token holders to simplify automated voting without compromising the account through the use of a new operation that can be used only for voting.

Copyright

This document is placed in the public domain.

See Also

@MichelSantos
Copy link
Contributor

@xeroc @Dimfred Some of the merger merits your review.

  1. I removed "Setting the proxy is explicitly not part of this opertation. Authors of this BSIP came to the conclusion that proxy settings should require active authority. It is the opinion that setting proxy settings are distinct from voting." in consideration of the merger. Is this acceptable to both of you?

  2. There is a new specification that might conflict with your anticipated uses for BSIP40 for automated voting?

"account must be a valid account ID, must exist, must authorize the operation, must pay fee"

This could maybe be relaxed or changed depending on your intended usage

@MichelSantos
Copy link
Contributor

@pmconrad Regarding

"There will be a performance impact on account_create, account_update, and account_update_votes due to the filtering (if implemented as suggested). This impact should be small overall, because these operations are comparatively rare."

If this is a concern for the account_update_votes operation, we could change

/// New account options
flat_set<vote_id_type>    votes_to_add;
flat_set<vote_id_type>    votes_to_remove;

to be

/// New account options
flat_set<vote_id_type>    committee_votes_to_add;
flat_set<vote_id_type>    committee_votes_to_remove;
flat_set<vote_id_type>    witness_votes_to_add;
flat_set<vote_id_type>    witness_votes_to_remove;
flat_set<vote_id_type>    worker_votes_to_add;
flat_set<vote_id_type>    worker_votes_to_remove;

This will address the performance issue but will add a small amount of complexity to client implementations.

@MichelSantos
Copy link
Contributor

Another question is how should the renewal of a voting slate or the renewal of a proxy be handled? This question is relevant to the vote decay proposal where the latest vote time must be updated. The current logic only updates the time if the voting slate or the proxy changes.

I think that the best two options are:

  • for a voting slate to be changed in a meaningless way in one operation, and to then be reset in a second operation
  • to include an explicit "renew" field in the account_update_votes_operation

I chose the latter option by adding a field called renew_votes into account_update_votes_operation which can be set to true for this purpose.

@pmconrad
Copy link
Contributor

pmconrad commented Jul 29, 2019

This impact should be small overall, because these operations are comparatively rare.

If this is a concern for the account_update_votes operation,

It is not, because the operation is rare.

how should the renewal of a voting slate or the renewal of a proxy be handled?

The proposed "renew" field is redundant - account_update_votes_operation implies "renew", IMO. No actual changes need to be contained in the operation, its presence alone would be sufficient to renew the timer.

@MichelSantos
Copy link
Contributor

how should the renewal of a voting slate or the renewal of a proxy be handled?

The proposed "renew" field is redundant - account_update_votes_operation implies "renew", IMO. No actual changes need to be contained in the operation, its presence alone would be sufficient to renew the timer.

I understand now. The new evaluator for the account_update_votes_operation will not contain the same filtering logic as is found in the account_update_evaluator.do_apply().

The proposed text now reflects the removal of this field.

@xeroc
Copy link
Member Author

xeroc commented Aug 7, 2019

I updated the pull request/commit to use @MichelSantos 's text. I like it!

@xeroc xeroc force-pushed the bsip-account_update_votes_operation branch from d716db1 to b459785 Compare August 7, 2019 13:04
Copy link
Contributor

@ryanRfox ryanRfox left a comment

Choose a reason for hiding this comment

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

@xeroc please remove the README from the file list. I will update that file in a separate PR. Thanks

@pmconrad
Copy link
Contributor

pmconrad commented Aug 7, 2019

I updated the pull request/commit to use @MichelSantos 's text. I like it!

@xeroc please double-check. I can't see your update to the proposal text.

@xeroc xeroc force-pushed the bsip-account_update_votes_operation branch from 6756848 to 480bc96 Compare August 14, 2019 09:51
@xeroc
Copy link
Member Author

xeroc commented Aug 14, 2019

cleaned up. sorry for the delay

bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
@ryanRfox
Copy link
Contributor

It feels like this is very close to "done". @pmconrad are you awaiting anything further from @MichelSantos prior to completing your review? @xeroc are you agreeing with the modifications made thus far?

bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
bsip-0047.md Outdated Show resolved Hide resolved
pmconrad
pmconrad previously approved these changes Sep 19, 2019
@sschiessl-bcp
Copy link
Collaborator

Adjusted the Readme, and added the explicit voting operation in title. Please re-approve.

@ryanRfox ryanRfox merged commit 254fdf6 into master Sep 19, 2019
@abitmore abitmore deleted the bsip-account_update_votes_operation branch November 29, 2019 21:48
@abitmore abitmore changed the title BSIP47 - An explicit voting operation BSIP47 - Vote Proxies for Different Referendum Categories and explicit voting operation Apr 21, 2020
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.

9 participants