fix: refactored contracts #234
Merged
Conversation
This file contains hidden or 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
shaspitz
approved these changes
Jul 12, 2024
Contributor
shaspitz
left a comment
shaspitz
left a comment
There was a problem hiding this comment.
Looks solid! Just a few comments
| ) internal pure returns (string memory) { | ||
| bytes memory HEXCHARS = "0123456789abcdef"; | ||
| bytes memory _string = new bytes(64); | ||
| for (uint8 i = 0; i < 32; i++) { |
Contributor
There was a problem hiding this comment.
Suggested change
| for (uint8 i = 0; i < 32; i++) { | |
| for (uint8 i = 0; i < 32; ++i) { |
| ) public pure returns (string memory) { | ||
| bytes memory HEXCHARS = "0123456789abcdef"; | ||
| bytes memory _string = new bytes(_bytes.length * 2); | ||
| for (uint256 i = 0; i < _bytes.length; i++) { |
Contributor
There was a problem hiding this comment.
Suggested change
| for (uint256 i = 0; i < _bytes.length; i++) { | |
| for (uint256 i = 0; i < _bytes.length; ++i) { |
|
|
||
| // Check if the dispatch timestamp is within the allowed dispatch window | ||
| require(dispatchTimestamp > minTime, "Invalid dispatch timestamp"); | ||
| require(dispatchTimestamp >= minTime, "Invalid dispatch timestamp"); |
Contributor
There was a problem hiding this comment.
Is this change of behavior intentional? After clarifying with the auditors their stance is When inside the require statements, non-strict inequalities (>=, <=) are usually costlier than strict equalities (>, <).
Contributor
Author
There was a problem hiding this comment.
I did it, after reading this quote:
When inside the “if/require” statements, non-strict inequalities (>=, <=) are
usually cheaper than the strict equalities (>, <).
Thank you for clarifying, I will return it back
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.
Closes: #233