Skip to content

Commit

Permalink
fix: improve DX (#398)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
ZeroEkkusu and mds1 committed Jun 25, 2023
1 parent 7b0c65a commit 27035c8
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 37 deletions.
8 changes: 7 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down Expand Up @@ -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: |
Expand Down
5 changes: 3 additions & 2 deletions src/Script.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down
2 changes: 0 additions & 2 deletions src/StdChains.sol
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.6.2 <0.9.0;

pragma experimental ABIEncoderV2;

import {VmSafe} from "./Vm.sol";

/**
Expand Down
43 changes: 34 additions & 9 deletions src/StdCheats.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion src/StdInvariant.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions src/StdStyle.sol
Original file line number Diff line number Diff line change
@@ -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";
Expand Down
2 changes: 1 addition & 1 deletion src/StdUtils.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
6 changes: 4 additions & 2 deletions src/Test.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/Vm.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 16 additions & 16 deletions test/StdCheats.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand All @@ -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));
}
}
Expand Down

0 comments on commit 27035c8

Please sign in to comment.