// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import "yield-utils-v2/contracts/token/IERC20.sol";
/**
@title An on-chain name registry
@author Calnix
@dev Vault for a specific ERC20 token; token address to be passed to constructor.
@notice Vault for a pre-defined ERC20 token that was set on deployment.
*/
contract BasicVault {
///@notice ERC20 interface specifying token contract functions
///@dev For constant variables, the value has to be fixed at compile-time, while for immutable, it can still be assigned at construction time.
IERC20 public immutable wmdToken;
///@notice mapping addresses to their respective token balances
mapping(address => uint) public balances;
/// @notice Emit event when tokens are deposited into Vault
event Deposited(address indexed from, uint amount);
/// @notice Emit event when tokens are withdrawn from Vault
event Withdrawal(address indexed from, uint amount);
///@param wmdToken_ ERC20 contract address
constructor(IERC20 wmdToken_){
wmdToken = wmdToken_;
}
/// @notice User can deposit tokens into Vault
/// @dev Expect Deposit to revert if transferFrom fails
/// @param amount The amount of tokens to deposit
function deposit(uint amount) external {
balances[msg.sender] += amount;
bool success = wmdToken.transferFrom(msg.sender, address(this), amount);
require(success, "Deposit failed!");
emit Deposited(msg.sender, amount);
}
/// @notice User can withdraw tokens from Vault
/// @dev Expect Withdraw to revert if transfer fails
/// @param amount The amount of tokens to withdraw
function withdraw(uint amount) external {
balances[msg.sender] -= amount;
bool success = wmdToken.transfer(msg.sender, amount);
require(success, "Withdrawal failed!");
emit Withdrawal(msg.sender, amount);
}
}
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import "yield-utils-v2/contracts/mocks/ERC20Mock.sol";
contract WMDToken is ERC20Mock {
///@dev Inherited constructor from ERC20Mock.sol.
///@dev No parameters need to be passed for top-level constructor.
constructor() ERC20Mock("WoMiGawd", "WMD") {
}
}
Checks-effect-interaction pattern
Why does it matter?
When calling an external address, for example when transferring tokens to another account, the calling contract is also transferring the control flow to the external entity.
Assuming this external entity is a smart contract as well, the external entity is now in charge of the control flow and can execute any inherent code within it.
This can leave your contract open to re-entrancy attacks.
The high-level idea is as follows:
check & update the internal states (balances)
keep external interactions (function calls) to the last step
Essentially we will update our mappings balances to reflect any outflow, before initiating the transfer.
Testing
Vault contract should be fully tested.
No need to test ERC20Mock. Assume it has been battle-tested.
WMDTokenWithFailedTransfers.sol is used to simulate token transfer failure, so that we can ensure our Vault functions behave accordingly in such situations.
Testing States
StateZero (user has no tokens)
cannot deposit (testUserCannotWithdraw)
cannot withdraw (testUserCannotDeposit)
fuzz testing (testUserMintApproveDeposit)
StateTokensMinted (user has minted 100 wmd tokens - only action available is deposit)
if transfer fails, deposit should revert (testDepositRevertsIfTransferFails)
deposit tokens into vault (testDeposit)
StateTokensDeposited (user has deposited tokens into Vault)
cannot withdraw if transfer fails (testWithdrawRevertsIfTransferFails)
cannot withdraw more than deposit (testUserCannotWithdrawExcessOfDeposit)
partial withdrawal (testUserWithdrawPartial)
full withdrawal (testUserWithdrawAll)
It’s better to test for a specific error type, than use vm.expectRevert as a catch-all leading to false positives.
As such, with regards to failing token transfers, observe the use of stdError.arithmeticError in both
testFuzzUserCannotWithdraw
testUserCannotWithdrawExcessOfDeposit
Testing Guidelines
All state variable changes in the contracts that you code.
All state variable changes in other contracts caused by calls from contracts that you code.
All require or revert in the contracts that you code.
All events being emitted.
All return values in contracts that you code.
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import "forge-std/Test.sol";
import "forge-std/console2.sol";
import 'src/BasicVault.sol';
import 'test/WMDTokenWithFailedTransfers.sol';
abstract contract StateZero is Test {
BasicVault public vault;
WMDTokenWithFailedTransfers public wmd;
address user;
event Deposited(address indexed from, uint amount);
event Withdrawal(address indexed from, uint amount);
function setUp() public virtual {
wmd = new WMDTokenWithFailedTransfers();
vault = new BasicVault(wmd);
user = address(1);
vm.label(user, "user");
}
}
contract StateZeroTest is StateZero {
function testGetUserBalance() public {
console2.log("Check User has zero tokens in vault");
vm.prank(user);
uint balance = vault.balances(user);
assertTrue(balance == 0);
}
function testUserCannotDeposit(uint amount) public {
console2.log("User cannot deposit without tokens");
vm.assume(amount > 0);
vm.prank(user);
vm.expectRevert("ERC20: Insufficient approval");
vault.deposit(amount);
}
function testUserMintApproveDeposit(uint amount) public {
console2.log("User mints tokens and deposits into vault");
vm.startPrank(user);
vm.assume(amount > 0);
wmd.mint(user, amount);
wmd.approve(address(vault), amount);
vm.expectEmit(true, false, false, true);
emit Deposited(user, amount);
vault.deposit(amount);
assertTrue(wmd.balanceOf(user) == 0);
assertTrue(vault.balances(user) == amount);
vm.stopPrank();
}
}
abstract contract StateMinted is StateZero {
uint userTokens;
function setUp() public override virtual {
super.setUp();
// state transition: user mints 100 tokens
userTokens = 100 * 10**18;
vm.prank(user);
wmd.mint(user, userTokens);
}
}
contract StateMintedTest is StateMinted {
function testFuzzUserCannotWithdraw(uint amount) public {
console2.log("User cannot withdraw with no balance");
vm.assume(amount > 0 && amount < wmd.balanceOf(user));
vm.prank(user);
vm.expectRevert(stdError.arithmeticError);
vault.withdraw(amount);
}
function testDepositRevertsIfTransferFails() public {
console2.log("Deposit transaction should revert if transfer fails");
wmd.setFailTransfers(true);
vm.startPrank(user);
wmd.approve(address(vault), userTokens);
vm.expectRevert("Deposit failed!");
vault.deposit(userTokens);
vm.stopPrank();
}
function testDeposit() public {
console2.log("User deposits into Vault");
vm.startPrank(user);
wmd.approve(address(vault), userTokens);
vm.expectEmit(true, false, false, true);
emit Deposited(user, userTokens);
vault.deposit(userTokens);
assertTrue(vault.balances(user) == userTokens);
assertTrue(wmd.balanceOf(user) == 0);
vm.stopPrank();
}
}
abstract contract StateDeposited is StateMinted {
function setUp() public override virtual {
super.setUp();
vm.startPrank(user);
wmd.approve(address(vault), userTokens);
vault.deposit(userTokens);
vm.stopPrank();
}
}
contract StateDepositedTest is StateDeposited {
function testWithdrawRevertsIfTransferFails() public {
console2.log("Withdraw transaction should revert if transfer fails");
wmd.setFailTransfers(true);
vm.prank(user);
vm.expectRevert("Withdrawal failed!");
vault.withdraw(userTokens);
}
function testUserCannotWithdrawExcessOfDeposit() public {
console2.log("User cannot withdraw more than he has deposited");
vm.prank(user);
vm.expectRevert(stdError.arithmeticError);
vault.withdraw(userTokens + 100*10**18);
}
function testUserWithdrawPartial() public {
console2.log("User withdraws half of deposit from Vault");
vm.prank(user);
vm.expectEmit(true, false, false, true);
emit Withdrawal(user, userTokens/2);
vault.withdraw(userTokens/2);
assertEq(vault.balances(user), userTokens/2);
assertEq(wmd.balanceOf(user), userTokens/2);
}
function testUserWithdrawAll() public {
console2.log("User to withdraw all deposits from Vault");
vm.prank(user);
vm.expectEmit(true, false, false, true);
emit Withdrawal(user, userTokens);
vault.withdraw(userTokens);
assertEq(vault.balances(user), 0);
assertEq(wmd.balanceOf(user), userTokens);
}
}
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;
import 'src/WMDToken.sol';
contract WMDTokenWithFailedTransfers is WMDToken {
bool public transferFail = false;
function setFailTransfers(bool state_) public {
transferFail = state_;
}
function _transfer(address src, address dst, uint wad) internal override returns (bool) {
if (transferFail) {
return false;
}
else {
return super._transfer(src, dst, wad);
}
}
}
I realized that for future testing perhaps splitting each state into a different file would be more sensible and accessible for the reader.
Github CI
foundry-ci.yml
name: Foundry-CI
on:
push:
branches:
- RevisedCode
pull_request:
jobs:
run-tests: # job_id value
name: Basic Vault # Use jobs.<job_id>.name to a name for the job, which is displayed on GitHub.
runs-on: ubuntu-latest # Container OS env
steps:
- uses: actions/checkout@v2
with:
submodules: recursive
- name: Install Foundry
uses: onbjerg/foundry-toolchain@v1
with:
version: nightly
- name: Run Forge tests
run: forge test
id: test
Its interesting to note that I could not apply this with windows-latest container. The action Install Foundry will return an Error: Unexpected HTTP response: 404)