Skip to content

Commit

Permalink
formatting: Natspec and variables (#30)
Browse files Browse the repository at this point in the history 
* formatting: natspec and variables

* fix: update Natspec spacing

* fix: update tests to no longer use view function of s value

* formatting: update test names
  • Loading branch information
Lucas Manuel authored Mar 15, 2022
1 parent 8d55417 commit ab76d12
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 45 deletions.
39 changes: 18 additions & 21 deletions contracts/ERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ pragma solidity ^0.8.7;
import { IERC20 } from "./interfaces/IERC20.sol";

/**
* @title Modern and gas efficient ERC-20 implementation.
* @dev Acknowledgements to Solmate, OpenZeppelin, and DSS for inspiring this code.
* @title Modern ERC-20 implementation.
* @dev Acknowledgements to Solmate, OpenZeppelin, and DSS for inspiring this code.
*/
contract ERC20 is IERC20 {

Expand All @@ -29,15 +29,14 @@ contract ERC20 is IERC20 {
/****************/

// PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 amount,uint256 nonce,uint256 deadline)");
bytes32 public constant override PERMIT_TYPEHASH = 0xfc77c2b9d30fe91687fd39abb7d16fcdfe1472d065740051ab8b13e4bf4a617f;
uint256 public constant S_VALUE_INCLUSIVE_UPPER_BOUND = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0;
bytes32 public constant override PERMIT_TYPEHASH = 0xfc77c2b9d30fe91687fd39abb7d16fcdfe1472d065740051ab8b13e4bf4a617f;

mapping (address => uint256) public override nonces;
mapping(address => uint256) public override nonces;

/**
* @param name_ The name of the token.
* @param symbol_ The symbol of the token.
* @param decimals_ The decimal precision used by the token.
* @param name_ The name of the token.
* @param symbol_ The symbol of the token.
* @param decimals_ The decimal precision used by the token.
*/
constructor(string memory name_, string memory symbol_, uint8 decimals_) {
name = name_;
Expand Down Expand Up @@ -69,8 +68,12 @@ contract ERC20 is IERC20 {

// 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) <= S_VALUE_INCLUSIVE_UPPER_BOUND && (v == 27 || v == 28), "ERC20:P:MALLEABLE");

require(
uint256(s) <= 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0 &&
(v == 27 || v == 28),
"ERC20:P:MALLEABLE"
);

// Nonce realistically cannot overflow.
unchecked {
bytes32 digest = keccak256(
Expand All @@ -83,7 +86,7 @@ contract ERC20 is IERC20 {
address recoveredAddress = ecrecover(digest, v, r, s);
require(recoveredAddress == owner && owner != address(0), "ERC20:P:INVALID_SIGNATURE");
}

_approve(owner, spender, amount);
}

Expand Down Expand Up @@ -126,20 +129,16 @@ contract ERC20 is IERC20 {
balanceOf[owner_] -= amount_;

// Cannot underflow because a user's balance will never be larger than the total supply.
unchecked {
totalSupply -= amount_;
}

unchecked { totalSupply -= amount_; }

emit Transfer(owner_, address(0), amount_);
}

function _mint(address recipient_, uint256 amount_) internal {
totalSupply += amount_;

// Cannot overflow because totalSupply would first overflow in the statement above.
unchecked {
balanceOf[recipient_] += amount_;
}
unchecked { balanceOf[recipient_] += amount_; }

emit Transfer(address(0), recipient_, amount_);
}
Expand All @@ -148,9 +147,7 @@ contract ERC20 is IERC20 {
balanceOf[owner_] -= amount_;

// Cannot overflow because minting prevents overflow of totalSupply, and sum of user balances == totalSupply.
unchecked {
balanceOf[recipient_] += amount_;
}
unchecked { balanceOf[recipient_] += amount_; }

emit Transfer(owner_, recipient_, amount_);
}
Expand Down
36 changes: 21 additions & 15 deletions contracts/interfaces/IERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,54 +92,60 @@ interface IERC20 {
/**********************/

/**
* @dev Function that returns the allowance that one account has given another over their tokens.
* @param owner_ Account that tokens are approved from.
* @param spender_ Account that tokens are approved for.
* @dev Returns the allowance that one account has given another over their tokens.
* @param owner_ Account that tokens are approved from.
* @param spender_ Account that tokens are approved for.
* @return allowance_ Allowance that one account has given another over their tokens.
*/
function allowance(address owner_, address spender_) external view returns (uint256 allowance_);

/**
* @dev Returns the amount of tokens owned by a given account.
* @param account_ Account that owns the tokens.
* @dev Returns the amount of tokens owned by a given account.
* @param account_ Account that owns the tokens.
* @return balance_ Amount of tokens owned by a given account.
*/
function balanceOf(address account_) external view returns (uint256 balance_);

/**
* @dev Returns the decimal precision used by the token.
* @dev Returns the decimal precision used by the token.
* @return decimals_ The decimal precision used by the token.
*/
function decimals() external view returns (uint8 decimals_);

/**
* @dev Returns the signature domain separator.
* @return domainSeparator_ The domain for the contract.
* @dev Returns the signature domain separator.
* @return domainSeparator_ The signature domain separator.
*/
function DOMAIN_SEPARATOR() external view returns (bytes32 domainSeparator_);

/**
* @dev Returns the name of the token.
* @dev Returns the name of the token.
* @return name_ The name of the token.
*/
function name() external view returns (string memory name_);

/**
* @dev Returns the nonce for the given owner.
* @param owner The address of the owner account.
* @return nonce_ The current nonce.
* @dev Returns the nonce for the given owner.
* @param owner The address of the owner account.
* @return nonce_ The nonce for the given owner.
*/
function nonces(address owner) external view returns (uint256 nonce_);

/**
* @dev Returns the permit type hash.
* @return hash_ The typehash for the commit.
* @return hash_ The permit type hash.
*/
function PERMIT_TYPEHASH() external view returns (bytes32 hash_);

/**
* @dev Returns the symbol of the token.
* @dev Returns the symbol of the token.
* @return symbol_ The symbol of the token.
*/
function symbol() external view returns (string memory symbol_);

/**
* @dev Returns the total amount of tokens in existence.
* @dev Returns the total amount of tokens in existence.
* @return totalSupply_ The total amount of tokens in existence.
*/
function totalSupply() external view returns (uint256 totalSupply_);

Expand Down
18 changes: 10 additions & 8 deletions contracts/test/ERC20.t.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,8 @@ contract ERC20PermitTest is TestUtils {

bytes constant ARITHMETIC_ERROR = abi.encodeWithSignature("Panic(uint256)", 0x11);

uint256 constant S_VALUE_INCLUSIVE_UPPER_BOUND = 0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0;

ERC20 token;
ERC20User user;

Expand Down Expand Up @@ -222,15 +224,15 @@ contract ERC20PermitTest is TestUtils {
assertEq(token.nonces(owner), 1);
}

function test_permitZeroAddress() external {
function test_permit_zeroAddress() external {
uint256 amount = 10 * WAD;
( uint8 v, bytes32 r, bytes32 s ) = _getValidPermitSignature(amount, owner, skOwner, deadline);

vm.expectRevert(bytes("ERC20:P:INVALID_SIGNATURE"));
user.erc20_permit(address(token), address(0), spender, amount, deadline, v, r, s);
}

function test_permitNonOwnerAddress() external {
function test_permit_nonOwnerAddress() external {
uint256 amount = 10 * WAD;

( uint8 v, bytes32 r, bytes32 s ) = _getValidPermitSignature(amount, owner, skOwner, deadline);
Expand All @@ -244,7 +246,7 @@ contract ERC20PermitTest is TestUtils {
user.erc20_permit(address(token), owner, spender, amount, deadline, v, r, s);
}

function test_permitWithExpiry() external {
function test_permit_withExpiry() external {
uint256 amount = 10 * WAD;
uint256 expiry = 482112000 + 1 hours;

Expand All @@ -271,7 +273,7 @@ contract ERC20PermitTest is TestUtils {
assertEq(token.nonces(owner), 1);
}

function test_permitReplay() external {
function test_permit_replay() external {
uint256 amount = 10 * WAD;
( uint8 v, bytes32 r, bytes32 s ) = _getValidPermitSignature(amount, owner, skOwner, deadline);

Expand All @@ -283,23 +285,23 @@ contract ERC20PermitTest is TestUtils {
user.erc20_permit(address(token), owner, spender, amount, deadline, v, r, s);
}

function test_permitBadS() external {
function test_permit_badS() external {
uint256 amount = 10 * WAD;
( uint8 v, bytes32 r, bytes32 s ) = _getValidPermitSignature(amount, owner, skOwner, deadline);

// Send in an s that is above the upper bound.
bytes32 badS = bytes32(token.S_VALUE_INCLUSIVE_UPPER_BOUND() + 1);
bytes32 badS = bytes32(S_VALUE_INCLUSIVE_UPPER_BOUND + 1);
vm.expectRevert(bytes("ERC20:P:MALLEABLE"));
user.erc20_permit(address(token), owner, spender, amount, deadline, v, r, badS);

user.erc20_permit(address(token), owner, spender, amount, deadline, v, r, s);
}

function test_permitBadV() external {
function test_permit_badV() external {
uint256 amount = 10 * WAD;

// Get valid signature. The `v` value is the expected v value that will cause `permit` to succeed, and must be 27 or 28.
// Any other value should fail.
// Any other value should fail.
// If v is 27, then 28 should make it past the MALLEABLE require, but should result in an invalid signature, and vice versa when v is 28.
( uint8 v, bytes32 r, bytes32 s ) = _getValidPermitSignature(amount, owner, skOwner, deadline);

Expand Down
2 changes: 1 addition & 1 deletion contracts/test/mocks/MockERC20.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { ERC20 } from "../../ERC20.sol";

contract MockERC20 is ERC20 {

constructor(string memory name_, string memory symbol_, uint8 decimals_) ERC20(name_, symbol_, decimals_) {}
constructor(string memory name_, string memory symbol_, uint8 decimals_) ERC20(name_, symbol_, decimals_) { }

function mint(address to_, uint256 value_) external {
_mint(to_, value_);
Expand Down

0 comments on commit ab76d12

Please sign in to comment.