Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inherit asset decimals in ERC4626 #3639

Merged
merged 17 commits into from
Aug 26, 2022
Merged
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
* `GovernorCompatibilityBravo`: remove unused `using` statements. ([#3506](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3506))
* `ERC20`: optimize `_transfer`, `_mint` and `_burn` by using `unchecked` arithmetic when possible. ([#3513](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3513))
* `ERC20FlashMint`: add an internal `_flashFee` function for overriding. ([#3551](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3551))
* `ERC4626`: use the same `decimals()` as the underlying asset by default (if available). ([#3639](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3639))
* `ERC4626`: add internal `_initialConvertToShares` and `_initialConvertToAssets` functions to customize empty vaults behavior. ([#3639](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3639))
* `ERC721`: optimize transfers by making approval clearing implicit instead of emitting an event. ([#3481](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3481))
* `ERC721`: optimize burn by making approval clearing implicit instead of emitting an event. ([#3538](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3538))
* `ERC721`: Fix balance accounting when a custom `_beforeTokenTransfer` hook results in a transfer of the token under consideration. ([#3611](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3611))
Expand All @@ -19,6 +21,10 @@
* `ECDSA`: Remove redundant check on the `v` value. ([#3591](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3591))
* `VestingWallet`: add `releasable` getters. ([#3580](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3580))

### Breaking changes

* `ERC4626`: Conversion from shares to assets (and vice-versa) in an empty vault used to consider the possible mismatch between the underlying asset's and the vault's decimals. With the vault now using the assets decimals by default, this initial conversion rate is now set to 1-to-1. Developers overriding the vault decimals to a value that does not match the underlying asset may want to override the `_initialConvertToShares` and `_initialConvertToAssets` to replicate the previous behavior. Note that this change should not affect non-empty vault upgrades.
Amxx marked this conversation as resolved.
Show resolved Hide resolved

### Deprecations

* `EIP712`: Added the file `EIP712.sol` and deprecated `draft-EIP712.sol` since the EIP is no longer a Draft. Developers are encouraged to update their imports. ([#3621](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3621))
Expand Down
6 changes: 6 additions & 0 deletions contracts/interfaces/README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,12 @@ are useful to interact with third party contracts that implement them.
- {IERC1363}
- {IERC1820Implementer}
- {IERC1820Registry}
- {IERC1822Proxiable}
- {IERC2612}
- {IERC2981}
- {IERC3156FlashLender}
- {IERC3156FlashBorrower}
- {IERC4626}

== Detailed ABI

Expand All @@ -41,10 +43,14 @@ are useful to interact with third party contracts that implement them.

{{IERC1820Registry}}

{{IERC1822Proxiable}}

{{IERC2612}}

{{IERC2981}}

{{IERC3156FlashLender}}

{{IERC3156FlashBorrower}}

{{IERC4626}}
40 changes: 39 additions & 1 deletion contracts/mocks/ERC4626Mock.sol
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity ^0.8.0;

import "../token/ERC20/extensions/ERC4626.sol";

// mock class using ERC20
contract ERC4626Mock is ERC4626 {
constructor(
IERC20Metadata asset,
Expand All @@ -20,3 +19,42 @@ contract ERC4626Mock is ERC4626 {
_burn(account, amount);
}
}

contract ERC4626DecimalMock is ERC4626Mock {
using Math for uint256;

uint8 private immutable _decimals;

constructor(
IERC20Metadata asset,
string memory name,
string memory symbol,
uint8 decimalsOverride
) ERC4626Mock(asset, name, symbol) {
_decimals = decimalsOverride;
}

function decimals() public view virtual override returns (uint8) {
return _decimals;
}

function _initialConvertToShares(uint256 assets, Math.Rounding rounding)
internal
view
virtual
override
returns (uint256 shares)
{
return assets.mulDiv(10**decimals(), 10**super.decimals(), rounding);
frangio marked this conversation as resolved.
Show resolved Hide resolved
}

function _initialConvertToAssets(uint256 shares, Math.Rounding rounding)
internal
view
virtual
override
returns (uint256 assets)
{
return shares.mulDiv(10**super.decimals(), 10**decimals(), rounding);
}
}
45 changes: 39 additions & 6 deletions contracts/token/ERC20/extensions/ERC4626.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,24 @@ import "../../../utils/math/Math.sol";
abstract contract ERC4626 is ERC20, IERC4626 {
using Math for uint256;

IERC20Metadata private immutable _asset;
IERC20 private immutable _asset;

/**
* @dev Set the underlying asset contract. This must be an ERC20-compatible contract (ERC20 or ERC777).
*/
constructor(IERC20Metadata asset_) {
constructor(IERC20 asset_) {
_asset = asset_;
}

/** @dev See {IERC4626-asset}. */
function decimals() public view virtual override(IERC20Metadata, ERC20) returns (uint8) {
try IERC20Metadata(address(_asset)).decimals() returns (uint8 value) {
Amxx marked this conversation as resolved.
Show resolved Hide resolved
return value;
} catch {
return super.decimals();
}
}

/** @dev See {IERC4626-asset}. */
function asset() public view virtual override returns (address) {
return address(_asset);
Expand Down Expand Up @@ -153,19 +162,43 @@ abstract contract ERC4626 is ERC20, IERC4626 {
uint256 supply = totalSupply();
return
(assets == 0 || supply == 0)
? assets.mulDiv(10**decimals(), 10**_asset.decimals(), rounding)
? _initialConvertToShares(assets, rounding)
: assets.mulDiv(supply, totalAssets(), rounding);
}

/**
* @dev Internal conversion function (from assets to shares) to apply when the vault is empty.
frangio marked this conversation as resolved.
Show resolved Hide resolved
* Note: make sure to keep this function consistent with {_initialConvertToAssets} when overriding it.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
*/
function _initialConvertToShares(uint256 assets, Math.Rounding /*rounding*/)
internal
view
virtual
returns (uint256 shares)
{
return assets;
}

/**
* @dev Internal conversion function (from shares to assets) with support for rounding direction.
*/
function _convertToAssets(uint256 shares, Math.Rounding rounding) internal view virtual returns (uint256 assets) {
uint256 supply = totalSupply();
return
(supply == 0)
? shares.mulDiv(10**_asset.decimals(), 10**decimals(), rounding)
: shares.mulDiv(totalAssets(), supply, rounding);
(supply == 0) ? _initialConvertToAssets(shares, rounding) : shares.mulDiv(totalAssets(), supply, rounding);
}

/**
* @dev Internal conversion function (from shares to assets) to apply when the vault is empty.
* Note: make sure to keep this function consistent with {_initialConvertToShares} when overriding it.
Amxx marked this conversation as resolved.
Show resolved Hide resolved
*/
function _initialConvertToAssets(uint256 shares, Math.Rounding /*rounding*/)
internal
view
virtual
returns (uint256 assets)
{
return shares;
}

/**
Expand Down
4 changes: 3 additions & 1 deletion contracts/token/ERC20/extensions/draft-IERC20Permit.sol
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

pragma solidity ^0.8.0;

import "../IERC20.sol";

/**
* @dev Interface of the ERC20 Permit extension allowing approvals to be made via signatures, as defined in
* https://eips.ethereum.org/EIPS/eip-2612[EIP-2612].
Expand All @@ -11,7 +13,7 @@ pragma solidity ^0.8.0;
* presenting a message signed by the account. By not relying on {IERC20-approve}, the token holder account doesn't
* need to send a transaction, and thus is not required to hold Ether at all.
*/
interface IERC20Permit {
interface IERC20Permit is IERC20 {
Amxx marked this conversation as resolved.
Show resolved Hide resolved
/**
* @dev Sets `value` as the allowance of `spender` over ``owner``'s tokens,
* given ``owner``'s signed approval.
Expand Down
12 changes: 11 additions & 1 deletion test/token/ERC20/extensions/ERC4626.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const { expect } = require('chai');

const ERC20DecimalsMock = artifacts.require('ERC20DecimalsMock');
const ERC4626Mock = artifacts.require('ERC4626Mock');
const ERC4626DecimalMock = artifacts.require('ERC4626DecimalMock');

const parseToken = (token) => (new BN(token)).mul(new BN('1000000000000'));
const parseShare = (share) => (new BN(share)).mul(new BN('1000000000000000000'));
Expand All @@ -15,7 +16,7 @@ contract('ERC4626', function (accounts) {

beforeEach(async function () {
this.token = await ERC20DecimalsMock.new(name, symbol, 12);
this.vault = await ERC4626Mock.new(this.token.address, name + ' Vault', symbol + 'V');
this.vault = await ERC4626DecimalMock.new(this.token.address, name + ' Vault', symbol + 'V', 18);

await this.token.mint(holder, web3.utils.toWei('100'));
await this.token.approve(this.vault.address, constants.MAX_UINT256, { from: holder });
Expand All @@ -25,9 +26,18 @@ contract('ERC4626', function (accounts) {
it('metadata', async function () {
expect(await this.vault.name()).to.be.equal(name + ' Vault');
expect(await this.vault.symbol()).to.be.equal(symbol + 'V');
expect(await this.vault.decimals()).to.be.bignumber.equal('18');
expect(await this.vault.asset()).to.be.equal(this.token.address);
});

it('inherit decimals if from asset', async function () {
for (const decimals of [ 0, 9, 12, 18, 36 ].map(web3.utils.toBN)) {
const token = await ERC20DecimalsMock.new('', '', decimals);
const vault = await ERC4626Mock.new(token.address, '', '');
expect(await vault.decimals()).to.be.bignumber.equal(decimals);
}
});

describe('empty vault: no assets & no shares', function () {
it('status', async function () {
expect(await this.vault.totalAssets()).to.be.bignumber.equal('0');
Expand Down