# AdvancedERC20: Smart Contract Audit Report Resolution #

## Resolution Summary ##

| ID      |                                                                    | Resolution   |
|---------|--------------------------------------------------------------------|--------------|
| Minor-1 | Inconsistency Between `totalSupply` Comment and Mint Functionality | Fixed        |
| Minor-2 | Reentrancy Consideration in `GenericFactoryV1.clone`               | Acknowledged |
| Notes-1 | Version Constraints with Known Issues                              | Mitigated    |
| Notes-2 | Visibility Optimization for Gas Savings                            | Acknowledged |
| Notes-3 | Test Coverage                                                      | Fixed        |
| Notes-4 | Use of Block Timestamps in Authorization Functions                 | Acknowledged |
| Notes-5 | Gas Optimization through Smaller Data Types                        | Not Valid    |
| Notes-6 | Simplification of add and sub Functions                            | Fixed        |

For issues which were ignored, acknowledged, mitigated, or fixed differently than suggested by the auditor, see the
[Comments](#comments) section below.

## Comments ##
### Minor-2. Reentrancy Consideration in `GenericFactoryV1.clone` ###
`GenericFactoryV1.clone()` function can indeed be used in reentrancy mode, but this is an intended behavior:
1. Cloning functionality just batches deploy + initialize steps on the implementation without adding any logic on top
2. Implementation contract is responsible for its own security when deployed using `GenericFactoryV1.clone()`
3. In the context of `AdvancedERC20` reentrancy is not possible as `AdvancedERC20` doesn't call `GenericFactoryV1`
4. `GenericFactoryV1` is fully permissionless, and if someone wants to use it in reentrancy mode, they absolutely can

### Notes-1. Version Constraints with Known Issues ###
To allow the use as a parent contract for a wide range of RBAC-based applications, and serve as a Solidity library,
we try to keep pragma constraint as low as possible. This approach maximizes compatibility.

Mitigated by updating the compiler version to 0.8.29 in `hardhat.config.js`.

### Notes-2. Visibility Optimization for Gas Savings ###
ERC20 token contracts are often inherited from and used as a base for a more custom ERC20 token implementations.
Therefore, it is hard to say beforehand which functions won't be used in inheriting contracts.

Visibility modifier was changed from `public` to `external` for functions `postConstruct()` and `clone()` in
`GenericFactoryV1`.

### Notes-4. Use of Block Timestamps in Authorization Functions ###
Precise timing is usually not critical for signature verification. Timestamps monotonically increase and this
property is enough for signature verification.

### Notes-5. Gas Optimization through Smaller Data Types ###
Indeed, token balances are represented as `uint256` in most of the places in a smart contract, while the real
supported values are limited to `uint192` data type due to on-chain governance functionality, in particular
voting delegation, and voting power and their core data structure mapping a block number to a token balance:
```
struct KV {
    /*
     * @dev key, a block number
     */
    uint64 k;

    /*
     * @dev value, token balance or voting power
     */
    uint192 v;
}
```

This raises a question if changing the token balances data type to `uint192` allows to make gas optimizations.
However, taking a closer look at implementation, we can see that relevant storage variables have no neighbors:

1. `totalSupply` is the only variable in storage, thus cutting its size has no saving effect
2. `tokenBalances` mapping stores nothing but balances, thus cutting balance size has no saving effect
3. The same applies to `nonces` and `transferAllowances` mappings.