chore: BalCoW v1.3.0 release

.forge-snapshots/exitPool.snap

@@ -0,0 +1 @@
+174743

.forge-snapshots/joinPool.snap

@@ -0,0 +1 @@
+138985

.forge-snapshots/newBFactory.snap

@@ -1 +1 @@
-3528278
+4130621

.forge-snapshots/newBPool.snap

@@ -1 +1 @@
-3315258
+3477592

.forge-snapshots/settlementCoWSwap.snap

@@ -1 +1 @@
-186763
+215793

.forge-snapshots/settlementCoWSwapInverse.snap

@@ -1 +1 @@
-196603
+225641

.forge-snapshots/swapExactAmountIn.snap

@@ -1 +1 @@
-81507
+104914

.forge-snapshots/swapExactAmountInInverse.snap

@@ -1 +1 @@
-91166
+114583

.github/workflows/ci.yml

@@ -64,14 +64,15 @@ jobs:
       - name: Run tests
         run: yarn test:integration
 
+      - name: Ensure gas snapshots were updated
+        run: git diff --exit-code -- .forge-snapshots/
+
   lint:
     name: Static Analysis
     runs-on: ubuntu-latest
 
     steps:
       - uses: actions/checkout@v3
-        with:
-          fetch-depth: 0
 
       - uses: wagoid/commitlint-github-action@v5
 
@@ -86,11 +87,22 @@ jobs:
           node-version: 18.x
           cache: 'yarn'
 
+      - name: Install bulloak 
+        uses: baptiste0928/cargo-install@v3
+        with:
+          crate: bulloak
+
       - name: Install dependencies
         run: yarn --frozen-lockfile --network-concurrency 1
 
       - name: Run forge-fmt && solhint
         run: yarn lint:check
 
+      - name: Run bulloak check
+        run: yarn lint:bulloak
+
+      - name: Ensure auto-generated smocks are up to date
+        run: yarn smock && git diff --exit-code -- test/smock
+
       - name: Run natspec-smells
         run: yarn lint:natspec 2>&1 >/dev/null | grep 'No issues found'

.solhint.json

@@ -4,9 +4,10 @@
     "avoid-low-level-calls": "off",
     "constructor-syntax": "warn",
     "func-visibility": ["warn", { "ignoreConstructors": true }],
+    "no-empty-blocks": "error",
     "no-inline-assembly": "off",
     "ordering": "warn",
-    "private-vars-leading-underscore": ["warn", { "strict": false }],
+    "private-vars-leading-underscore": ["warn", { "strict": true }],
     "quotes": "off",
     "one-contract-per-file": "warn",
     "style-guide-casing": ["warn", { "ignoreEvents": true } ]

README.md

@@ -15,7 +15,7 @@ yarn build   # build artifacts to `out/`
 yarn test    # run the tests
 ```
 
-## Changes on BPool from (Balancer V1)[https://github.com/balancer/balancer-core]
+## Changes on BPool from [Balancer V1](https://github.com/balancer/balancer-core)
 - Migrated to Foundry project structure
 - Implementation of interfaces with Natspec documentation
 - Replaced `require(cond, 'STRING')` for `if(!cond) revert CustomError()`
@@ -37,18 +37,20 @@ yarn test    # run the tests
 - Immutably stores CoW Protocol's `SolutionSettler` and `VaultRelayer` addresses at deployment
 - Immutably stores Cow Protocol's a Domain Separator at deployment (to avoid replay attacks)
 - Immutably stores Cow Protocol's `GPv2Order.appData` to be allowed to swap
-- Gives infinite ERC20 approval to the CoW Protocol's `VaultRelayer` contract
+- Gives infinite ERC20 approval to the CoW Protocol's `VaultRelayer` contract at finalization time.
 - Implements IERC1271 `isValidSignature` method to allow for validating intentions of swaps
 - Implements a `commit` method to avoid multiple swaps from conflicting with each other.
   - This is stored in the same transient storage slot as reentrancy locks in order to prevent calls to swap/join functions within a settlement execution or vice versa.
   - It's an error to override a commitment since that could be used to clear reentrancy locks. Commitments can only be cleared by ending a transaction.
 - Validates the `GPv2Order` requirements before allowing the swap
 
 ## Features on BCoWFactory
-- Added a `logBCoWPool` to log the finalization of BCoWPool contracts, to be called by a child pool
+- Added a `logBCoWPool` to log the finalization of BCoWPool contracts, to be called by a child pool.
 
 ## Creating a Pool
-- Create a new pool by calling `IBFactory.newBPool()`
+- Create a new pool by calling the corresponding pool factory:
+  - `IBFactory.newBPool()` for regular Balancer `BPool`s
+  - `IBCoWFactory.newBPool()` for Balancer `BCoWPool`s, compatible with CoW Protocol
 - Give ERC20 allowance to the pool by calling `IERC20.approve(pool, amount)`
 - Bind tokens one by one by calling `IBPool.bind(token, amount, weight)`
   - The amount represents the initial balance of the token in the pool (pulled from the caller's balance)

foundry.toml

@@ -11,10 +11,14 @@ sort_imports = true
 [profile.default]
 solc_version = '0.8.25'
 libs = ["node_modules", "lib"]
-optimizer_runs = 50             # TODO: increase for production and add via-ir
-ffi = true
+optimizer_runs = 500
 evm_version = 'cancun'
 fs_permissions = [{ access = "read-write", path = ".forge-snapshots/"}]
+# 2018: function can be view, so far only caused by mocks
+# 2394: solc insists on reporting on every transient storage use
+# 5574, 3860: bytecode size limit, so far only caused by test contracts
+ignored_error_codes = [2018, 2394, 5574, 3860]
+deny_warnings = true
 
 [profile.optimized]
 via_ir = true

package.json

@@ -15,20 +15,22 @@
   "scripts": {
     "build": "forge build",
     "build:optimized": "FOUNDRY_PROFILE=optimized forge build",
-    "coverage": "forge coverage --match-contract Unit",
+    "coverage": "forge coverage --match-path 'test/unit/**'",
     "deploy:bcowfactory:mainnet": "bash -c 'source .env && forge script DeployBCoWFactory -vvvvv --rpc-url $MAINNET_RPC --broadcast --chain mainnet --private-key $MAINNET_DEPLOYER_PK --verify --etherscan-api-key $ETHERSCAN_API_KEY'",
     "deploy:bcowfactory:testnet": "bash -c 'source .env && forge script DeployBCoWFactory -vvvvv --rpc-url $SEPOLIA_RPC --broadcast --chain sepolia --private-key $SEPOLIA_DEPLOYER_PK --verify --etherscan-api-key $ETHERSCAN_API_KEY'",
     "deploy:bfactory:mainnet": "bash -c 'source .env && forge script DeployBFactory -vvvvv --rpc-url $MAINNET_RPC --broadcast --chain mainnet --private-key $MAINNET_DEPLOYER_PK --verify --etherscan-api-key $ETHERSCAN_API_KEY'",
     "deploy:bfactory:testnet": "bash -c 'source .env && forge script DeployBFactory -vvvvv --rpc-url $SEPOLIA_RPC --broadcast --chain sepolia --private-key $SEPOLIA_DEPLOYER_PK --verify --etherscan-api-key $ETHERSCAN_API_KEY'",
+    "lint:bulloak": "find test/unit -name '*.tree' | xargs bulloak check",
     "lint:check": "solhint 'src/**/*.sol' 'test/**/*.sol' 'script/**/*.sol' && forge fmt --check",
     "lint:fix": "solhint --fix 'src/**/*.sol' 'test/**/*.sol' 'script/**/*.sol' && sort-package-json && forge fmt",
     "lint:natspec": "npx @defi-wonderland/natspec-smells --config natspec-smells.config.js",
     "prepare": "husky install",
     "smock": "smock-foundry --contracts src/contracts",
-    "test": "forge test -vvv",
-    "test:integration": "forge test --match-contract Integration -vvv --isolate",
+    "test": "yarn test:integration && yarn test:unit",
+    "test:integration": "forge test --ffi --match-path 'test/integration/**' -vvv --isolate",
     "test:local": "FOUNDRY_FUZZ_RUNS=100 forge test -vvv",
-    "test:unit": "forge test --match-contract Unit -vvv",
+    "test:scaffold": "bulloak check --fix test/unit/*.tree && forge fmt",
+    "test:unit": "forge test --match-path 'test/unit/**' -vvv",
     "test:unit:deep": "FOUNDRY_FUZZ_RUNS=5000 yarn test:unit"
   },
   "lint-staged": {

script/DeployBCoWFactory.s.sol

@@ -7,11 +7,11 @@ import {Params} from 'script/Params.s.sol';
 
 contract DeployBCoWFactory is Script, Params {
   function run() public {
-    BCoWFactoryDeploymentParams memory _params = _bCoWFactoryDeploymentParams[block.chainid];
+    BCoWFactoryDeploymentParams memory params = _bCoWFactoryDeploymentParams[block.chainid];
 
     vm.startBroadcast();
-    BCoWFactory bCoWFactory = new BCoWFactory(_params.settlement, _params.appData);
-    bCoWFactory.setBLabs(_params.bLabs);
+    BCoWFactory bCoWFactory = new BCoWFactory(params.settlement, params.appData);
+    bCoWFactory.setBDao(params.bDao);
     vm.stopBroadcast();
   }
 }

script/DeployBFactory.s.sol

@@ -7,11 +7,11 @@ import {Params} from 'script/Params.s.sol';
 
 contract DeployBFactory is Script, Params {
   function run() public {
-    BFactoryDeploymentParams memory _params = _bFactoryDeploymentParams[block.chainid];
+    BFactoryDeploymentParams memory params = _bFactoryDeploymentParams[block.chainid];
 
     vm.startBroadcast();
     BFactory bFactory = new BFactory();
-    bFactory.setBLabs(_params.bLabs);
+    bFactory.setBDao(params.bDao);
     vm.stopBroadcast();
   }
 }

script/Params.s.sol

@@ -3,11 +3,11 @@ pragma solidity 0.8.25;
 
 contract Params {
   struct BFactoryDeploymentParams {
-    address bLabs;
+    address bDao;
   }
 
   struct BCoWFactoryDeploymentParams {
-    address bLabs;
+    address bDao;
     address settlement;
     bytes32 appData;
   }

src/contracts/BCoWConst.sol

@@ -6,16 +6,10 @@ pragma solidity 0.8.25;
  * @notice Constants used in the scope of the BCoWPool contract.
  */
 contract BCoWConst {
-  /**
-   * @notice The value representing the absence of a commitment.
-   * @return _emptyCommitment The commitment value representing no commitment.
-   */
-  bytes32 public constant EMPTY_COMMITMENT = bytes32(0);
-
   /**
    * @notice The largest possible duration of any AMM order, starting from the
    * current block timestamp.
    * @return _maxOrderDuration The maximum order duration.
    */
-  uint32 public constant MAX_ORDER_DURATION = 5 * 60;
+  uint32 public constant MAX_ORDER_DURATION = 5 minutes;
 }

src/contracts/BCoWFactory.sol

@@ -4,7 +4,6 @@ pragma solidity 0.8.25;
 import {BCoWPool} from './BCoWPool.sol';
 import {BFactory} from './BFactory.sol';
 import {IBCoWFactory} from 'interfaces/IBCoWFactory.sol';
-import {IBFactory} from 'interfaces/IBFactory.sol';
 import {IBPool} from 'interfaces/IBPool.sol';
 
 /**
@@ -19,9 +18,9 @@ contract BCoWFactory is BFactory, IBCoWFactory {
   /// @inheritdoc IBCoWFactory
   bytes32 public immutable APP_DATA;
 
-  constructor(address _solutionSettler, bytes32 _appData) BFactory() {
-    SOLUTION_SETTLER = _solutionSettler;
-    APP_DATA = _appData;
+  constructor(address solutionSettler, bytes32 appData) BFactory() {
+    SOLUTION_SETTLER = solutionSettler;
+    APP_DATA = appData;
   }
 
   /// @inheritdoc IBCoWFactory
@@ -32,9 +31,9 @@ contract BCoWFactory is BFactory, IBCoWFactory {
 
   /**
    * @dev Deploys a BCoWPool instead of a regular BPool.
-   * @return _pool The deployed BCoWPool
+   * @return bCoWPool The deployed BCoWPool
    */
-  function _newBPool() internal virtual override returns (IBPool _pool) {
-    return new BCoWPool(SOLUTION_SETTLER, APP_DATA);
+  function _newBPool() internal virtual override returns (IBPool bCoWPool) {
+    bCoWPool = new BCoWPool(SOLUTION_SETTLER, APP_DATA);
   }
 }

src/contracts/BCoWPool.sol

@@ -46,54 +46,46 @@ contract BCoWPool is IERC1271, IBCoWPool, BPool, BCoWConst {
   /// @inheritdoc IBCoWPool
   bytes32 public immutable APP_DATA;
 
-  constructor(address _cowSolutionSettler, bytes32 _appData) BPool() {
-    SOLUTION_SETTLER = ISettlement(_cowSolutionSettler);
-    SOLUTION_SETTLER_DOMAIN_SEPARATOR = ISettlement(_cowSolutionSettler).domainSeparator();
-    VAULT_RELAYER = ISettlement(_cowSolutionSettler).vaultRelayer();
-    APP_DATA = _appData;
+  constructor(address cowSolutionSettler, bytes32 appData) BPool() {
+    SOLUTION_SETTLER = ISettlement(cowSolutionSettler);
+    SOLUTION_SETTLER_DOMAIN_SEPARATOR = ISettlement(cowSolutionSettler).domainSeparator();
+    VAULT_RELAYER = ISettlement(cowSolutionSettler).vaultRelayer();
+    APP_DATA = appData;
   }
 
   /// @inheritdoc IBCoWPool
-  function commit(bytes32 orderHash) external {
+  function commit(bytes32 orderHash) external _viewlock_ {
     if (msg.sender != address(SOLUTION_SETTLER)) {
       revert CommitOutsideOfSettlement();
     }
-    if (_getLock() != _MUTEX_FREE) {
-      revert BCoWPool_CommitmentAlreadySet();
-    }
     _setLock(orderHash);
   }
 
   /**
    * @inheritdoc IERC1271
    * @dev this function reverts if the order hash does not match the current commitment
    */
-  function isValidSignature(bytes32 _hash, bytes memory signature) external view returns (bytes4) {
+  function isValidSignature(bytes32 orderHash, bytes memory signature) external view returns (bytes4 magicValue) {
     (GPv2Order.Data memory order) = abi.decode(signature, (GPv2Order.Data));
 
     if (order.appData != APP_DATA) {
       revert AppDataDoesNotMatch();
     }
 
-    bytes32 orderHash = order.hash(SOLUTION_SETTLER_DOMAIN_SEPARATOR);
-    if (orderHash != _hash) {
+    bytes32 orderHash_ = order.hash(SOLUTION_SETTLER_DOMAIN_SEPARATOR);
+    if (orderHash_ != orderHash) {
       revert OrderDoesNotMatchMessageHash();
     }
 
-    if (orderHash != commitment()) {
+    if (orderHash_ != _getLock()) {
       revert OrderDoesNotMatchCommitmentHash();
     }
 
     verify(order);
 
     // A signature is valid according to EIP-1271 if this function returns
     // its selector as the so-called "magic value".
-    return this.isValidSignature.selector;
-  }
-
-  /// @inheritdoc IBCoWPool
-  function commitment() public view returns (bytes32 value) {
-    value = _getLock();
+    magicValue = this.isValidSignature.selector;
   }
 
   /// @inheritdoc IBCoWPool
@@ -107,7 +99,7 @@ contract BCoWPool is IERC1271, IBCoWPool, BPool, BCoWConst {
     if (order.receiver != GPv2Order.RECEIVER_SAME_AS_OWNER) {
       revert BCoWPool_ReceiverIsNotBCoWPool();
     }
-    if (order.validTo >= block.timestamp + MAX_ORDER_DURATION) {
+    if (order.validTo > block.timestamp + MAX_ORDER_DURATION) {
       revert BCoWPool_OrderValidityTooLong();
     }
     if (order.feeAmount != 0) {
@@ -122,7 +114,7 @@ contract BCoWPool is IERC1271, IBCoWPool, BPool, BCoWConst {
 
     uint256 buyTokenBalance = order.buyToken.balanceOf(address(this));
     if (order.buyAmount > bmul(buyTokenBalance, MAX_IN_RATIO)) {
-      revert BPool_TokenAmountInAboveMaxIn();
+      revert BPool_TokenAmountInAboveMaxRatio();
     }
 
     uint256 tokenAmountOut = calcOutGivenIn({
@@ -145,15 +137,16 @@ contract BCoWPool is IERC1271, IBCoWPool, BPool, BCoWConst {
    * pool after the finalization of the setup. Also emits COWAMMPoolCreated() event.
    */
   function _afterFinalize() internal override {
-    for (uint256 i; i < _tokens.length; i++) {
+    uint256 tokensLength = _tokens.length;
+    for (uint256 i; i < tokensLength; i++) {
       IERC20(_tokens[i]).approve(VAULT_RELAYER, type(uint256).max);
     }
 
     // Make the factory emit the event, to be easily indexed by off-chain agents
     // If this pool was not deployed using a bCoWFactory, this will revert and catch
     // And the event will be emitted by this contract instead
     // solhint-disable-next-line no-empty-blocks
-    try IBCoWFactory(_factory).logBCoWPool() {}
+    try IBCoWFactory(FACTORY).logBCoWPool() {}
     catch {
       emit IBCoWFactory.COWAMMPoolCreated(address(this));
     }

src/contracts/BConst.sol

@@ -41,7 +41,7 @@ contract BConst {
   uint256 public constant BPOW_PRECISION = BONE / 10 ** 10;
 
   /// @notice The maximum ratio of input tokens vs the current pool balance.
-  uint256 public constant MAX_IN_RATIO = BONE / 2;
+  uint256 public constant MAX_IN_RATIO = BONE >> 1;
   /// @notice The maximum ratio of output tokens vs the current pool balance.
   uint256 public constant MAX_OUT_RATIO = (BONE / 3) + 1 wei;