diff --git a/.gas-snapshot b/.gas-snapshot index 323dd5c..e7bfe77 100644 --- a/.gas-snapshot +++ b/.gas-snapshot @@ -34,11 +34,11 @@ TimeLockTest:test_OnlyAdmin_CanQueue() (gas: 16143) TimeLockTest:test_QueueThenExecute() (gas: 55711) TimeLockTest:test_Queue_RevertOnDuplicate() (gas: 46068) TokenTest:testFuzz_Approve(address,uint256) (runs: 1000, μ: 36452, ~: 36831) -TokenTest:testFuzz_Transfer_PreservesTotalSupply(uint256) (runs: 1000, μ: 47212, ~: 47844) -TokenTest:test_Approve_AndTransferFrom() (gas: 72961) -TokenTest:test_InfiniteAllowance_NotDecremented() (gas: 69946) +TokenTest:testFuzz_Transfer_PreservesTotalSupply(uint256) (runs: 1000, μ: 47249, ~: 47882) +TokenTest:test_Approve_AndTransferFrom() (gas: 73017) +TokenTest:test_InfiniteAllowance_NotDecremented() (gas: 70002) TokenTest:test_Metadata() (gas: 21459) -TokenTest:test_Transfer() (gas: 44638) +TokenTest:test_Transfer() (gas: 44676) TokenTest:test_TransferFrom_RevertOnInsufficientAllowance() (gas: 39482) TokenTest:test_Transfer_RevertOnInsufficientBalance() (gas: 15571) TokenTest:test_Transfer_RevertOnZeroAddress() (gas: 11220) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd31995..4156064 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,9 @@ jobs: - name: Install Foundry uses: foundry-rs/foundry-toolchain@v1 + - name: Check formatting + run: forge fmt --check + - name: Install dependencies run: forge install @@ -30,5 +33,5 @@ jobs: - name: Run invariant tests run: forge test -vvv --match-contract Invariant - - name: Gas snapshot + - name: Gas snapshot (informational) run: forge snapshot diff --git a/.gitignore b/.gitignore index d517577..24646bb 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,6 @@ out/ cache/ broadcast/ -lib/ .env .env.* !.env.example diff --git a/.gitmodules b/.gitmodules new file mode 100644 index 0000000..888d42d --- /dev/null +++ b/.gitmodules @@ -0,0 +1,3 @@ +[submodule "lib/forge-std"] + path = lib/forge-std + url = https://github.com/foundry-rs/forge-std diff --git a/README.md b/README.md index 8230eab..cf51801 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # foundry-testing -Foundry workspace built around three small contracts. The interesting part is the test suite: unit tests, fuzz tests, and a stateful invariant suite with a handler. Full suite runs in ~130ms across 13 tests, including 256 invariant runs and 3,840 random calls. +Foundry workspace built around seven small contracts. The interesting part is the test suite: unit tests, fuzz tests, and stateful invariant suites with handlers. The full suite runs in under 200ms across 51 tests, including invariant runs that exercise thousands of random call sequences. ## Stack @@ -29,15 +29,26 @@ forge snapshot forge test --gas-report ``` +Formatting and the gas baseline are enforced in CI: + +```bash +forge fmt --check +forge snapshot --check +``` + ## What's in here -| Contract | Test types | -|---|---| -| `src/Counter.sol` | unit, fuzz | -| `src/Vault.sol` | unit, fuzz, invariant (with `VaultHandler.sol`) | -| `src/Staking.sol` | unit, fuzz | +| Contract | Description | Test types | +|---|---|---| +| `src/Counter.sol` | Trivial counter | unit, fuzz | +| `src/Token.sol` | Minimal ERC20-style token | unit, fuzz | +| `src/NFT.sol` | Minimal ERC721-style NFT | unit, fuzz | +| `src/Vault.sol` | ETH vault | unit, fuzz, invariant | +| `src/Staking.sol` | ETH staking | unit, fuzz, invariant | +| `src/TimeLock.sol` | Queue/execute/cancel timelock | unit, fuzz | +| `src/Ownable.sol` | Two-step ownership mixin | unit | -Tests live in `test/`. The invariant handler in `test/VaultHandler.sol` constrains the random call surface so invariants converge instead of bouncing off reverts. +Tests live in `test/`. The invariant handler in `test/VaultInvariant.t.sol` constrains the random call surface (bounded amounts, a fixed actor set) so invariants converge instead of bouncing off reverts, and it tracks ghost variables to cross-check the contract's own accounting. ## Why Foundry over Hardhat diff --git a/foundry.lock b/foundry.lock new file mode 100644 index 0000000..7521bfd --- /dev/null +++ b/foundry.lock @@ -0,0 +1,8 @@ +{ + "lib/forge-std": { + "tag": { + "name": "v1.16.1", + "rev": "620536fa5277db4e3fd46772d5cbc1ea0696fb43" + } + } +} \ No newline at end of file diff --git a/foundry.toml b/foundry.toml index d497abf..a0889af 100644 --- a/foundry.toml +++ b/foundry.toml @@ -3,6 +3,7 @@ src = "src" out = "out" libs = ["lib"] solc = "0.8.24" +evm_version = "cancun" optimizer = true optimizer_runs = 200 diff --git a/lib/forge-std b/lib/forge-std new file mode 160000 index 0000000..620536f --- /dev/null +++ b/lib/forge-std @@ -0,0 +1 @@ +Subproject commit 620536fa5277db4e3fd46772d5cbc1ea0696fb43 diff --git a/src/NFT.sol b/src/NFT.sol index 76e6eab..22072e2 100644 --- a/src/NFT.sol +++ b/src/NFT.sol @@ -35,7 +35,9 @@ contract NFT { require(to != address(0), "NFT: mint to zero"); require(_owners[tokenId] == address(0), "NFT: already minted"); _owners[tokenId] = to; - unchecked { _balances[to] += 1; } + unchecked { + _balances[to] += 1; + } emit Transfer(address(0), to, tokenId); } @@ -56,9 +58,7 @@ contract NFT { address owner = ownerOf(tokenId); require(owner == from, "NFT: from is not owner"); require( - msg.sender == owner || - isApprovedForAll[owner][msg.sender] || - getApproved[tokenId] == msg.sender, + msg.sender == owner || isApprovedForAll[owner][msg.sender] || getApproved[tokenId] == msg.sender, "NFT: not authorized" ); diff --git a/src/TimeLock.sol b/src/TimeLock.sol index 9936d8e..f895cdc 100644 --- a/src/TimeLock.sol +++ b/src/TimeLock.sol @@ -33,7 +33,11 @@ contract TimeLock { return keccak256(abi.encode(target, value, data, salt)); } - function queue(address target, uint256 value, bytes calldata data, bytes32 salt) external onlyAdmin returns (bytes32 id) { + function queue(address target, uint256 value, bytes calldata data, bytes32 salt) + external + onlyAdmin + returns (bytes32 id) + { id = hashOp(target, value, data, salt); if (queuedAt[id] != 0) revert AlreadyQueued(); uint256 eta = block.timestamp + delay; @@ -47,7 +51,12 @@ contract TimeLock { emit Cancelled(id); } - function execute(address target, uint256 value, bytes calldata data, bytes32 salt) external payable onlyAdmin returns (bytes memory) { + function execute(address target, uint256 value, bytes calldata data, bytes32 salt) + external + payable + onlyAdmin + returns (bytes memory) + { bytes32 id = hashOp(target, value, data, salt); uint256 eta = queuedAt[id]; if (eta == 0) revert NotQueued(); diff --git a/test/Token.t.sol b/test/Token.t.sol index d2263f0..e0f536d 100644 --- a/test/Token.t.sol +++ b/test/Token.t.sol @@ -14,7 +14,7 @@ contract TokenTest is Test { function setUp() public { token = new Token("Test", "TST", INITIAL); // Move the entire supply to alice so tests start from a known state - token.transfer(alice, INITIAL); + assertTrue(token.transfer(alice, INITIAL)); } function test_Metadata() public view { @@ -26,7 +26,7 @@ contract TokenTest is Test { function test_Transfer() public { vm.prank(alice); - token.transfer(bob, 100 ether); + assertTrue(token.transfer(bob, 100 ether)); assertEq(token.balanceOf(alice), INITIAL - 100 ether); assertEq(token.balanceOf(bob), 100 ether); @@ -50,7 +50,7 @@ contract TokenTest is Test { assertEq(token.allowance(alice, bob), 50 ether); vm.prank(bob); - token.transferFrom(alice, bob, 30 ether); + assertTrue(token.transferFrom(alice, bob, 30 ether)); assertEq(token.balanceOf(bob), 30 ether); assertEq(token.allowance(alice, bob), 20 ether); @@ -70,7 +70,7 @@ contract TokenTest is Test { token.approve(bob, type(uint256).max); vm.prank(bob); - token.transferFrom(alice, bob, 100 ether); + assertTrue(token.transferFrom(alice, bob, 100 ether)); // Infinite allowance should be preserved across transfers assertEq(token.allowance(alice, bob), type(uint256).max); @@ -79,7 +79,7 @@ contract TokenTest is Test { function testFuzz_Transfer_PreservesTotalSupply(uint256 amount) public { amount = bound(amount, 0, INITIAL); vm.prank(alice); - token.transfer(bob, amount); + assertTrue(token.transfer(bob, amount)); assertEq(token.balanceOf(alice) + token.balanceOf(bob), INITIAL); assertEq(token.totalSupply(), INITIAL); diff --git a/test/VaultInvariant.t.sol b/test/VaultInvariant.t.sol index 9dd36dd..cc07c85 100644 --- a/test/VaultInvariant.t.sol +++ b/test/VaultInvariant.t.sol @@ -67,10 +67,7 @@ contract VaultInvariantTest is Test { /// Ghost accounting: net flow (deposits - withdrawals) must equal contract balance function invariant_netFlowEqualsBalance() public view { - assertEq( - address(vault).balance, - handler.ghostTotalDeposits() - handler.ghostTotalWithdrawals() - ); + assertEq(address(vault).balance, handler.ghostTotalDeposits() - handler.ghostTotalWithdrawals()); } /// totalAssets() is a thin view but should never lie