diff --git a/.forge-snapshots/Increment counter number.snap b/.forge-snapshots/Increment counter number.snap deleted file mode 100644 index 5f8239b..0000000 --- a/.forge-snapshots/Increment counter number.snap +++ /dev/null @@ -1 +0,0 @@ -26263 \ No newline at end of file diff --git a/.forge-snapshots/Set counter number.snap b/.forge-snapshots/Set counter number.snap deleted file mode 100644 index 1a947f3..0000000 --- a/.forge-snapshots/Set counter number.snap +++ /dev/null @@ -1 +0,0 @@ -26337 \ No newline at end of file diff --git a/.gitmodules b/.gitmodules index 7dc401b..9183682 100644 --- a/.gitmodules +++ b/.gitmodules @@ -4,6 +4,6 @@ [submodule "lib/forge-chronicles"] path = lib/forge-chronicles url = https://github.com/0xPolygon/forge-chronicles -[submodule "lib/forge-gas-snapshot"] - path = lib/forge-gas-snapshot - url = https://github.com/marktoda/forge-gas-snapshot +[submodule "lib/briefcase"] + path = lib/briefcase + url = https://github.com/Uniswap/briefcase diff --git a/.prettierignore b/.prettierignore index 45f4529..f646773 100644 --- a/.prettierignore +++ b/.prettierignore @@ -5,3 +5,4 @@ cache/ docs/autogenerated *.sol deployments/ +snapshots/ \ No newline at end of file diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 95b5244..99f19cb 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,23 +1,28 @@ # Contributing +For the latest version of this document, see [here](https://github.com/Uniswap/foundry-template/blob/main/CONTRIBUTING.md). + - [Install](#install) - [Pre-commit Hooks](#pre-commit-hooks) - [Requirements for merge](#requirements-for-merge) - [Branching](#branching) - [Main](#main) - - [Staging](#staging) - [Dev](#dev) - [Feature](#feature) - - [Fix](#fix) + - [Audit](#audit) - [Code Practices](#code-practices) - [Code Style](#code-style) + - [Solidity Versioning](#solidity-versioning) - [Interfaces](#interfaces) - [NatSpec \& Comments](#natspec--comments) - [Testing](#testing) + - [Best Practices](#best-practices) + - [IR Compilation](#ir-compilation) - [Gas Metering](#gas-metering) - [Deployment](#deployment) - - [Deployment](#deployment-1) - - [Deployment Info Generation](#deployment-info-generation) + - [Bytecode Hash](#bytecode-hash) + - [Monorepo](#monorepo) +- [Dependency Management](#dependency-management) - [Releases](#releases) ## Install @@ -56,23 +61,19 @@ This section outlines the branching strategy of this repo. ### Main -The main branch is supposed to reflect the deployed state on all networks. Any pull requests into this branch MUST come from the staging branch. - -### Staging - -The staging branch reflects new code complete deployments or upgrades containing fixes and/or features. Any pull requests into this branch MUST come from the dev branch. The staging branch is used for security audits and deployments. Once the deployment is complete and verified as well as deployment log files are generated, the branch can be merged into main. For more information on the deployment and log file generation check [here](#deployment). +The main branch is supposed to reflect the deployed state on all networks. Only audited code should be merged into main. Squashed commits from dev or feature branches should be merged into the main branch using a regular merge strategy. ### Dev -This is the active development branch. All pull requests into this branch MUST come from fix or feature branches. Upon code completion this branch is merged into staging for auditing and deployment. PRs into this branch should squash all commits into a single commit. +This is the active development branch. Upon code completion this branch is frozen on an audit branch. PRs into this branch should squash all commits into a single commit. In case of multiple parallel development efforts, each project should have its own dev branch with the naming convention `dev/` to ensure work in progress is isolated and does not end up on the main branch. ### Feature -Any new feature should be developed on a separate branch. The naming convention for these branches is `feat/*`. Once the feature is complete, a pull request into the dev branch can be created. +Feature branches should be owned by one responsible developer. The dev branch should be the target of the feature branch. In pre-audit and pre-deployment repositories feature branches may be merged into the main branch directly. Generally, feature branches should be squashed into a single commit before merging. -### Fix +### Audit -Any bug fixes should be developed on a separate branch. The naming convention for these branches is `fix/*`. Once the fix is complete, a pull request into the dev branch can be created. +Before an audit, the code should be frozen on a branch dedicated to the audit with the naming convention `audit/`. Each fix in response to an audit finding should be developed on its own branch. The naming convention for these branches is `audit//`. The PR title should include the provider and the issue number at minimum. Sometimes it is desirable to have all of the fixes in a single PR for review. In this case, each fix PR should be included via the `merge` strategy, and the combined PR can be merged into the dev branch via `merge` to preserve the order of the fix commits. ## Code Practices @@ -91,6 +92,18 @@ The repo follows the official [Solidity Style Guide](https://docs.soliditylang.o } ``` +- Naming collisions should be avoided using a single trailing underscore. + + ```solidity + contract TestContract { + uint256 public foo; + + constructor(uint256 foo_) { + foo = foo_; + } + } + ``` + - Events should generally be emitted immediately after the state change that they represent, and should be named in the past tense. Some exceptions may be made for gas efficiency if the result doesn't affect observable ordering of events. @@ -117,13 +130,29 @@ The repo follows the official [Solidity Style Guide](https://docs.soliditylang.o - Unchecked arithmetic blocks should contain comments explaining why overflow is guaranteed not to happen or permissible. If the reason is immediately apparent from the line above the unchecked block, the comment may be omitted. +### Solidity Versioning + +Contracts that are meant to be deployed MUST have an explicit version set in the `pragma` statement. + +```solidity +pragma solidity 0.8.X; +``` + +Abstract contracts, libraries and interfaces MUST use the caret (`^`) range operator to specify the version range to ensure better compatibility. + +```solidity +pragma solidity ^0.X.0; +``` + +Libraries and abstract contracts using functionality introduced in newer versions of Solidity can use caret range operators with higher path versions (e.g., `^0.8.24` when using transient storage opcodes). For interfaces, it should be considered to use the greater than or equal to (`>=`) range operator to ensure better compatibility with future versions of Solidity. + ### Interfaces Every contract MUST implement their corresponding interface that includes all externally callable functions, errors and events. ### NatSpec & Comments -Interfaces should be the entrypoint for all contracts. When exploring the a contract within the repository, the interface MUST contain all relevant information to understand the functionality of the contract in the form of NatSpec comments. This includes all externally callable functions, errors and events. The NatSpec documentation MUST be added to the functions, errors and events within the interface. This allows a reader to understand the functionality of a function before moving on to the implementation. The implementing functions MUST point to the NatSpec documentation in the interface using `@inheritdoc`. Internal and private functions shouldn't have NatSpec documentation except for `@dev` comments, whenever more context is needed. Additional comments within a function should only be used to give more context to more complex operations, otherwise the code should be kept readable and self-explanatory. +Interfaces should be the entrypoint for all contracts. When exploring the a contract within the repository, the interface MUST contain all relevant information to understand the functionality of the contract in the form of NatSpec comments. This includes all externally callable functions, errors and events. The NatSpec documentation MUST be added to the functions, errors and events within the interface. This allows a reader to understand the functionality of a function before moving on to the implementation. The implementing functions MUST point to the NatSpec documentation in the interface using `@inheritdoc`. Internal and private functions shouldn't have NatSpec documentation except for `@dev` comments, whenever more context is needed. Additional comments within a function should only be used to give more context to more complex operations, otherwise the code should be kept readable and self-explanatory. Single line NatSpec comments should use a triple slash (`///`) to ensure compact documentation. ## Testing @@ -133,26 +162,59 @@ Differential testing should be used to compare assembly implementations with imp New features must be merged with associated tests. Bug fixes should have a corresponding test that fails without the bug fix. -### Gas Metering +### Best Practices -The [Forge Gas Snapshot](https://github.com/marktoda/forge-gas-snapshot) library is used to measure the gas cost of individual actions. To ensure that the measured gas is accurate, tests have to be run using the isolate argument to generate the correct snapshot and ensure that CI passes: +Best practices and naming conventions should be followed as outlined in the [Foundry Book](https://getfoundry.sh/forge/tests/overview). -```sh -$ forge test --isolate -``` +### IR Compilation -When adding new functionality, a new gas snapshot should be added, preferably using `snapLastCall`. +When the contracts are compiled via IR, tests should be compiled without IR and the contracts deployed from their bytecode to ensure quick compilation times and bytecode consistency. Check [here](https://github.com/Uniswap/foundry-template/blob/613f81c107cd2885a869dbe4afc1da4f96ed9218/foundry.toml#L22-L28) and [here](https://github.com/Uniswap/foundry-template/blob/main/test/deployers/CounterDeployer.sol) for an example. + +### Gas Metering + +Gas for function calls should be metered using the built in `vm.snapshotGasLastCall` function in forge. To meter across multiple calls `vm.startSnapshotGas` and `vm.stopSnapshotGas` can be used. Tests that measure gas should be annotated with `/// forge-config: default.isolate = true` and not be fuzzed to ensure that the gas snapshot is accurate and consistent for CI verification. All external functions should have a gas snapshot test, diverging paths within a function should have appropriate gas snapshot tests. +For more information on gas metering see the [Forge cheatcodes reference](https://getfoundry.sh/reference/cheatcodes/gas-snapshots/#snapshotgas-cheatcodes). ## Deployment -After deployments are executed a script is provided that extracts deployment information from the `run-latest.json` file within the `broadcast` directory generated while the forge script runs. From this information a JSON and markdown file is generated using the [Forge Chronicles](https://github.com/0xPolygon/forge-chronicles) library containing various information about the deployment itself as well as past deployments. +After deployments are executed a script is provided that extracts deployment information from the `run-latest.json` file within the `broadcast` directory generated while the forge script runs. From this information a JSON and markdown file is generated using the [Forge Chronicles](https://github.com/uniswap/forge-chronicles) library containing various information about the deployment itself as well as past deployments. + +### Bytecode Hash -### Deployment +Bytecode hash MUST be set to `none` in the `foundry.toml` file to ensure that the bytecode is consistent. -To deploy the contracts, provide the `--broadcast` flag to the forge script command. Should the etherscan verification time out, it can be picked up again by replacing the `--broadcast` flag with `--resume`. -Deploy the contracts to one of the predefined networks by providing the according key with the `--rpc-url` flag. Most of the predefined networks require the `INFURA_KEY` environment variable to be set in the `.env` file. -Including the `--verify` flag will verify deployed contracts on Etherscan. Define the appropriate environment variable for the Etherscan api key in the `.env` file. +### Monorepo -```shell -forge script script/Deploy.s.sol --broadcast --rpc-url --verify +Contracts should be integrated into the [Smart Contract Monorepo](https://github.com/uniswap/contracts) to be deployed via the deployer cli tool. + +## Dependency Management + +The preferred way to manage dependencies is using [`forge install`](https://book.getfoundry.sh/forge/dependencies). This ensures that your project uses the correct versions and structure for all external libraries. + +However, in cases where there is a Solidity version mismatch between your project and a dependency, it may be required to include the compiled bytecode directly in a utility contract. This approach allows to deploy the dependency using the correct bytecode, regardless of source compatibility. This may be especially helpful for integration testing. + +First, it should be checked if the deployer is already part of [Briefcase](https://github.com/Uniswap/briefcase/tree/main/src/deployers). If so, import the deployer from here. + +Otherwise, a custom deploy contract should be created. Below is an example of how to deploy a contract using hardcoded bytecode and the `CREATE2` opcode. + +```solidity +// SPDX-License-Identifier: MIT +pragma solidity 0.8.24; +contract BytecodeDeployer { + /// @dev Deploys a contract using CREATE2 with the provided bytecode and salt. + /// @param bytecode The contract bytecode to deploy. + /// @param salt The salt to use for CREATE2. + /// @return addr The address of the deployed contract. + function deploy(bytes memory bytecode, bytes32 salt) public returns (address addr) { + require(bytecode.length != 0, "Bytecode is empty"); + assembly { + addr := create2(bytecode, salt) + if iszero(extcodesize(addr)) { revert(0, 0) } + } + } +} ``` + +## Releases + +Every deployment and changes made to contracts after deployment should be accompanied by a tag and release on GitHub. diff --git a/docs/autogen/book.toml b/docs/autogen/book.toml index 9ef6709..6654d46 100644 --- a/docs/autogen/book.toml +++ b/docs/autogen/book.toml @@ -6,6 +6,7 @@ title = "" no-section-label = true additional-js = ["solidity.min.js"] additional-css = ["book.css"] +mathjax-support = true git-repository-url = "https://github.com/Uniswap/foundry-template" [output.html.fold] diff --git a/foundry.toml b/foundry.toml index 391d308..43c57a1 100644 --- a/foundry.toml +++ b/foundry.toml @@ -6,6 +6,7 @@ optimizer = true optimizer_runs = 999999 via_ir = true solc = "0.8.26" +bytecode_hash = "none" verbosity = 2 ffi = true fs_permissions = [ diff --git a/lib/briefcase b/lib/briefcase new file mode 160000 index 0000000..2b00819 --- /dev/null +++ b/lib/briefcase @@ -0,0 +1 @@ +Subproject commit 2b00819e5694c075a5437a9a0cde109fac0e6578 diff --git a/lib/forge-chronicles b/lib/forge-chronicles index c2be7a4..d1fb566 160000 --- a/lib/forge-chronicles +++ b/lib/forge-chronicles @@ -1 +1 @@ -Subproject commit c2be7a462d87b8e3a72f7e2b739cdf12807a71e4 +Subproject commit d1fb566915f23a01f08747264c56f8925f15751a diff --git a/lib/forge-gas-snapshot b/lib/forge-gas-snapshot deleted file mode 160000 index 9161f7c..0000000 --- a/lib/forge-gas-snapshot +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 9161f7c0b6c6788a89081e2b3b9c67592b71e689 diff --git a/lib/forge-std b/lib/forge-std index 5475f85..60acb7a 160000 --- a/lib/forge-std +++ b/lib/forge-std @@ -1 +1 @@ -Subproject commit 5475f852e3f530d7e25dfb4596aa1f9baa8ffdfc +Subproject commit 60acb7aaadcce2d68e52986a0a66fe79f07d138f diff --git a/snapshots/CounterTest_Deployed.json b/snapshots/CounterTest_Deployed.json new file mode 100644 index 0000000..c88dfa7 --- /dev/null +++ b/snapshots/CounterTest_Deployed.json @@ -0,0 +1,4 @@ +{ + "Increment counter number": "26263", + "Set counter number": "26337" +} \ No newline at end of file diff --git a/test/Counter.t.sol b/test/Counter.t.sol index e23ed0d..cc06c84 100644 --- a/test/Counter.t.sol +++ b/test/Counter.t.sol @@ -1,7 +1,6 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity 0.8.26; -import {GasSnapshot} from 'forge-gas-snapshot/GasSnapshot.sol'; import 'forge-std/Test.sol'; import {CounterDeployer, ICounter} from './deployers/CounterDeployer.sol'; @@ -16,14 +15,15 @@ abstract contract Deployed is CounterDeployer { } } -contract CounterTest_Deployed is Deployed, GasSnapshot { +contract CounterTest_Deployed is Deployed { function test_IsInitialized() public view { assertEq(counter.number(), 10); } + /// forge-config: default.isolate = true function test_IncrementsNumber() public { counter.increment(); - snapLastCall('Increment counter number'); + vm.snapshotGasLastCall('Increment counter number'); assertEq(counter.number(), 11); } @@ -32,10 +32,11 @@ contract CounterTest_Deployed is Deployed, GasSnapshot { assertEq(counter.number(), x); } + /// forge-config: default.isolate = true function test_SetNumber_gas() public { uint256 x = 100; counter.setNumber(x); - snapLastCall('Set counter number'); + vm.snapshotGasLastCall('Set counter number'); } }