ERC20PermitUpgradeable Code Style Findings

ERP-01C: Sub-Optimal EIP-712 Implementation

Description:

The EIP-712 implementation present in @openzeppelin/contracts-upgradeable at version 3.4.1 is sub-optimal as it does not cache the domain separator of the actively deployed blockchain, meaning that there is room for significant gas improvement on the permit function.

Example:

Copy
9import "@openzeppelin/contracts-upgradeable/drafts/EIP712Upgradeable.sol";

Recommendation:

We advise the Stakewise team to consider forking the EIP-712 draft present at the latest OpenZeppelin iteration that does apply such a caching optimization to ensure optimal gas usage in their system.

Alleviation:

The Stakewise team confirmed this exhibit, however, they will retain the current implementation in place to avoid upgrading the token contract.

ERP-02C: Unoptimized Variable Mutability

Description:

The _PERMIT_TYPEHASH value is set only once during the __ERC20Permit_init_unchained hook and is being set to a static pre-calculated value.

Example:

Copy
28// solhint-disable-next-line var-name-mixedcase
29bytes32 private _PERMIT_TYPEHASH;
30
31/**
32 * @dev Initializes the {EIP712} domain separator using the `name` parameter, and setting `version` to `"1"`.
33 *
34 * It's a good idea to use the same `name` that is defined as the ERC20 token name.
35 */
36// solhint-disable-next-line func-name-mixedcase
37function __ERC20Permit_init(string memory name) internal initializer {
38    __EIP712_init_unchained(name, "1");
39    __ERC20Permit_init_unchained();
40}
41
42// solhint-disable-next-line func-name-mixedcase
43function __ERC20Permit_init_unchained() internal initializer {
44    _PERMIT_TYPEHASH = keccak256("Permit(address owner,address spender,uint256 value,uint256 nonce,uint256 deadline)");
45}

Recommendation:

We advise the variable to be set as constant, its assignment to be relocated to its declaration and the unchained initializer of the ERC20Permit to be omitted as it would be redundant once this optimization is applied.

Alleviation:

The Stakewise team confirmed this exhibit, however, they will retain the current implementation in place to avoid upgrading the token contract.