Omniscia LimeChain Audit

ERC20Permit Code Style Findings

ERC20Permit Code Style Findings

ERC-01C: Absence of Event

TypeSeverityLocation
Language SpecificInformationalERC20Permit.sol:L37-L111

Description:

A permit function call is indiscernible from an approve one given that no event is emitted signaling the permittance.

Example:

contracts/ERC20Permit.sol
37function permit(
38    address _owner,
39    address _spender,
40    uint256 _amount,
41    uint256 _deadline,
42    uint8 _v,
43    bytes32 _r,
44    bytes32 _s
45) public virtual override {
46    require(block.timestamp <= _deadline, "ERC20Permit: expired _deadline");
47

48    // Assembly for more efficiently computing:
49    // bytes32 hashStruct = keccak256(
50    //     abi.encode(
51    //         _PERMIT_TYPEHASH,
52    //         _owner,
53    //         _spender,
54    //         _amount,
55    //         nonces[_owner].current(),
56    //         _deadline
57    //     )
58    // );
59

60    bytes32 hashStruct;
61    Counters.Counter storage nonceCounter = _nonces[_owner];
62    uint256 nonce = nonceCounter.current();
63

64    assembly {
65        // Load free memory pointer
66        let memPtr := mload(64)
67

68        // keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)")
69        mstore(
70            memPtr,
71            0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9
72        )
73        mstore(add(memPtr, 32), _owner)
74        mstore(add(memPtr, 64), _spender)
75        mstore(add(memPtr, 96), _amount)
76        mstore(add(memPtr, 128), nonce)
77        mstore(add(memPtr, 160), _deadline)
78

79        hashStruct := keccak256(memPtr, 192)
80    }
81

82    bytes32 eip712DomainHash = _domainSeparator();
83

84    // Assembly for more efficient computing:
85    // bytes32 hash = keccak256(
86    //     abi.encodePacked(uint16(0x1901), eip712DomainHash, hashStruct)
87    // );
88

89    bytes32 hash;
90

91    assembly {
92        // Load free memory pointer
93        let memPtr := mload(64)
94

95        mstore(
96            memPtr,
97            0x1901000000000000000000000000000000000000000000000000000000000000
98        ) // EIP191 header
99        mstore(add(memPtr, 2), eip712DomainHash) // EIP712 domain hash
100        mstore(add(memPtr, 34), hashStruct) // Hash of struct
101

102        hash := keccak256(memPtr, 66)
103    }
104

105    address signer = _recover(hash, _v, _r, _s);
106

107    require(signer == _owner, "ERC20Permit: invalid signature");
108

109    nonceCounter.increment();
110    _approve(_owner, _spender, _amount);
111}

Recommendation:

We advise an event to be declared and emitted whenever the function is invoked to ensure off-chain ease of integration.

Alleviation:

The contract was omitted from the codebase and replaced by the OpenZeppelin draft implementation thus rendering this exhibit null.

ERC-02C: Caching Optimization

TypeSeverityLocation
Gas OptimizationInformationalERC20Permit.sol:L126-L130

Description:

The values of the first three arguments for the domain separator calculation in _updateDomainSeparator can be cached to optimize the gas cost of the function.

Example:

contracts/ERC20Permit.sol
124bytes32 newDomainSeparator = keccak256(
125    abi.encode(
126        keccak256(
127            "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
128        ),
129        keccak256(bytes(name())), // ERC-20 Name
130        keccak256(bytes("1")), // Version
131        chainID,
132        address(this)
133    )
134);

Recommendation:

We advise them to be done so, preferably to constant / immutable contract level variables depending on the argument's nature.

Alleviation:

The contract was omitted from the codebase and replaced by the OpenZeppelin draft implementation thus rendering this exhibit null.

ERC-03C: Duplicate Invocation of Chain ID

TypeSeverityLocation
Gas OptimizationInformationalERC20Permit.sol:L121, L143, L149

Description:

The _domainSeparator function can be optimized by passing in the calculated _chainID to _updateDomainSeparator thereby reducing the _chainID calls to one instead of two.

Example:

contracts/ERC20Permit.sol
120function _updateDomainSeparator() private returns (bytes32) {
121    uint256 chainID = _chainID();
122

123    // no need for assembly, running very rarely
124    bytes32 newDomainSeparator = keccak256(
125        abi.encode(
126            keccak256(
127                "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"
128            ),
129            keccak256(bytes(name())), // ERC-20 Name
130            keccak256(bytes("1")), // Version
131            chainID,
132            address(this)
133        )
134    );
135

136    domainSeparators[chainID] = newDomainSeparator;
137

138    return newDomainSeparator;
139}
140

141// Returns the domain separator, updating it if chainID changes
142function _domainSeparator() private returns (bytes32) {
143    bytes32 domainSeparator = domainSeparators[_chainID()];
144

145    if (domainSeparator != 0x00) {
146        return domainSeparator;
147    }
148

149    return _updateDomainSeparator();
150}

Recommendation:

We advise it to be done so to reduce the gas cost of the function, additionally changing the constructor's way of _updateDomainSeparator invocation.

Alleviation:

The contract was omitted from the codebase and replaced by the OpenZeppelin draft implementation thus rendering this exhibit null.

ERC-04C: Unconventional Free Memory Pointer Representation

TypeSeverityLocation
Code StyleInformationalERC20Permit.sol:L66, L93

Description:

The free memory pointer for the assembly language sub-set of Solidity is depicted in the documentation as 0x40 in hexadecimal which is equivalent to 64.

Example:

contracts/ERC20Permit.sol
65// Load free memory pointer
66let memPtr := mload(64)

Recommendation:

For the sake of convention, we advise the value of 64 to be replaced by 0x40.

Alleviation:

The contract was omitted from the codebase and replaced by the OpenZeppelin draft implementation thus rendering this exhibit null.

ERC-05C: require Over revert

TypeSeverityLocation
Code StyleInformationalERC20Permit.sol:L176-L181, L183-L185

Description:

The linked if-revert clauses can be substituted with require checks.

Example:

contracts/ERC20Permit.sol
176if (
177    uint256(_s) >
178    0x7FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF5D576E7357A4501DDFE92F46681B20A0
179) {
180    revert("ECDSA: invalid signature '_s' value");
181}
182

183if (_v != 27 && _v != 28) {
184    revert("ECDSA: invalid signature '_v' value");
185}

Recommendation:

We advise them to be done.

Alleviation:

The contract was omitted from the codebase and replaced by the OpenZeppelin draft implementation thus rendering this exhibit null.

Prev

RouterFacet.sol (RFT-M)

Next

Router.sol (ROU-C)

On this page