-
Notifications
You must be signed in to change notification settings - Fork 471
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
REST: add the round number to algod box endpoint repsonse #5340
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like a good idea and correct implementation to me.
I made up the round numbers in the e2e test, they need to be replaced with the right answer. |
Will that even be possible? Are these e2e tests run deterministically? (the e2e_subs are not) |
Ran in sandbox, caught up to testnet, found an app with a box, ran box endpoint:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems right
"round": { | ||
"description": "The round for which this information is relevant", | ||
"type": "integer" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jasonpaulos would it be better to add the round in BoxResponse instead? BoxResponse uses a schema reference though, and I seem to remember something about not being able to add to a schema reference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this makes sense here. The schema ref that BoxResponse uses means it's really just an alias to Box. See our generated type here:
type BoxResponse = Box |
"round": { | ||
"description": "The round for which this information is relevant", | ||
"type": "integer" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this makes sense here. The schema ref that BoxResponse uses means it's really just an alias to Box. See our generated type here:
type BoxResponse = Box |
// To reduce flakiness, only check the round from boxes is within a range. | ||
a.GreaterOrEqual(boxResponse.Round, currentRoundBeforeBoxes) | ||
a.LessOrEqual(boxResponse.Round, currentRoundAfterBoxes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nicely done
Summary
Include the round that the value of the box's contents is pulled from to the response from v2/box endpoint
Test Plan
Modified e2e to check rounds.