Best Practices

This guide documents the suggested best practices when developing with Foundry. In general, it's recommended to handle as much as possible with forge fmt, and anything this doesn't handle is below.

General Contract Guidance

  1. Always use named import syntax, don't import full files. This restricts what is being imported to just the named items, not everything in the file. Importing full files can result in solc complaining about duplicate definitions and slither erroring, especially as repos grow and have more dependencies with overlapping names.

    • Good: import {MyContract} from "src/MyContract.sol" to only import MyContract.
    • Bad: import "src/MyContract.sol" imports everything in MyContract.sol. (Importing forge-std/Test or Script can be an exception here, so you get the console library, etc).
  2. Sort imports by forge-std/ first, then dependencies, test/, script/, and finally src/. Within each, sort alphabetically by path (not by the explicit named items being imported). (Note: This may be removed once foundry-rs/foundry#3396 is merged).

  3. Similarly, sort named imports. (Note: This may be removed once foundry-rs/foundry#3396 is resolved).

    • Good: import {bar, foo} from "src/MyContract.sol"
    • Bad: import {foo, bar} from "src/MyContract.sol"
  4. Note the tradeoffs between absolute and relative paths for imports (where absolute paths are relative to the repo root, e.g. "src/interfaces/IERC20.sol"), and choose the best approach for your project:

    • Absolute paths make it easier to see where files are from and reduces churn when moving files around.
    • Relative paths make it more likely your editor can provide features like linting and autocomplete, since editors/extensions may not understand your remappings.
  5. If copying a library from a dependency (instead of importing it), use the ignore = [] option in the config file to avoid formatting that file. This makes diffing it against the original simpler for reviewers and auditors.

  6. Similarly, feel free to use the // forgefmt: disable-* comment directives to ignore lines/sections of code that look better with manual formatting. Supported values for * are:

    • disable-line
    • disable-next-line
    • disable-next-item
    • disable-start
    • disable-end

Additional best practices from samsczun's How Do You Even Write Secure Code Anyways talk:

  • Use descriptive variable names.
  • Limit the number of active variables.
  • No redundant logic.
  • Early exit as much as possible to reduce mental load when seeing the code.
  • Related code should be placed near each other.
  • Delete unused code.

Tests

General Test Guidance

  1. For testing MyContract.sol, the test file should be MyContract.t.sol. For testing MyScript.s.sol, the test file should be MyScript.t.sol.

    • If the contract is big and you want to split it over multiple files, group them logically like MyContract.owner.t.sol, MyContract.deposits.t.sol, etc.
  2. Never make assertions in the setUp function, instead use a dedicated test like test_SetUpState() if you need to ensure your setUp function does what you expected. More info on why in foundry-rs/foundry#1291

  3. For unit tests, treat contracts as describe blocks:

    • contract Add holds all unit tests for the add method.
    • contract Supply holds all tests for the supply method.
    • contract Constructor hold all tests for the constructor.
    • One benefit of this approach is that smaller contracts should compile faster than large ones, so this approach of many small contracts should save time as test suites get large.
  4. Integration tests should live in the same test directory, with a clear naming convention. These may be in dedicated files, or they may live next to related unit tests in existing test files.

  5. Be consistent with test naming, as it's helpful for filtering tests (e.g. for gas reports you might want to filter out revert tests). When combining naming conventions, keep them alphabetical.

    • test_Description for standard tests.
    • testFuzz_Description for fuzz tests.
    • test_Revert[If|When]_Condition for tests expecting a revert.
    • testFork_Description for tests that fork from a network.
    • testForkFuzz_Revert[If|When]_Condition for a fuzz test that forks and expects a revert.
  6. When using assertions like assertEq, consider leveraging the last string param to make it easier to identify failures. These can be kept brief, or even just be numbers—they basically serve as a replacement for showing line numbers of the revert, e.g. assertEq(x, y, "1") or assertEq(x, y, "sum1"). (Note: foundry-rs/foundry#2328 tracks integrating this natively).

  7. When testing events, prefer setting all expectEmit arguments to true, i.e. vm.expectEmit(true, true, true, true). Benefits:

    • This ensures you test everything in your event.
    • If you add a topic (i.e. a new indexed parameter), it's now tested by default.
    • Even if you only have 1 topic, the extra true arguments don't hurt.
  8. Remember to write invariant tests! For the assertion string, use a verbose english description of the invariant: assertEq(x + y, z, "Invariant violated: the sum of x and y must always equal z"). More info on best practices coming soon.

Fork Tests

  1. Don't feel like you need to give forks tests special treatment, and use them liberally:

    • Mocks are required in closed-source web2 development—you have to mock API responses because the code for that API isn't open source so you cannot just run it locally. But for blockchains that's not true: any code you're interacting with that's already deployed can be locally executed and even modified for free. So why introduce the risk of a wrong mock if you don't need to?
    • A common reason to avoid fork tests and prefer mocks is that fork tests are slow. But this is not always true. By pinning to a block number, forge caches RPC responses so only the first run is slower, and subsequent runs are significantly faster. See this benchmark, where it took forge 7 minutes for the first run with a remote RPC, but only half a second once data was cached results. Alchemy and Infura both offer free archive data, so pinning to a block shouldn't be problematic. (Note that you may need to configure your CI to cache the RPC responses between runs).
  2. Be careful with with fuzz tests on a fork to avoid burning through RPC requests with non-deterministic fuzzing.

  3. When writing fork tests, do not use the --fork-url flag. Instead, prefer the following approach for it's improved flexibility:

    • Define [rpc_endpoints] in the foundry.toml config file and use the forking cheatcodes.
    • Access the RPC URL endpoint in your test with forge-std's stdChains.ChainName.rpcUrl. See the list of supported chains and expected config file aliases here.
    • Always pin to a block so tests are deterministic and RPC responses are cached.
    • More info on this fork test approach can be found here (this predates StdChains so that aspect is a bit out of date).

Test Harnesses

Internal Functions

To test internal functions, write a harness contract that inherits from the contract under test (CuT). Harness contracts that inherit from the CuT expose the internal functions as external ones.

Each internal function that is tested should be exposed via an external one with a name that follows the pattern exposed_<function_name>. For example:

// file: src/MyContract.sol
contract MyContract {
  function myInternalMethod() internal returns (uint) {
    return 42;
  }
}

// file: test/MyContract.t.sol
import {MyContract} from "src/MyContract.sol";

contract MyContractHarness is MyContract {
  // Deploy this contract then cll this method to test `myInternalMethod`.
  function exposed_myInternalMethod() external returns (uint) {
    return myInternalMethod();
  }
}

Private Functions

Unfortunately there is currently no good way to unit test private methods since they cannot be accessed by any other contracts. Options include:

  • Converting private functions to internal.
  • Copy/pasting the logic into your test contract and writing a script that runs in CI check to ensure both functions are identical.

Workaround Functions

Harnesses can also be used to expose functionality or information otherwise unavailable in the original smart contract. The most straightforward example is when we want to test the length of a public array. The functions should follow the pattern: workaround_<function_name>, such as workaround_queueLength().

Another use case for this is tracking data that you would not track in production to help test invariants. For example, you might store a list of all token holders to simplify validation of the invariant "sum of all balances must equal total supply". These are often known as "ghost variables". You can learn more about this in Rikard Hjort's Formal Methods for the Working DeFi Dev talk.

Best practices

Thanks to @samsczun's How Do You Even Write Secure Code Anyways talk for the tips in this section and the following section.

  • Don't optimize for coverage, optimize for well thought-out tests.
  • Write positive and negative unit tests.
    • Write positive unit tests for things that the code should handle. Validate all state that changes from these tests.
    • Write negative unit tests for things that the code should not handle. It's helpful to follow up (as an adjacent test) with the positive test and make the change that it needs to pass.
    • Each code path should have it's own unit test.
  • Write integration tests to test entire features.
  • Write fork tests to verify the correct behavior with existing deployed contract.

Taint Analysis

When testing, you should prioritize functions that an attacker can affect, that means functions that accept some kind of user input. These are called sources.

Consider that input data as tainted until it has been checked by the code, at which point it's considered clean.

A sink is a part of the code where some important operation is happening. For example, in MakerDAO that would be vat.sol.

You should ensure that no tainted data ever reaches a sink. That means that all data that find themselves in the sink, should, at some point, have been checked by you. So, you need to define what the data should be and then make sure your checks ensure that the data will be how you expect it to be.

Scripts

  1. Stick with run as the default function name for clarity.

  2. Any methods that are not intended to be called directly in the script should be internal or private. Generally the only public method should be run, as it's easier to read/understand when each script file just does one thing.

  3. Consider prefixing scripts with a number based on the order they're intended to be run over the protocol's lifecycle. For example, 01_Deploy.s.sol, 02_TransferOwnership.s.sol. This makes things more self-documenting. This may not always apply depending on your project.

  4. Test your scripts.

    • Unit test them by writing tests that assert on the state changes made from running the script.
    • Write your deploy script and scaffold tests by running that script. Then, run all tests against the state resulting from your production deployment script. This is a great way to gain confidence in a deploy script.
  5. Carefully audit which transactions are broadcasted. Transactions not broadcasted are still executed in the context of a test, so missing broadcasts or extra broadcasts are easy sources of error in the previous step.

  6. Watch out for frontrunning. Forge simulates your script, generates transaction data from the simulation results, then broadcasts the transactions. Make sure your script is robust against chain-state changing between the simulation and broadcast. A sample script vulnerable to this is below:

// Pseudo-code, may not compile.
contract VulnerableScript is Script {
   function run() public {
      vm.startBroadcast();

      // Transaction 1: Deploy a new Gnosis Safe with CREATE.
      // Because we're using CREATE instead of CREATE2, the address of the new
      // Safe is a function of the nonce of the gnosisSafeProxyFactory.
      address mySafe = gnosisSafeProxyFactory.createProxy(singleton, data);

      // Transaction 2: Send tokens to the new Safe.
      // We know the address of mySafe is a function of the nonce of the
      // gnosisSafeProxyFactory. If someone else deploys a Gnosis Safe between
      // the simulation and broadcast, the address of mySafe will be different,
      // and this script will send 1000 DAI to the other person's Safe. In this
      // case, we can protect ourselves from this by using CREATE2 instead of
      // CREATE, but every situation may have different solutions.
      dai.transfer(mySafe, 1000e18);

      vm.stopBroadcast();
   }
}
  1. For scripts that read from JSON input files, put the input files in script/input/<chainID>/<description>.json. Then have run(string memory input) (or take multiple string inputs if you need to read from multiple files) as the script's signature, and use the below method to read the JSON file.
function readInput(string memory input) internal returns (string memory) {
  string memory inputDir = string.concat(vm.projectRoot(), "/script/input/");
  string memory chainDir = string.concat(vm.toString(block.chainid), "/");
  string memory file = string.concat(input, ".json");
  return vm.readFile(string.concat(root, chainInputFolder, input));
}

Comments

  1. For public or external methods and variables, use NatSpec comments.

    • forge doc will parse these to autogenerate documentation.
    • Etherscan will display them in the contract UI.
  2. For simple NatSpec comments, consider just documenting params in the docstring, such as /// @notice Returns the sum of `x` and `y`., instead of using @param tags.

  3. For complex NatSpec comments, consider using a tool like PlantUML to generate ASCII art diagrams to help explain complex aspects of the codebase.

  4. Any markdown in your comments will carry over properly when generating docs with forge doc, so structure comments with markdown when useful.

    • Good: /// @notice Returns the sum of `x` and `y`.
    • Bad: /// @notice Returns the sum of x and y.

Resources

Write more secure code and better tests:

Foundry in Action:

  • Nomad Monorepo: All the contracts-* packages. Good example of using many Foundry features including fuzzing, ffi and various cheatcodes.
  • Uniswap Periphery: Good example of using inheritance to isolate test fixtures.
  • awesome-foundry: A curated list of awesome of the Foundry development framework.