fix edge cases in assert_before parsing #140
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
arvidn
commented
Feb 15, 2023
arvidn
commented
Feb 15, 2023
…a negative value is an automatic fail. assert_before a value exceeding the max is an automatic pass
arvidn
force-pushed
the
assert-before-edge-cases
branch
from
fd60489
to
2b21dce
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Specifically, assert_before a negative value is an automatic fail. assert_before a value exceeding the max is an automatic pass.
The two functions used to parse block height and timestamps (seconds) for the original
ASSERT_HEIGHT_*andASSERT_SECONDS_*has edge-case behavior tailored to those conditions. Those conditions requires the block height or timestamp to exceed the specified limit. If the limit is 0 or negative, the condition is always true. If the limit exceeds the max value however (uint32 and uint64 respectively), the conditions always fail.Since the
ASSERT_BEFORE_*conditions have the inverse semantics, the edge case behavior also need to be inverse. This patch introducesparse_positive_height()andparse_positive_seconds()with the inverse behavior for values outside the limits.The bulk of this patch is to fix up the unit tests to explicitly cover these cases, both for the existing
parse_height()and the newparse_positive_height()(as well as the "seconds" counterpart).