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.

Sign up for GitHub

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

api: Fix vote status computation. #5228

Merged
merged 1 commit into from
Mar 30, 2023
Merged

api: Fix vote status computation. #5228

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/goal/messages.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ const (
infoNodeShuttingDown = "Algorand node is shutting down..."
infoNodeSuccessfullyStopped = "The node was successfully stopped."
infoNodeStatus = "Last committed block: %d\nTime since last block: %s\nSync Time: %s\nLast consensus protocol: %s\nNext consensus protocol: %s\nRound for next consensus protocol: %d\nNext consensus protocol supported: %v"
infoNodeStatusConsensusUpgradeVoting = "Consensus upgrate state: Voting\nYes votes: %d\nNo votes: %d\nVotes remaining: %d\nYes votes required: %d\nVote window close round: %d"
infoNodeStatusConsensusUpgradeVoting = "Consensus upgrade state: Voting\nYes votes: %d\nNo votes: %d\nVotes remaining: %d\nYes votes required: %d\nVote window close round: %d"
infoNodeStatusConsensusUpgradeScheduled = "Consensus upgrade state: Scheduled"
catchupStoppedOnUnsupported = "Last supported block (%d) is committed. The next block consensus protocol is not supported. Catchup service is stopped."
infoNodeCatchpointCatchupStatus = "Last committed block: %d\nSync Time: %s\nCatchpoint: %s"
Expand Down
6 changes: 5 additions & 1 deletion cmd/goal/node.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,7 @@ func makeStatusString(stat model.NodeStatusResponse) string {
upgradeVotesRequired := uint64(0)
upgradeNoVotes := uint64(0)
upgradeYesVotes := uint64(0)
upgradeVoteRounds := uint64(0)
if stat.UpgradeVotesRequired != nil {
upgradeVotesRequired = *stat.UpgradeVotesRequired
}
Expand All @@ -491,11 +492,14 @@ func makeStatusString(stat model.NodeStatusResponse) string {
if stat.UpgradeYesVotes != nil {
upgradeYesVotes = *stat.UpgradeYesVotes
}
if stat.UpgradeVoteRounds != nil {
upgradeVoteRounds = *stat.UpgradeVoteRounds
}
statusString = statusString + "\n" + fmt.Sprintf(
infoNodeStatusConsensusUpgradeVoting,
upgradeYesVotes,
upgradeNoVotes,
upgradeNextProtocolVoteBefore-stat.LastRound,
upgradeVoteRounds-upgradeYesVotes-upgradeNoVotes,
Copy link
Contributor

Choose a reason for hiding this comment

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

yes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

upgradeVoteRounds was calculated in algod so this more direct formula can be used in the client

upgradeVotesRequired,
upgradeNextProtocolVoteBefore,
)
Expand Down
2 changes: 1 addition & 1 deletion daemon/algod/api/algod.oas2.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3979,7 +3979,7 @@
"type": "integer"
},
"upgrade-vote-rounds": {
"description": "Total voting ounds for current upgrade",
"description": "Total voting rounds for current upgrade",
"type": "integer"
}
}
Expand Down
6 changes: 3 additions & 3 deletions daemon/algod/api/algod.oas3.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -631,7 +631,7 @@
"type": "boolean"
},
"upgrade-vote-rounds": {
"description": "Total voting ounds for current upgrade",
"description": "Total voting rounds for current upgrade",
"type": "integer"
},
"upgrade-votes": {
Expand Down Expand Up @@ -4727,7 +4727,7 @@
"type": "boolean"
},
"upgrade-vote-rounds": {
"description": "Total voting ounds for current upgrade",
"description": "Total voting rounds for current upgrade",
"type": "integer"
},
"upgrade-votes": {
Expand Down Expand Up @@ -4902,7 +4902,7 @@
"type": "boolean"
},
"upgrade-vote-rounds": {
"description": "Total voting ounds for current upgrade",
"description": "Total voting rounds for current upgrade",
"type": "integer"
},
"upgrade-votes": {
Expand Down
178 changes: 89 additions & 89 deletions daemon/algod/api/server/v2/generated/data/routes.go
View file
Open in desktop

Large diffs are not rendered by default.

176 changes: 88 additions & 88 deletions daemon/algod/api/server/v2/generated/experimental/routes.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion daemon/algod/api/server/v2/generated/model/types.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

242 changes: 121 additions & 121 deletions daemon/algod/api/server/v2/generated/nonparticipating/private/routes.go
View file
Open in desktop

Large diffs are not rendered by default.

240 changes: 120 additions & 120 deletions daemon/algod/api/server/v2/generated/nonparticipating/public/routes.go
View file
Open in desktop

Large diffs are not rendered by default.

246 changes: 123 additions & 123 deletions daemon/algod/api/server/v2/generated/participating/private/routes.go
View file
Open in desktop

Large diffs are not rendered by default.

186 changes: 93 additions & 93 deletions daemon/algod/api/server/v2/generated/participating/public/routes.go
View file
Open in desktop

Large diffs are not rendered by default.

21 changes: 12 additions & 9 deletions daemon/algod/api/server/v2/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -793,26 +793,29 @@ func (v2 *Handlers) GetStatus(ctx echo.Context) error {
CatchpointAcquiredBlocks: &stat.CatchpointCatchupAcquiredBlocks,
}

nextProtocolVoteBefore := uint64(stat.NextProtocolVoteBefore)
var votesToGo int64 = int64(nextProtocolVoteBefore) - int64(stat.LastRound)
if votesToGo < 0 {
votesToGo = 0
}
if nextProtocolVoteBefore > 0 {
// Make sure a vote is happening
if stat.NextProtocolVoteBefore > 0 {
votesToGo := uint64(0)
// Check if the vote window is still open.
if stat.NextProtocolVoteBefore > stat.LastRound {
// subtract 1 because the variables are referring to "Last" round and "VoteBefore"
votesToGo = uint64(stat.NextProtocolVoteBefore - stat.LastRound - 1)
Copy link
Contributor

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can't be an underflow because it is inside a check that NextProtocolVoteBefore > LastRound.

}

consensus := config.Consensus[protocol.ConsensusCurrentVersion]
upgradeVoteRounds := consensus.UpgradeVoteRounds
upgradeThreshold := consensus.UpgradeThreshold
votes := uint64(consensus.UpgradeVoteRounds) - uint64(votesToGo)
votes := consensus.UpgradeVoteRounds - votesToGo
Copy link
Contributor

Choose a reason for hiding this comment

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

always nonnegative

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you asking to change these to uint64? I think a negative number and an underflow are both equally wrong, but a negative number would be easier to debug.

Copy link
Contributor

Choose a reason for hiding this comment

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

no I think I meant, "I followed the definitions of these things enough to convince me that this value is always nonnegative" in the normal use case

votesYes := stat.NextProtocolApprovals
votesNo := votes - votesYes
upgradeDelay := uint64(stat.UpgradeDelay)
upgradeDelay := stat.UpgradeDelay
response.UpgradeVotesRequired = &upgradeThreshold
response.UpgradeNodeVote = &stat.UpgradeApprove
response.UpgradeDelay = &upgradeDelay
response.UpgradeVotes = &votes
response.UpgradeYesVotes = &votesYes
response.UpgradeNoVotes = &votesNo
response.UpgradeNextProtocolVoteBefore = &nextProtocolVoteBefore
response.UpgradeNextProtocolVoteBefore = numOrNil(uint64(stat.NextProtocolVoteBefore))
response.UpgradeVoteRounds = &upgradeVoteRounds
}

Expand Down
2 changes: 1 addition & 1 deletion daemon/algod/api/server/v2/test/handlers_resources_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func setupTestForLargeResources(t *testing.T, acctSize, maxResults int, accountM
acctData = accountMaker(acctSize)
ml.accounts[fakeAddr] = acctData

mockNode := makeMockNode(&ml, t.Name(), nil, false)
mockNode := makeMockNode(&ml, t.Name(), nil, cannedStatusReportGolden)
mockNode.config.MaxAPIResourcesPerAccount = uint64(maxResults)
dummyShutdownChan := make(chan struct{})
handlers = v2.Handlers{
Expand Down
Loading