chore: Standardization and cleanup #31
Conversation
deluca-mike
force-pushed
the
chore/cleanup
branch
from
32e0f4e to
9bfb1e6
Compare
contracts/ERC20.sol
Outdated
|
|
||
| // Appendix F in the Ethereum Yellow paper (https://ethereum.github.io/yellowpaper/paper.pdf), defines | ||
| // the valid range for s in (301): 0 < s < secp256k1n ÷ 2 + 1, and for v in (302): v ∈ {27, 28}. | ||
| require( | ||
| uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0 && | ||
| (v == 27 || v == 28), | ||
| uint256(s_) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0 && |
There was a problem hiding this comment.
You can also mark the limit explicitly as a uint256.
contracts/interfaces/IERC20.sol
Outdated
| @@ -133,9 +133,9 @@ interface IERC20 { | |||
|
|
|||
| /** | |||
| * @dev Returns the permit type hash. | |||
| * @return hash_ The permit type hash. | |||
| * @return permitTypeHash_ The permit type hash. | |||
There was a problem hiding this comment.
Hmm, should this be permitTypehash instead? Based on the name of the function.
| @@ -10,240 +10,243 @@ import { MockERC20 } from "./mocks/MockERC20.sol"; | |||
|
|
|||
| contract ERC20BaseTest is TestUtils { | |||
|
|
|||
| bytes constant ARITHMETIC_ERROR = abi.encodeWithSignature("Panic(uint256)", 0x11); | |||
| address internal immutable self = address(this); | |||
There was a problem hiding this comment.
AFAIK immutable should be all caps as well.
There was a problem hiding this comment.
You'd think that, but check out issue 9554 in solidity's issues... They don't consider immutables constant.
|
|
||
| function setUp() public virtual { | ||
| token = new MockERC20("Token", "TKN", 18); | ||
| function setUp() external virtual { |
There was a problem hiding this comment.
The style guide has the test functions marked as public, but not sure what the convention is.
There was a problem hiding this comment.
ight be just a copy and paste where it wasn't considered.
contracts/test/ERC20.t.sol
Outdated
| bytes32 digest = _getDigest(owner_, spender, value_, nonce, deadline_); | ||
| ( uint8 v, bytes32 r, bytes32 s ) = vm.sign(ownerSk_, digest); | ||
| return (v, r, s); | ||
| function _getValidPermitSignature(uint256 amount_, address owner_, uint256 ownerSk_, uint256 deadline_) internal returns (uint8 v_, bytes32 r_, bytes32 s_) { |
There was a problem hiding this comment.
Maybe reorder parameters based on order in the function call?
There was a problem hiding this comment.
Yup, but I'll do one slightly better.
Description
Integrations Checklist
Changelog
Function Signature Changes
Features
Events