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

Update confirmation_info RPC call to include representatives final votes for monitoring purposes #3734

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

MajorChump
Copy link
Contributor

This is the same as #3618 without the unrelated commits.

This PR adds a list of representatives_final to the confirmation_info RPC which are representatives which have voted with final votes.

It's backwards compatible. This also adds a check for final_tally in the tests which was already implemented on the RPC previously.

@zhyatt zhyatt added debug Updates assisting with debugging and development efforts rpc Changes related to Remote Procedure Calls labels Feb 11, 2022
@dsiganos
Copy link
Contributor

dsiganos commented Feb 12, 2022

The code changes look good to me except that previously the list of representatives was sorted whereas after this change it will not be sorted anymore. The documentation does not mention sorting but there might be services that empirically learned that it is sorted and expect it to be sorted. So if we want to be backwards compatible, we should sort the list of representatives. If we decide to sort, or not, we should update the documentation to reflect that.
@zhyatt, @clemahieu should we keep the sorted representatives functionality?

However, from a conceptual point of view, this change is not as good as it could be.
Instead of just giving final votes only, we could create a new option called votes, which when specified, could give out the following information:
voting_account_id, timestamp, duration, is_final_flag, election_start_time
I assume here that nano::vote_info::time represents the election start time.
That will give a much more complete picture of what is happening and it will be ready for debugging vote durations when they start to take effect properly.

What do you guys think?

@dsiganos
Copy link
Contributor

dsiganos commented Feb 12, 2022

To clarify, I am saying let's not change the response to representatives option.
Let's create a new option called votes which will return votes and their attributes.

@zhyatt
Copy link
Collaborator

zhyatt commented Feb 14, 2022

I am in favor of the suggestions for a new votes option to enable these additional responses and expanding it to include all vote types and more information about them and the election.

@theohax
Copy link
Contributor

theohax commented Feb 15, 2022

Change looks good, but I also agree that a votes subobject might probably be the best way though especially for future data added to the response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debug Updates assisting with debugging and development efforts rpc Changes related to Remote Procedure Calls
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants