From 06502613affbe5f5070196319313a1af53f3dd2c Mon Sep 17 00:00:00 2001 From: Peter Robinson Date: Thu, 16 May 2024 16:38:03 +1000 Subject: [PATCH] Revised test plans --- test/token/common/README.md | 14 ++++ .../ERC20MinterBurnerPermitCommon.t.sol | 28 +++---- .../ImmutableERC20FixedSupplyNoBurn.t.sol | 13 +++- .../ImmutableERC20MinterBurnerPermit.t.sol | 1 - test/token/erc20/preset/README.md | 76 ++++++++++++++----- 5 files changed, 94 insertions(+), 38 deletions(-) create mode 100644 test/token/common/README.md diff --git a/test/token/common/README.md b/test/token/common/README.md new file mode 100644 index 00000000..44893a2b --- /dev/null +++ b/test/token/common/README.md @@ -0,0 +1,14 @@ +# Test Plan for Common Token contracts + +## HubOwner.sol +This section defines tests for contracts/token/common/HubOwner.sol. +All of these tests are in test/token/common/HubOwner.t.sol. + +| Test name |Description | Happy Case | Implemented | +|---------------------------------| --------------------------------------------------|------------|-------------| +| testInit | Check that deployment work. | Yes | Yes | +| testRenounceAdmin | Check that default admins can call renounce. | Yes | Yes | +| testRenounceLastAdminBlocked | Check that the last admin can not call renounce. | No | Yes | +| testRenounceHubOwner | Check that hub owners can call renounce. | Yes | Yes | +| testRenounceLastHubOwnerBlocked | Check that the last hub owner can not call renounce. | No | Yes | +| testOwnerWhenNoHubOwner | Check operation when there are no hub owners. | No | Yes | diff --git a/test/token/erc20/preset/ERC20MinterBurnerPermitCommon.t.sol b/test/token/erc20/preset/ERC20MinterBurnerPermitCommon.t.sol index 60d16a83..4156585f 100644 --- a/test/token/erc20/preset/ERC20MinterBurnerPermitCommon.t.sol +++ b/test/token/erc20/preset/ERC20MinterBurnerPermitCommon.t.sol @@ -26,6 +26,14 @@ abstract contract ERC20MinterBurnerPermitCommonTest is ERC20TestCommon { assertTrue(erc20.hasRole(erc20.HUB_OWNER_ROLE(), hubOwner), "hub owner"); } + function testMint() public { + address to = makeAddr("to"); + uint256 amount = 100; + vm.prank(minter); + erc20.mint(to, amount); + assertEq(erc20.balanceOf(to), amount); + } + function testOnlyMinterCanMint() public { address to = makeAddr("to"); uint256 amount = 100; @@ -34,12 +42,15 @@ abstract contract ERC20MinterBurnerPermitCommonTest is ERC20TestCommon { erc20.mint(to, amount); } - function testMint() public { + function testCanOnlyMintUpToMaxSupply() public { address to = makeAddr("to"); - uint256 amount = 100; - vm.prank(minter); + uint256 amount = supply; + vm.startPrank(minter); erc20.mint(to, amount); assertEq(erc20.balanceOf(to), amount); + vm.expectRevert("ERC20Capped: cap exceeded"); + erc20.mint(to, 1); + vm.stopPrank(); } function testBurn() public { @@ -52,17 +63,6 @@ abstract contract ERC20MinterBurnerPermitCommonTest is ERC20TestCommon { assertEq(erc20.balanceOf(tokenReceiver), 0); } - function testCanOnlyMintUpToMaxSupply() public { - address to = makeAddr("to"); - uint256 amount = supply; - vm.startPrank(minter); - erc20.mint(to, amount); - assertEq(erc20.balanceOf(to), amount); - vm.expectRevert("ERC20Capped: cap exceeded"); - erc20.mint(to, 1); - vm.stopPrank(); - } - function testBurnFrom() public { uint256 amount = 100; address operator = makeAddr("operator"); diff --git a/test/token/erc20/preset/ImmutableERC20FixedSupplyNoBurn.t.sol b/test/token/erc20/preset/ImmutableERC20FixedSupplyNoBurn.t.sol index 12704b14..7a1dc8be 100644 --- a/test/token/erc20/preset/ImmutableERC20FixedSupplyNoBurn.t.sol +++ b/test/token/erc20/preset/ImmutableERC20FixedSupplyNoBurn.t.sol @@ -19,9 +19,16 @@ contract ImmutableERC20FixedSupplyNoBurnTest is ERC20TestCommon { assertEq(erc20.owner(), hubOwner, "Hub owner"); } - function testRenounceOwnership() public { - vm.startPrank(hubOwner); + function testChangeOwner() public { + address newOwner = makeAddr("newOwner"); + vm.prank(hubOwner); + erc20.transferOwnership(newOwner); + assertEq(erc20.owner(), newOwner, "new owner"); + } + + function testRenounceOwnerBlocked() public { + vm.prank(hubOwner); vm.expectRevert(abi.encodeWithSelector(ImmutableERC20FixedSupplyNoBurn.RenounceOwnershipNotAllowed.selector)); erc20.renounceOwnership(); - } + } } diff --git a/test/token/erc20/preset/ImmutableERC20MinterBurnerPermit.t.sol b/test/token/erc20/preset/ImmutableERC20MinterBurnerPermit.t.sol index d1958a59..8ebd875f 100644 --- a/test/token/erc20/preset/ImmutableERC20MinterBurnerPermit.t.sol +++ b/test/token/erc20/preset/ImmutableERC20MinterBurnerPermit.t.sol @@ -19,7 +19,6 @@ contract ImmutableERC20MinterBurnerPermitTest is ERC20MinterBurnerPermitCommonTe basicERC20 = IERC20Metadata(address(erc20)); } - function testRenounceAdmin() public { address secondAdmin = makeAddr("secondAdmin"); vm.startPrank(admin); diff --git a/test/token/erc20/preset/README.md b/test/token/erc20/preset/README.md index 2f5c7b57..1d4c9e18 100644 --- a/test/token/erc20/preset/README.md +++ b/test/token/erc20/preset/README.md @@ -1,37 +1,73 @@ # Test Plan for Immutable ERC20 Preset contracts -## ImmutableERC20FixedSupplyNoBurn.sol -This section defines tests for contracts/erc20/preset/ImmutableERC20FixedSupplyNoBurn.sol. Note -that this contract extends Open Zeppelin's ERC 20 contract which is extensively tested here: +The ERC 20 contracts test the additional features supplied over and above the Open Zeppelin contracts. +These base contracts are extensively tested here: https://github.com/OpenZeppelin/openzeppelin-contracts/tree/release-v4.9/test/token/ERC20 . + +## Common Tests +ERC20TestCommon.t.sol provides a test that is common to all ERC20 contracts: checking initialisation. + +ERC20MinternBurnerPermitCommon.t.sol provides tests used by both ImmutableERC20MinterBurnerPermit.t.sol +and ImmutableERC20MinterBurnerPermitV2.t.sol. The ERC20MinternBurnerPermitCommon.t.sol tests are shown below: + +| Test name |Description | Happy Case | Implemented | +|---------------------------------| --------------------------------------------------|------------|-------------| +| testInitExtended | Check initialisation. | Yes | Yes | +| testMint | Ensure successful minting by minter | Yes | Yes | +| testOnlyMinterCanMint | Ensure Only minter role can mint reverts. | No | Yes | +| testCanOnlyMintUpToMaxSupply | Ensure can only mint up to max supply | No | Yes | +| testBurn | Ensure allowance is required to burn | Yes | Yes | +| testBurnFrom | Ensure allowance is required to burnFrom | Yes | Yes | +| testPermit | Ensure Permit works | Yes | Yes | + + +## ImmutableERC20FixedSupplyNoBurn.sol +This section defines tests for contracts/erc20/preset/ImmutableERC20FixedSupplyNoBurn.sol. All of the tests defined in the table below are in test/erc20/preset/ImmutableERC20FixedSupplyNoBurn.t.sol. | Test name |Description | Happy Case | Implemented | |---------------------------------| --------------------------------------------------|------------|-------------| -| testInit | Check constructor. | Yes | Yes | +| testInitExtended | Check constructor. | Yes | Yes | | testChangeOwner | Check change ownership. | Yes | Yes | | testRenounceOwnershipBlocked | Ensure renounceOwnership reverts. | No | Yes | +## ImmutableERC20FixedSupplyNoBurnV2.sol +This section defines tests for contracts/erc20/preset/ImmutableERC20FixedSupplyNoBurnV2.sol. +All of the tests defined in the table below are in test/erc20/preset/ImmutableERC20FixedSupplyNoBurnV2.t.sol. +Note that ImmutableERC20FixedSupplyNoBurnV2 extends HubOwner.sol. The ownership features reside in +HubOwner.sol, and hence are tested in HubOwner.t.sol. -## ImmutableERC20MinterBurnerPermit.sol -This section defines tests for contracts/erc20/preset/ImmutableERC20MinterBurnerPermit.sol. Note -that this contract extends Open Zeppelin's ERC 20 contract which is extensively tested here: -https://github.com/OpenZeppelin/openzeppelin-contracts/tree/release-v4.9/test/token/ERC20 . +| Test name |Description | Happy Case | Implemented | +|---------------------------------| --------------------------------------------------|------------|-------------| +| testInitExtended | Check constructor. | Yes | Yes | + +## ImmutableERC20MinterBurnerPermit.sol +This section defines tests for contracts/erc20/preset/ImmutableERC20MinterBurnerPermit.sol. All of the tests defined in the table below are in test/erc20/preset/ImmutableERC20MinterBurnerPermit.t.sol. +Minter, Burner and Permit features are tested in ERC20MinternBurnerPermitCommon.t.sol, described above. | Test name |Description | Happy Case | Implemented | |---------------------------------| --------------------------------------------------|------------|-------------| -| testInit | Check constructor. | Yes | Yes | -| testChangeOwner | Check change ownership. | Yes | Yes | -| testRenounceOwnershipBlocked | Ensure renounceOwnership reverts. | No | Yes | -| testOnlyMinterCanMunt | Ensure Only minter role can mint reverts. | No | Yes | -| testMint | Ensure successful minting by minter | No | Yes | -| testCanOnlyMintUpToMaxSupply | Ensure can only mint up to max supply | No | Yes | -| testRenounceLastHubOwnerBlocked | Ensure the last hub owner cannot be renounced | No | Yes | -| testRenounceLastAdminBlocked | Ensure the last default admin cannot be renounced | No | Yes | -| testRenounceAdmin | Ensure admin role can be renounced | No | Yes | -| testRenounceHubOwner | Ensure hub owner role can be renounced | No | Yes | -| testBurnFrom | Ensure allowance is required to burnFrom | Yes | Yes | -| testPermit | Ensure Permit works | Yes | Yes | +| testInitExtended | Check constructor. | Yes | Yes | +| testRenounceAdmin | Check that default admins can call renounce. | Yes | Yes | +| testRenounceLastAdminBlocked | Check that the last admin can not call renounce. | No | Yes | +| testRenounceHubOwner | Check that hub owners can call renounce. | Yes | Yes | +| testRenounceLastHubOwnerBlocked | Check that the last hub owner can not call renounce. | No | Yes | + +## ImmutableERC20MinterBurnerPermitV2.sol +This section defines tests for contracts/erc20/preset/ImmutableERC20MinterBurnerPermitV2.sol. +All of the tests defined in the table below are in test/erc20/preset/ImmutableERC20MinterBurnerPermitV2.t.sol. +Note that ImmutableERC20MinterBurnerPermitV2 extends HubOwner.sol. The ownership features reside in +HubOwner.sol, and hence are tested in HubOwner.t.sol. Minter, Burner and Permit features are +tested in ERC20MinternBurnerPermitCommon.t.sol, described above. + + +| Test name |Description | Happy Case | Implemented | +|---------------------------------| --------------------------------------------------|------------|-------------| +| testInitExtended | Check constructor. | Yes | Yes | +| testRenounceAdmin | Check that default admins can call renounce. | Yes | Yes | +| testRenounceLastAdminBlocked | Check that the last admin can not call renounce. | No | Yes | +| testRenounceHubOwner | Check that hub owners can call renounce. | Yes | Yes | +| testRenounceLastHubOwnerBlocked | Check that the last hub owner can not call renounce. | No | Yes |