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

blockchain: Make zero val threshold tuple invalid. #3080

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Mar 10, 2023

This requires #3079.

This makes the zero value for a threshold state tuple act as the invalid state and choice instead of using magic sentinel values.

In practice, callers should be checking the error of any methods before using any other returned values, however, the code typically tries to adopt a defense in depth model where it makes sure unusable/invalid values are also returned in error cases to help make it obvious when a caller incorrectly uses the returned value without checking the error.

It accomplishes this by reording the threshold states so that the invalid state is 0 and converting the choice field to a pointer to the relevant choice as opposed to a choice index. That way the zero value for the overall type is the invalid state and nil the choice, exactly as expected.

In addition to being more ergonomic and inline with typical Go code, it has a few additional benefits:

  • Makes the type harder to misuse
  • Simplifies identification of a specific resulting choice for votes that have multiple affirmative choices since the choice can be checked by its ID instead of a tightly coupled array index
  • Provides an easy method to distinguish between a vote that failed due to a majority vote and one that failed due to expiring before a majority result was achieved

Closes #3078.

@davecgh davecgh added this to the 1.8.0 milestone Mar 10, 2023
@davecgh davecgh force-pushed the blockchain_threshold_state_zero_val_represents_invalid branch 2 times, most recently from 139902f to 2ecfd51 Compare March 13, 2023 18:22
@davecgh davecgh requested a review from jholdstock March 13, 2023 18:23
This makes the zero value for a threshold state tuple act as the invalid
state and choice instead of using magic sentinel values.

In practice, callers should be checking the error of any methods before
using any other returned values, however, the code typically tries to
adopt a defense in depth model where it makes sure unusable/invalid
values are also returned in error cases to help make it obvious when a
caller incorrectly uses the returned value without checking the error.

It accomplishes this by reording the threshold states so that the
invalid state is 0 and converting the choice field to a pointer to the
relevant choice as opposed to a choice index.  That way the zero value
for the overall type is the invalid state and nil the choice, exactly as
expected.

In addition to being more ergonomic and inline with typical Go code, it
has a few additional benefits:

* Makes the type harder to misuse
* Simplifies identification of a specific resulting choice for votes
  that have multiple affirmative choices since the choice can be checked
  by its ID instead of a tightly coupled array index
* Provides an easy method to distinguish between a vote that failed due
  to a majority vote and one that failed due to expiring before a
  majority result was achieved
@davecgh davecgh force-pushed the blockchain_threshold_state_zero_val_represents_invalid branch from 2ecfd51 to 59e1d24 Compare March 14, 2023 03:17
@davecgh davecgh merged commit 59e1d24 into decred:master Mar 16, 2023
@davecgh davecgh deleted the blockchain_threshold_state_zero_val_represents_invalid branch March 16, 2023 14:53
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.

Remove need for invalid choice index sentinal form threshold state tuple.
2 participants