nano-node icon indicating copy to clipboard operation
nano-node copied to clipboard

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

Open MajorChump opened this issue 3 years ago • 4 comments

This is the same as https://github.com/nanocurrency/nano-node/pull/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.

MajorChump avatar Feb 11 '22 18:02 MajorChump

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 avatar Feb 12 '22 01:02 dsiganos

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.

dsiganos avatar Feb 12 '22 01:02 dsiganos

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.

zhyatt avatar Feb 14 '22 18:02 zhyatt

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.

theohax avatar Feb 15 '22 11:02 theohax