Storage gap variable when modifying ERC20Upgradeable contract

Hi,

I’m trying to make an upgradeable interest bearing token that requires modifying the emitted events of Transfer and Allowance. Thus, I need to modify all functions that emit those events, such as _transfer(). However, all of those functions are required to modify the _balance variable, which is private. Therefore, the only option I found is to modify OpenZeppelin’s implementation of ERC20.

Then there is uint256[45] private __gap variable at the end of OZ’s implementation. The question is should I leave this variable, recalculate the length, or remove the variable?

Here is the example

contract Token is
	IERC20Upgradeable,
	Initializable,
	ContextUpgradeable,
	OwnableUpgradeable
{     
    // Add new variables
    uint256 private _foo;

    ...ERC20 Implementation...

    // Add new functions
    function example() external {
        ...
    }

    // Modify 
    function _transfer(
	  address sender,
	  address recipient,
	  uint256 amount
	) private returns (uint256) {
      ... basic implementation with some modification...

      emit Transfer(sender, recipient, modifiedAmount);

      return amount;
    }

    ...

    // This is the main concern
    uint256[45] private __gap
}

Thanks.

How do you need to modify these events? If what you want to do is compliant with the ERC20 spec, you can definitely implement it without modifying the base contract. If you can't do it, you may be thinking about it the wrong way.

Basically what I'm trying to achieve is to use a multiplier that will scale the user's balance according to how much the supply has been increased, the mechanism is similar to AToken. So, in every balance call, the amount will be scaled with the multiplier and every transfer the amount needs to be unscaled. Thus, the emitted event (Transfer and Allowance) is showing the unscaled balance if I only override the transfer() function.

You should override transfer and pass in scaled values when you call super.transfer.

Thats what I have done before

contract Token is ERC20Upgradeable, OwnableUpgradeable {
    function transfer(address recipient, uint256 amount)
		public
		virtual
		override
		returns (bool)
	{
		uint256 unscaledAmount = getUnscaledAmount(amount);
		super.transfer(recipient, unscaledAmount);

		return true;
	}

However, when the multiplier increased, the emitted amount is incorrect.

  • Multiplier 1
    Alice balance 100 → Scaled Balance 100
    On transfer, emits 100

  • Multiplier 2
    Alice balance 100 → Scaled Balance 200
    On transfer, emits 100

So, this is the workaround

contract Token is
	IERC20Upgradeable,
	Initializable,
	ContextUpgradeable,
	OwnableUpgradeable
{
    ...

    function transfer(address recipient, uint256 amount)
		external
		override
		returns (bool)
	{
		_transfer(_msgSender(), recipient, amount);
		return true;
	}

    ...

    function _transfer(
		address sender,
		address recipient,
		uint256 amount
	) private returns (uint256) {
		require(sender != address(0), "ERC20: transfer from the zero address");
		require(recipient != address(0), "ERC20: transfer to the zero address");

		uint256 unscaledAmount = _getUnscaledAmount(amount);

		uint256 senderBalance = _balances[sender];
		require(
			senderBalance >= unscaledAmount,
			"ERC20: transfer amount exceeds balance"
		);
		_balances[sender] = senderBalance - unscaledAmount;
		_balances[recipient] += unscaledAmount;

		emit Transfer(sender, recipient, amount);

		return unscaledAmount;
	}

	/**
	 * @notice Function that will return the unscaled amount of the parameter
	 * according to the current _multiplier.
	 * Unscaled amount is the amount that is used internally as this is the
	 * unit that is used in the values stored in the contract (eg. balances, totalSupply)
	 *
	 * @param amount The `amount` that wants to be unscaled
	 * @return The unscaled amount
	 */
	function _getUnscaledAmount(uint256 amount) private view returns (uint256) {
		return amount.rayDiv(_multiplier);
	}
}

Ok I see what you’re doing now. I would say this fits under the “not compliant with ERC20” category. One of the problems with this approach you’re following is that the Transfer events essentially become meaningless: if I look at the log of events and accumulate the parameters of Transfer events in order to calculate an account’s balance, I would get meaningless values if the events contain values with a changing scaling factor.

I would highly advise against this.