fix: improve DX (#398)

* fix: dx, warnings, cleanup

* feat(cheats): etchify

* chore: forge fmt

* fix: ABIEncoderV2

* revert: `safeconsole` -> `consoleSafe`

* revert: add `etchify`

* chore: forge fmt

* docs: improve comment

* ci: add back 0.6.12 backwards compatibility check

* fix: add assumeNotBlacklisted alias

* chore: add comment explaining _viewChainId/_pureChainId

---------

Co-authored-by: Matt Solomon <[email protected]>

Author: ZeroEkkusu

.github/workflows/ci.yml

@@ -21,7 +21,9 @@ jobs:
       - name: Print forge version
         run: forge --version
 
-      # Backwards compatibility checks.
+      # Backwards compatibility checks:
+      # - the oldest and newest version of each supported minor version
+      # - versions with specific issues
       - name: Check compatibility with latest
         if: always()
         run: |
@@ -72,6 +74,10 @@ jobs:
             exit 1
           fi
 
+      - name: Check compatibility with 0.6.12
+        if: always()
+        run: forge build --skip test --use solc:0.6.12
+
       - name: Check compatibility with 0.6.2
         if: always()
         run: |

src/Script.sol

@@ -2,17 +2,18 @@
 pragma solidity >=0.6.2 <0.9.0;
 
 // 💬 ABOUT
-// Standard Library's default Script.
+// Forge Std's default Script.
 
 // 🧩 MODULES
-import {ScriptBase} from "./Base.sol";
 import {console} from "./console.sol";
 import {console2} from "./console2.sol";
+import {safeconsole} from "./safeconsole.sol";
 import {StdChains} from "./StdChains.sol";
 import {StdCheatsSafe} from "./StdCheats.sol";
 import {stdJson} from "./StdJson.sol";
 import {stdMath} from "./StdMath.sol";
 import {StdStorage, stdStorageSafe} from "./StdStorage.sol";
+import {StdStyle} from "./StdStyle.sol";
 import {StdUtils} from "./StdUtils.sol";
 import {VmSafe} from "./Vm.sol";
 

src/StdChains.sol

@@ -1,8 +1,6 @@
 // SPDX-License-Identifier: MIT
 pragma solidity >=0.6.2 <0.9.0;
 
-pragma experimental ABIEncoderV2;
-
 import {VmSafe} from "./Vm.sol";
 
 /**

src/StdCheats.sol

@@ -194,13 +194,13 @@ abstract contract StdCheatsSafe {
     }
 
     // Checks that `addr` is not blacklisted by token contracts that have a blacklist.
-    function assumeNoBlacklisted(address token, address addr) internal virtual {
+    function assumeNotBlacklisted(address token, address addr) internal view virtual {
         // Nothing to check if `token` is not a contract.
         uint256 tokenCodeSize;
         assembly {
             tokenCodeSize := extcodesize(token)
         }
-        require(tokenCodeSize > 0, "StdCheats assumeNoBlacklisted(address,address): Token address is not a contract.");
+        require(tokenCodeSize > 0, "StdCheats assumeNotBlacklisted(address,address): Token address is not a contract.");
 
         bool success;
         bytes memory returnData;
@@ -214,13 +214,16 @@ abstract contract StdCheatsSafe {
         vm.assume(!success || abi.decode(returnData, (bool)) == false);
     }
 
-    function assumeNoPrecompiles(address addr) internal virtual {
-        // Assembly required since `block.chainid` was introduced in 0.8.0.
-        uint256 chainId;
-        assembly {
-            chainId := chainid()
-        }
-        assumeNoPrecompiles(addr, chainId);
+    // Checks that `addr` is not blacklisted by token contracts that have a blacklist.
+    // This is identical to `assumeNotBlacklisted(address,address)` but with a different name, for
+    // backwards compatibility, since this name was used in the original PR which has already has
+    // a release. This function can be removed in a future release once we want a breaking change.
+    function assumeNoBlacklisted(address token, address addr) internal view virtual {
+        assumeNotBlacklisted(token, addr);
+    }
+
+    function assumeNoPrecompiles(address addr) internal pure virtual {
+        assumeNoPrecompiles(addr, _pureChainId());
     }
 
     function assumeNoPrecompiles(address addr, uint256 chainId) internal pure virtual {
@@ -515,6 +518,28 @@ abstract contract StdCheatsSafe {
         (bool success,) = payable(addr).call{value: 0}("");
         vm.assume(success);
     }
+
+    // We use this complex approach of `_viewChainId` and `_pureChainId` to ensure there are no
+    // compiler warnings when accessing chain ID in any solidity version supported by forge-std. We
+    // can't simply access the chain ID in a normal view or pure function because the solc View Pure
+    // Checker changed `chainid` from pure to view in 0.8.0.
+    function _viewChainId() private view returns (uint256 chainId) {
+        // Assembly required since `block.chainid` was introduced in 0.8.0.
+        assembly {
+            chainId := chainid()
+        }
+
+        address(this); // Silence warnings in older Solc versions.
+    }
+
+    function _pureChainId() private pure returns (uint256 chainId) {
+        function() internal view returns (uint256) fnIn = _viewChainId;
+        function() internal pure returns (uint256) pureChainId;
+        assembly {
+            pureChainId := fnIn
+        }
+        chainId = pureChainId();
+    }
 }
 
 // Wrappers around cheatcodes to avoid footguns

src/StdInvariant.sol

@@ -3,7 +3,7 @@ pragma solidity >=0.6.2 <0.9.0;
 
 pragma experimental ABIEncoderV2;
 
-contract StdInvariant {
+abstract contract StdInvariant {
     struct FuzzSelector {
         address addr;
         bytes4[] selectors;

src/StdStyle.sol

@@ -1,10 +1,10 @@
 // SPDX-License-Identifier: MIT
 pragma solidity >=0.4.22 <0.9.0;
 
-import {Vm} from "./Vm.sol";
+import {VmSafe} from "./Vm.sol";
 
 library StdStyle {
-    Vm private constant vm = Vm(address(uint160(uint256(keccak256("hevm cheat code")))));
+    VmSafe private constant vm = VmSafe(address(uint160(uint256(keccak256("hevm cheat code")))));
 
     string constant RED = "\u001b[91m";
     string constant GREEN = "\u001b[92m";

src/StdUtils.sol

@@ -85,7 +85,7 @@ abstract contract StdUtils {
         console2_log("Bound result", vm.toString(result));
     }
 
-    function boundPrivateKey(uint256 privateKey) internal view virtual returns (uint256 result) {
+    function boundPrivateKey(uint256 privateKey) internal pure virtual returns (uint256 result) {
         result = _bound(privateKey, 1, SECP256K1_ORDER - 1);
     }
 

src/Test.sol

@@ -4,11 +4,12 @@ pragma solidity >=0.6.2 <0.9.0;
 pragma experimental ABIEncoderV2;
 
 // 💬 ABOUT
-// Standard Library's default Test
+// Forge Std's default Test.
 
 // 🧩 MODULES
 import {console} from "./console.sol";
 import {console2} from "./console2.sol";
+import {safeconsole} from "./safeconsole.sol";
 import {StdAssertions} from "./StdAssertions.sol";
 import {StdChains} from "./StdChains.sol";
 import {StdCheats} from "./StdCheats.sol";
@@ -17,15 +18,16 @@ import {StdInvariant} from "./StdInvariant.sol";
 import {stdJson} from "./StdJson.sol";
 import {stdMath} from "./StdMath.sol";
 import {StdStorage, stdStorage} from "./StdStorage.sol";
+import {StdStyle} from "./StdStyle.sol";
 import {StdUtils} from "./StdUtils.sol";
 import {Vm} from "./Vm.sol";
-import {StdStyle} from "./StdStyle.sol";
 
 // 📦 BOILERPLATE
 import {TestBase} from "./Base.sol";
 import {DSTest} from "ds-test/test.sol";
 
 // ⭐️ TEST
+// Note: DSTest and any contracts that inherit it must be inherited first, https://github.com/foundry-rs/forge-std/pull/241
 abstract contract Test is DSTest, StdAssertions, StdChains, StdCheats, StdInvariant, StdUtils, TestBase {
 // Note: IS_TEST() must return true.
 // Note: Must have failure system, https://github.com/dapphub/ds-test/blob/cd98eff28324bfac652e63a239a60632a761790b/src/test.sol#L39-L76.

src/Vm.sol

@@ -116,7 +116,7 @@ interface VmSafe {
     // Labels an address in call traces
     function label(address account, string calldata newLabel) external;
     // Gets the label for the specified address
-    function getLabel(address account) external returns (string memory label);
+    function getLabel(address account) external returns (string memory currentLabel);
     // Using the address that calls the test contract, has the next call (at this call depth only) create a transaction that can later be signed and sent onchain
     function broadcast() external;
     // Has the next call (at this call depth only) create a transaction with the address provided as the sender that can later be signed and sent onchain

test/StdCheats.t.sol

@@ -370,8 +370,8 @@ contract StdCheatsTest is Test {
 
 contract StdCheatsMock is StdCheats {
     // We deploy a mock version so we can properly test expected reverts.
-    function assumeNoBlacklisted_(address token, address addr) external {
-        return assumeNoBlacklisted(token, addr);
+    function assumeNotBlacklisted_(address token, address addr) external view {
+        return assumeNotBlacklisted(token, addr);
     }
 }
 
@@ -386,38 +386,38 @@ contract StdCheatsForkTest is Test {
     StdCheatsMock private stdCheats = new StdCheatsMock();
 
     function setUp() public {
-        // All tests of the `assumeNoBlacklisted` method are fork tests using live contracts.
+        // All tests of the `assumeNotBlacklisted` method are fork tests using live contracts.
         vm.createSelectFork({urlOrAlias: "mainnet", blockNumber: 16_428_900});
     }
 
-    function testCannotAssumeNoBlacklisted_EOA() external {
+    function testCannotAssumeNotBlacklisted_EOA() external {
         address eoa = vm.addr({privateKey: 1});
-        vm.expectRevert("StdCheats assumeNoBlacklisted(address,address): Token address is not a contract.");
-        assumeNoBlacklisted(eoa, address(0));
+        vm.expectRevert("StdCheats assumeNotBlacklisted(address,address): Token address is not a contract.");
+        assumeNotBlacklisted(eoa, address(0));
     }
 
-    function testAssumeNoBlacklisted_TokenWithoutBlacklist(address addr) external {
-        assumeNoBlacklisted(SHIB, addr);
+    function testAssumeNotBlacklisted_TokenWithoutBlacklist(address addr) external {
+        assumeNotBlacklisted(SHIB, addr);
         assertTrue(true);
     }
 
-    function testAssumeNoBlacklisted_USDC() external {
+    function testAssumeNotBlacklisted_USDC() external {
         vm.expectRevert();
-        stdCheats.assumeNoBlacklisted_(USDC, USDC_BLACKLISTED_USER);
+        stdCheats.assumeNotBlacklisted_(USDC, USDC_BLACKLISTED_USER);
     }
 
-    function testAssumeNoBlacklisted_USDC(address addr) external {
-        assumeNoBlacklisted(USDC, addr);
+    function testAssumeNotBlacklisted_USDC(address addr) external {
+        assumeNotBlacklisted(USDC, addr);
         assertFalse(USDCLike(USDC).isBlacklisted(addr));
     }
 
-    function testAssumeNoBlacklisted_USDT() external {
+    function testAssumeNotBlacklisted_USDT() external {
         vm.expectRevert();
-        stdCheats.assumeNoBlacklisted_(USDT, USDT_BLACKLISTED_USER);
+        stdCheats.assumeNotBlacklisted_(USDT, USDT_BLACKLISTED_USER);
     }
 
-    function testAssumeNoBlacklisted_USDT(address addr) external {
-        assumeNoBlacklisted(USDT, addr);
+    function testAssumeNotBlacklisted_USDT(address addr) external {
+        assumeNotBlacklisted(USDT, addr);
         assertFalse(USDTLike(USDT).isBlackListed(addr));
     }
 }