Skip to content

Commit

Permalink
fix(TokenVault): improve unit tests
Browse files Browse the repository at this point in the history
  • Loading branch information
PierrickGT committed May 27, 2022
1 parent 181e3c7 commit b945745
Show file tree
Hide file tree
Showing 3 changed files with 208 additions and 68 deletions.
61 changes: 41 additions & 20 deletions contracts/TokenVault.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,54 +7,75 @@ import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";

/**
* @title PoolTogether TokenVault
* @author PoolTogether Inc Team
* @title PoolTogether TokenVault
* @author PoolTogether Inc Team
* @notice The TokenVault contract stores ERC20 tokens that are swapped through the PrizePoolLiquidator contract.
* Stakers are then able to claim their share of rewards by interacting with the GaugeReward contract.
* Rewards are then transferred directly from the TokenVault to the staker account.
*/
contract TokenVault is Manageable {
using SafeERC20 for IERC20;

/// @notice Tracks addresses approved to spend tokens from the vault.
mapping(address => bool) public approved;

/**
* @notice Emitted when a `spender` address is approved to spend tokens from the vault.
* @param spender Address that is approved to spend tokens from the vault
* @param approved Whether the spender is approved to spend tokens from the vault or not
*/
event Approved(address indexed spender, bool approved);

/**
* @notice Constructs TokenVault
* @param _owner Owner address
*/
constructor(address _owner) Ownable(_owner) {}
constructor(address _owner) Ownable(_owner) {
require(_owner != address(0), "TVault/owner-not-zero-address");
}

function setApproved(address _account, bool _approved) external onlyOwner {
approved[_account] = _approved;
/**
* @notice Approves the given `spender` address to spend ERC20 tokens from the vault.
* @dev Only callable by the owner.
* @param _spender Address that will spend the tokens
* @param _approve Whether to approve `spender` or not
*/
function setApproval(address _spender, bool _approve) external onlyOwner {
approved[_spender] = _approve;
emit Approved(_spender, _approve);
}

/**
* @notice Decrease allowance of ERC20 tokens held by this contract.
* @dev Only callable by the owner or asset manager.
* @dev Current allowance should be computed off-chain to avoid any underflow.
* @param token Address of the ERC20 token to decrease allowance for
* @param spender Address of the spender of the tokens
* @param amount Amount of tokens to decrease allowance by
* @param _token Address of the ERC20 token to decrease allowance for
* @param _spender Address of the spender of the tokens
* @param _amount Amount of tokens to decrease allowance by
*/
function decreaseERC20Allowance(
IERC20 token,
address spender,
uint256 amount
IERC20 _token,
address _spender,
uint256 _amount
) external onlyManagerOrOwner {
token.safeDecreaseAllowance(spender, amount);
_token.safeDecreaseAllowance(_spender, _amount);
}

/**
* @notice Increase allowance of ERC20 tokens held by this contract.
* @dev Only callable by the owner or asset manager.
* @dev Allowance can only be increased for approved `spender` addresses.
* @dev Current allowance should be computed off-chain to avoid any overflow.
* @param token Address of the ERC20 token to increase allowance for
* @param spender Address of the spender of the tokens
* @param amount Amount of tokens to increase allowance by
* @param _token Address of the ERC20 token to increase allowance for
* @param _spender Address of the spender of the tokens
* @param _amount Amount of tokens to increase allowance by
*/
function increaseERC20Allowance(
IERC20 token,
address spender,
uint256 amount
IERC20 _token,
address _spender,
uint256 _amount
) external onlyManagerOrOwner {
require(approved[spender], "Spender must be approved");
token.safeIncreaseAllowance(spender, amount);
require(approved[_spender], "TVault/spender-not-approved");
_token.safeIncreaseAllowance(_spender, _amount);
}
}
3 changes: 1 addition & 2 deletions test/GaugeReward.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ describe('GaugeReward', () => {
let usdcToken: Contract;

let owner: SignerWithAddress;
let wallet2: SignerWithAddress;
let liquidator: SignerWithAddress;

let constructorTest = false;
Expand Down Expand Up @@ -81,7 +80,7 @@ describe('GaugeReward', () => {
};

beforeEach(async () => {
[owner, wallet2, liquidator] = await getSigners();
[owner, liquidator] = await getSigners();

gaugeAddress = '0xDe3825B1309E823D52C677E4981a1c67fF0d03E5';

Expand Down
212 changes: 166 additions & 46 deletions test/TokenVault.test.ts
Original file line number Diff line number Diff line change
@@ -1,72 +1,192 @@
import { SignerWithAddress } from '@nomiclabs/hardhat-ethers/signers';
import { expect } from 'chai';
import { deployMockContract } from 'ethereum-waffle';
import { Contract, Signer } from 'ethers';
import { ethers, artifacts } from 'hardhat';
import { BigNumber, Contract, ContractFactory } from 'ethers';
import { ethers } from 'hardhat';

const { getSigners } = ethers;
const { constants, getContractFactory, getSigners, utils } = ethers;
const { AddressZero, Zero } = constants;
const { parseEther: toWei } = utils;

describe('Vault', () => {
let TokenVaultFactory: ContractFactory;
let tokenVault: Contract;
let token: Contract;

let wallet1: SignerWithAddress;
let wallet2: SignerWithAddress;
let owner: SignerWithAddress;
let manager: SignerWithAddress;
let wallet3: SignerWithAddress;
let wallet4: SignerWithAddress;

let constructorTest = false;

beforeEach(async () => {
[wallet1, wallet2, wallet3, wallet4] = await getSigners();
[owner, manager, wallet3, wallet4] = await getSigners();

const ERC20MintableContract = await getContractFactory('ERC20Mintable', owner);
token = await ERC20MintableContract.deploy('PoolTogether', 'POOL');

const IERC20Artifact = await artifacts.readArtifact('IERC20');
token = await deployMockContract(wallet1 as Signer, IERC20Artifact.abi);
TokenVaultFactory = await getContractFactory('TokenVault', owner);

const TokenVaultFactory = await ethers.getContractFactory('TokenVault', wallet1);
tokenVault = await TokenVaultFactory.deploy(wallet1.address);
if (!constructorTest) {
tokenVault = await TokenVaultFactory.deploy(owner.address);
}
});

describe('constructor', () => {
beforeEach(() => {
constructorTest = true;
});

afterEach(() => {
constructorTest = false;
});

it('should set the owner', async () => {
expect(await tokenVault.owner()).to.equal(wallet1.address)
})
})
tokenVault = await TokenVaultFactory.deploy(owner.address);
expect(await tokenVault.owner()).to.equal(owner.address);
});

it('should fail if owner is address zero', async () => {
await expect(TokenVaultFactory.deploy(AddressZero)).to.be.revertedWith(
'TVault/owner-not-zero-address',
);
});
});

describe('setApproval()', () => {
it('should allow the owner to approve accounts', async () => {
await tokenVault.setApproved(wallet2.address, true)
expect(await tokenVault.approved(wallet2.address)).to.equal(true)
})
})
await expect(tokenVault.setApproval(wallet3.address, true))
.to.emit(tokenVault, 'Approved')
.withArgs(wallet3.address, true);

expect(await tokenVault.approved(wallet3.address)).to.equal(true);
});

it('should fail if not owner', async () => {
await expect(
tokenVault.connect(manager).setApproval(wallet3.address, true),
).to.be.revertedWith('Ownable/caller-not-owner');
});
});

describe('increaseERC20Allowance()', () => {
it('should allow owners to increase approval amount', async () => {
await tokenVault.setApproved(wallet2.address, true)
await token.mock.allowance.withArgs(tokenVault.address, wallet2.address).returns('0')
await token.mock.approve.withArgs(wallet2.address, '1111').returns(true)
await tokenVault.increaseERC20Allowance(token.address, wallet2.address, '1111')
})

it('should allow managers to increase approval amount', async () => {
await tokenVault.setManager(wallet3.address)
await tokenVault.setApproved(wallet2.address, true)
await token.mock.allowance.withArgs(tokenVault.address, wallet2.address).returns('0')
await token.mock.approve.withArgs(wallet2.address, '1111').returns(true)
await tokenVault.connect(wallet3).increaseERC20Allowance(token.address, wallet2.address, '1111')
})
})
let increaseAllowanceAmount: BigNumber;

beforeEach(async () => {
increaseAllowanceAmount = toWei('1111');
});

it('should allow owner to increase approval amount', async () => {
await tokenVault.setApproval(wallet3.address, true);
await tokenVault.increaseERC20Allowance(
token.address,
wallet3.address,
increaseAllowanceAmount,
);

expect(await token.allowance(tokenVault.address, wallet3.address)).to.equal(
increaseAllowanceAmount,
);
});

it('should allow manager to increase approval amount', async () => {
await tokenVault.setApproval(wallet3.address, true);
await tokenVault.setManager(manager.address);

await tokenVault
.connect(manager)
.increaseERC20Allowance(token.address, wallet3.address, increaseAllowanceAmount);

expect(await token.allowance(tokenVault.address, wallet3.address)).to.equal(
increaseAllowanceAmount,
);
});

it('should fail if spender is not approved', async () => {
await expect(
tokenVault.increaseERC20Allowance(
token.address,
wallet3.address,
increaseAllowanceAmount,
),
).to.be.revertedWith('TVault/spender-not-approved');
});

it('should fail if not owner or manager', async () => {
await tokenVault.setManager(wallet3.address);

await expect(
tokenVault
.connect(wallet4)
.increaseERC20Allowance(
token.address,
manager.address,
increaseAllowanceAmount,
),
).to.be.revertedWith('Manageable/caller-not-manager-or-owner');
});
});

describe('decreaseERC20Allowance()', () => {
it('should allow manager to decrease approval amount', async () => {
await token.mock.allowance.withArgs(tokenVault.address, wallet2.address).returns('1111')
await token.mock.approve.withArgs(wallet2.address, '111').returns(true)
await tokenVault.decreaseERC20Allowance(token.address, wallet2.address, '1000')
})

it('should allow manager to decrease approval amount', async () => {
await tokenVault.setManager(wallet3.address)
await token.mock.allowance.withArgs(tokenVault.address, wallet2.address).returns('1111')
await token.mock.approve.withArgs(wallet2.address, '0').returns(true)
await tokenVault.connect(wallet3).decreaseERC20Allowance(token.address, wallet2.address, '1111')
})
})
let decreaseAllowanceAmount: BigNumber;
let increaseAllowanceAmount: BigNumber;

beforeEach(async () => {
decreaseAllowanceAmount = toWei('111');
increaseAllowanceAmount = toWei('1111');

await tokenVault.setApproval(wallet3.address, true);
await tokenVault.increaseERC20Allowance(
token.address,
wallet3.address,
increaseAllowanceAmount,
);
});

it('should allow owner to decrease approval amount', async () => {
await tokenVault.decreaseERC20Allowance(
token.address,
wallet3.address,
decreaseAllowanceAmount,
);

expect(await token.allowance(tokenVault.address, wallet3.address)).to.equal(
increaseAllowanceAmount.sub(decreaseAllowanceAmount),
);
});

it('should allow owner to decrease approval amount', async () => {
await tokenVault.setManager(manager.address);

await tokenVault
.connect(manager)
.decreaseERC20Allowance(token.address, wallet3.address, decreaseAllowanceAmount);

expect(await token.allowance(tokenVault.address, wallet3.address)).to.equal(
increaseAllowanceAmount.sub(decreaseAllowanceAmount),
);
});

it('should decrease the full approval amount', async () => {
await tokenVault.decreaseERC20Allowance(
token.address,
wallet3.address,
increaseAllowanceAmount,
);

expect(await token.allowance(tokenVault.address, wallet3.address)).to.equal(Zero);
});

it('should fail if not owner of manager', async () => {
await expect(
tokenVault
.connect(wallet4)
.decreaseERC20Allowance(
token.address,
wallet3.address,
decreaseAllowanceAmount,
),
).to.be.revertedWith('Manageable/caller-not-manager-or-owner');
});
});
});

0 comments on commit b945745

Please sign in to comment.