Skip to content

Commit

Permalink
fix(contract): check address params in initialize
Browse files Browse the repository at this point in the history
  • Loading branch information
@PierrickGT
PierrickGT committed Jun 30, 2021
1 parent dc061c0 commit e113c76
Show file tree
Hide file tree
Showing 2 changed files with 129 additions and 36 deletions.
16 changes: 11 additions & 5 deletions contracts/yield-source/ATokenYieldSource.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import "@openzeppelin/contracts-upgradeable/math/SafeMathUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/ERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/SafeERC20Upgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";
import "@pooltogether/fixed-point/contracts/FixedPoint.sol";

Expand All @@ -21,6 +22,7 @@ import "../interfaces/IProtocolYieldSource.sol";
/// @dev This contract inherits AssetManager which extends OwnableUpgradable
/// @notice Yield source for a PoolTogether prize pool that generates yield by depositing into Aave V2
contract ATokenYieldSource is ERC20Upgradeable, IProtocolYieldSource, AssetManager, ReentrancyGuardUpgradeable {
using AddressUpgradeable for address;
using SafeMathUpgradeable for uint256;
using SafeERC20Upgradeable for IERC20Upgradeable;

Expand Down Expand Up @@ -69,13 +71,14 @@ contract ATokenYieldSource is ERC20Upgradeable, IProtocolYieldSource, AssetManag
/// @notice Interface for Aave lendingPoolAddressesProviderRegistry
ILendingPoolAddressesProviderRegistry public lendingPoolAddressesProviderRegistry;

/// @notice Mock Initializer to prevent

/// @notice Mock Initializer to prevent
function freeze() public initializer {
//no-op
}

/// @notice Initializes the yield source with Aave aToken
/// @dev `isContract()` will return false if an attacker is calling from a constructor https://ethereum.stackexchange.com/a/64340
/// @param _aToken Aave aToken address
/// @param _lendingPoolAddressesProviderRegistry Aave lendingPoolAddressesProviderRegistry address
/// @param _decimals Number of decimals the shares (inhereted ERC20) will have. Set as same as underlying asset to ensure sane ExchangeRates
Expand All @@ -93,14 +96,17 @@ contract ATokenYieldSource is ERC20Upgradeable, IProtocolYieldSource, AssetManag
initializer
returns (bool)
{
require(address(_aToken).isContract(), "ATokenYieldSource/aToken-not-contract-address");
aToken = _aToken;

require(address(_lendingPoolAddressesProviderRegistry).isContract(), "ATokenYieldSource/lendingPoolRegistry-not-contract-address");
lendingPoolAddressesProviderRegistry = _lendingPoolAddressesProviderRegistry;

__Ownable_init();
transferOwnership(_owner);

__ERC20_init(_name,_symbol);
require(_decimals > 0, "ATokenYieldSource/decimals-gt-zero");
require(_decimals > 0, "ATokenYieldSource/decimals-not-greater-than-zero");
_setupDecimals(_decimals);

emit ATokenYieldSourceInitialized (
Expand Down Expand Up @@ -171,7 +177,7 @@ contract ATokenYieldSource is ERC20Upgradeable, IProtocolYieldSource, AssetManag

/// @notice Deposit asset tokens to Aave
/// @param mintAmount The amount of asset tokens to be deposited
/// @return 0 if successful
/// @return 0 if successful
function _depositToAave(uint256 mintAmount) internal returns (uint256) {
IERC20Upgradeable _depositToken = IERC20Upgradeable(_tokenAddress());

Expand All @@ -189,7 +195,7 @@ contract ATokenYieldSource is ERC20Upgradeable, IProtocolYieldSource, AssetManag
function supplyTokenTo(uint256 mintAmount, address to) external override nonReentrant {
uint256 shares = _tokenToShares(mintAmount);

require(shares > 0, "ATokenYieldSource/shares-equal-zero");
require(shares > 0, "ATokenYieldSource/shares-not-greater-than-zero");
_depositToAave(mintAmount);
_mint(to, shares);

Expand Down
149 changes: 118 additions & 31 deletions test/ATokenYieldSource.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,22 @@ describe('ATokenYieldSource', () => {
let erc20Token: ERC20;
let underlyingToken: ERC20;

// Numerical error tests for shares decreasing
let isInitializeTest = false;

const initializeATokenYieldSource = async (
aTokenAddress: string,
lendingPoolAddressesProviderRegistryAddress: string,
decimals: number,
) => {
await aTokenYieldSource.initialize(
aTokenAddress,
lendingPoolAddressesProviderRegistryAddress,
decimals,
'Test',
'TEST',
yieldSourceOwner.address,
);
};

beforeEach(async () => {
const { deployMockContract } = waffle;
Expand Down Expand Up @@ -87,25 +102,88 @@ describe('ATokenYieldSource', () => {

debug('deploying ATokenYieldSource instance...');

const ATokenYieldSource = await ethers.getContractFactory(
'ATokenYieldSourceHarness',
);
const hardhatATokenYieldSourceHarness = await ATokenYieldSource.deploy()
const ATokenYieldSource = await ethers.getContractFactory('ATokenYieldSourceHarness');
const hardhatATokenYieldSourceHarness = await ATokenYieldSource.deploy();

aTokenYieldSource = (await ethers.getContractAt(
'ATokenYieldSourceHarness',
hardhatATokenYieldSourceHarness.address,
contractsOwner,
)) as ATokenYieldSourceHarness;

const initializeTx = await aTokenYieldSource.initialize(
aToken.address,
lendingPoolAddressesProviderRegistry.address,
18,
"Test",
"TEST",
yieldSourceOwner.address
);
'ATokenYieldSourceHarness',
hardhatATokenYieldSourceHarness.address,
contractsOwner,
)) as ATokenYieldSourceHarness;

if (!isInitializeTest) {
await initializeATokenYieldSource(
aToken.address,
lendingPoolAddressesProviderRegistry.address,
18,
);
}
});

describe('initialize()', () => {
let randomWalletAddress: string;

before(() => {
isInitializeTest = true;
});

beforeEach(() => {
randomWalletAddress = ethers.Wallet.createRandom().address;
});

after(() => {
isInitializeTest = false;
});

it('should fail if aToken is not a contract', async () => {
await expect(
initializeATokenYieldSource(
randomWalletAddress,
lendingPoolAddressesProviderRegistry.address,
18,
),
).to.be.revertedWith('ATokenYieldSource/aToken-not-contract-address');
});

it('should fail if aToken is address zero', async () => {
await expect(
initializeATokenYieldSource(
ethers.constants.AddressZero,
lendingPoolAddressesProviderRegistry.address,
18,
),
).to.be.revertedWith('ATokenYieldSource/aToken-not-contract-address');
});

it('should fail if lendingPoolAddressesProviderRegistry is not a contract', async () => {
await expect(
initializeATokenYieldSource(
aToken.address,
ethers.constants.AddressZero,
18,
),
).to.be.revertedWith('ATokenYieldSource/lendingPoolRegistry-not-contract-address');
});

it('should fail if lendingPoolAddressesProviderRegistry is address zero', async () => {
await expect(
initializeATokenYieldSource(
aToken.address,
ethers.constants.AddressZero,
18,
),
).to.be.revertedWith('ATokenYieldSource/lendingPoolRegistry-not-contract-address');
});

it('should fail if token decimal is not greater than 0', async () => {
await expect(
initializeATokenYieldSource(
aToken.address,
lendingPoolAddressesProviderRegistry.address,
0,
),
).to.be.revertedWith('ATokenYieldSource/decimals-not-greater-than-zero');
});
});

describe('create()', () => {
Expand Down Expand Up @@ -146,7 +224,7 @@ describe('ATokenYieldSource', () => {
});

it('should return 0 if tokens param is 0', async () => {
expect(await aTokenYieldSource.tokenToShares("0")).to.equal("0");
expect(await aTokenYieldSource.tokenToShares('0')).to.equal('0');
});

it('should return tokens if totalSupply is 0', async () => {
Expand All @@ -159,7 +237,9 @@ describe('ATokenYieldSource', () => {
.withArgs(aTokenYieldSource.address)
.returns(toWei('0.000000000000000005'));

expect(await aTokenYieldSource.tokenToShares(toWei('0.000000000000000005'))).to.equal(toWei('1'));
expect(await aTokenYieldSource.tokenToShares(toWei('0.000000000000000005'))).to.equal(
toWei('1'),
);
});

it('should return shares even if aToken total supply increases', async () => {
Expand All @@ -169,21 +249,27 @@ describe('ATokenYieldSource', () => {

expect(await aTokenYieldSource.tokenToShares(toWei('1'))).to.equal(toWei('2'));

await aToken.mock.balanceOf.withArgs(aTokenYieldSource.address).returns(ethers.utils.parseUnits('100', 36));
await aToken.mock.balanceOf
.withArgs(aTokenYieldSource.address)
.returns(ethers.utils.parseUnits('100', 36));
expect(await aTokenYieldSource.tokenToShares(toWei('1'))).to.equal(2);
});

it('should fail to return shares if aToken total supply increases too much', async () => { // failing here

it('should fail to return shares if aToken total supply increases too much', async () => {
// failing here

await aTokenYieldSource.mint(yieldSourceOwner.address, toWei('100'));
await aTokenYieldSource.mint(wallet2.address, toWei('100'));
await aToken.mock.balanceOf.withArgs(aTokenYieldSource.address).returns(toWei('100'));

expect(await aTokenYieldSource.tokenToShares(toWei('1'))).to.equal(toWei('2'));

await aToken.mock.balanceOf.withArgs(aTokenYieldSource.address).returns(ethers.utils.parseUnits('100', 37));
await expect(aTokenYieldSource.supplyTokenTo(toWei('1'), wallet2.address)).to.be.revertedWith('ATokenYieldSource/shares-equal-zero');

await aToken.mock.balanceOf
.withArgs(aTokenYieldSource.address)
.returns(ethers.utils.parseUnits('100', 37));
await expect(aTokenYieldSource.supplyTokenTo(toWei('1'), wallet2.address)).to.be.revertedWith(
'ATokenYieldSource/shares-not-greater-than-zero',
);
});
});

Expand All @@ -204,7 +290,9 @@ describe('ATokenYieldSource', () => {
await aTokenYieldSource.mint(yieldSourceOwner.address, toWei('0.000000000000000005'));
await aToken.mock.balanceOf.withArgs(aTokenYieldSource.address).returns(toWei('100'));

expect(await aTokenYieldSource.sharesToToken(toWei('0.000000000000000005'))).to.equal(toWei('100'));
expect(await aTokenYieldSource.sharesToToken(toWei('0.000000000000000005'))).to.equal(
toWei('100'),
);
});

it('should return tokens even if aToken total supply increases', async () => {
Expand All @@ -214,7 +302,9 @@ describe('ATokenYieldSource', () => {

expect(await aTokenYieldSource.sharesToToken(toWei('2'))).to.equal(toWei('1'));

await aToken.mock.balanceOf.withArgs(aTokenYieldSource.address).returns(ethers.utils.parseUnits('100', 36));
await aToken.mock.balanceOf
.withArgs(aTokenYieldSource.address)
.returns(ethers.utils.parseUnits('100', 36));
expect(await aTokenYieldSource.sharesToToken(2)).to.equal(toWei('1'));
});
});
Expand Down Expand Up @@ -441,6 +531,3 @@ describe('ATokenYieldSource', () => {
});
});
});
function toHex(arg0: number) {
throw new Error('Function not implemented.');
}

0 comments on commit e113c76

Please sign in to comment.