Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(test): fuzzed and symbolic tests #146

Merged
merged 43 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
21bc497
Merge commit 'ce1b925d296ac1bb5dbaf5c688c8592d43f64e36' into feat/fuz…
simon-something Jul 9, 2024
5b1182b
feat: echidna and halmos bmath bnum protocol
simon-something Jul 9, 2024
bec1de2
fix: remove tests halmos cannot handle
simon-something Jul 9, 2024
1d0d05d
fix: setup cow pool
simon-something Jul 9, 2024
013f72f
chore(wip): echidna
simon-something Jul 9, 2024
388c657
feat: clamp util
simon-something Jul 11, 2024
caf3c62
feat(echidna): prop and tob erc20
simon-something Jul 12, 2024
b01b4e6
test(echidna): protocol prop (part)
simon-something Jul 14, 2024
3753371
test(echidna): swap/join prop (part)
simon-something Jul 15, 2024
d12abd4
test(echidna): all properties
simon-something Jul 15, 2024
8cde0c0
test(halmos): part
simon-something Jul 15, 2024
0d0ca16
feat: summary prop md and last tests
simon-something Jul 16, 2024
87a9acb
chore: reorg
simon-something Jul 16, 2024
e946f09
feat: more bnum pain
simon-something Jul 16, 2024
59c2686
chore: format
simon-something Jul 20, 2024
462c38e
chore: format
simon-something Jul 20, 2024
68a6b59
chore: typo
simon-something Jul 20, 2024
fdd4617
chore: typo
simon-something Jul 20, 2024
cad698a
test(echidna): more bnum tests
simon-something Jul 22, 2024
c143808
chore: unused import
simon-something Jul 22, 2024
a65a9c0
chore: more assert in protocol
simon-something Jul 22, 2024
350c39d
feat: evm version
simon-something Jul 23, 2024
c60f5fa
chore: summary
simon-something Jul 23, 2024
c5c263f
fix: direct transfer then swapExactOut case
simon-something Jul 23, 2024
09d332d
chore: fmt
simon-something Jul 23, 2024
888ebe9
feat: shanghai in summary
simon-something Jul 23, 2024
0c0e1c4
fix: typo
simon-something Jul 26, 2024
00cd7d1
chore: typo
simon-something Jul 26, 2024
ea280d9
chore: revert consistency
simon-something Jul 26, 2024
5555afc
chore: summary fmt
simon-something Jul 26, 2024
637aa83
chore: fmt
simon-something Jul 26, 2024
42d2a04
Fix/fuzz tests improvements (#178)
0xteddybear Jul 29, 2024
302463c
chore: merge dev
wei3erHase Jul 29, 2024
23a8264
chore: update forge snapshots
wei3erHase Jul 29, 2024
b3f2cf6
chore: merge
wei3erHase Jul 29, 2024
5abef51
chore: update gas snapshots
wei3erHase Jul 29, 2024
a793e9d
chore: updating Properties document after merge
wei3erHase Jul 31, 2024
61a0930
Merge pull request #185 from defi-wonderland/chore/merge-dev-into-fuzz
wei3erHase Jul 31, 2024
df49eb9
dev: improving bmath fuzz test (#184)
wei3erHase Aug 1, 2024
7bbb6cc
Merge branch 'dev' into feat/fuzz-symb-tests
wei3erHase Aug 1, 2024
9d976ff
docs: update test summary (#189)
0xteddybear Aug 1, 2024
b101648
Merge branch 'dev' into feat/fuzz-symb-tests
wei3erHase Aug 1, 2024
cd1ef62
chore: fuzz sym fixes (#190)
0xteddybear Aug 2, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,6 @@ broadcast/*/*/*

# Out dir
out

# echidna corpuses
**/corpuses/*
1 change: 1 addition & 0 deletions .solhintignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
test/smock/*
test/manual-smock/*
test/invariants/*

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are not that many warnings, we could disable some rules (such as custom-errors) for the entire directory and add a few inline ignores (in the case of no-empty-blocks, for example)

1 change: 1 addition & 0 deletions crytic-export/combined_solc.json
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¯_(ツ)_/¯

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,16 @@ multiline_func_header = 'params_first'
sort_imports = true

[profile.default]
solc_version = '0.8.25'
wei3erHase marked this conversation as resolved.
Show resolved Hide resolved
solc_version = '0.8.23'
libs = ["node_modules", "lib"]
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]
# 1858: Some imports don't have the license identifier
ignored_error_codes = [2018, 2394, 5574, 3860, 1878]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WHICH ONES??! 👀

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ToB properties...

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1858 or 1878?

deny_warnings = true

[profile.optimized]
Expand Down
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
},
"dependencies": {
"@cowprotocol/contracts": "github:cowprotocol/contracts.git#a10f40788a",
"@crytic/properties": "https://github.com/crytic/properties.git",
"@openzeppelin/contracts": "5.0.2",
"composable-cow": "github:cowprotocol/composable-cow.git#24d556b",
"solmate": "github:transmissions11/solmate#c892309"
Expand All @@ -50,7 +51,8 @@
"@defi-wonderland/natspec-smells": "1.1.3",
"@defi-wonderland/smock-foundry": "1.5.0",
"forge-gas-snapshot": "github:marktoda/forge-gas-snapshot#9161f7c",
"forge-std": "github:foundry-rs/forge-std#5475f85",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will change the deployed build...

"forge-std": "github:foundry-rs/forge-std#1.8.2",
"halmos-cheatcodes": "github:a16z/halmos-cheatcodes#c0d8655",
"husky": ">=8",
"lint-staged": ">=10",
"solhint-community": "4.0.0",
Expand Down
2 changes: 2 additions & 0 deletions remappings.txt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ solmate/=node_modules/solmate/src
@cowprotocol/=node_modules/@cowprotocol/contracts/src/contracts
cowprotocol/=node_modules/@cowprotocol/contracts/src/
@composable-cow/=node_modules/composable-cow/
halmos-cheatcodes=node_modules/halmos-cheatcodes
@crytic/=node_modules/@crytic/

contracts/=src/contracts
interfaces/=src/interfaces
2 changes: 1 addition & 1 deletion script/DeployBCoWFactory.s.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BCoWFactory} from 'contracts/BCoWFactory.sol';
import {Script} from 'forge-std/Script.sol';
Expand Down
2 changes: 1 addition & 1 deletion script/DeployBFactory.s.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BFactory} from 'contracts/BFactory.sol';
import {Script} from 'forge-std/Script.sol';
Expand Down
2 changes: 1 addition & 1 deletion script/Params.s.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.23;

contract Params {
struct BFactoryDeploymentParams {
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BCoWConst.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

/**
* @title BCoWConst
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BCoWFactory.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BCoWPool} from './BCoWPool.sol';
import {BFactory} from './BFactory.sol';
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BCoWPool.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

/*

Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BConst.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

/**
* @title BConst
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BFactory.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BPool} from './BPool.sol';
import {SafeERC20} from '@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol';
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BMath.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BConst} from './BConst.sol';
import {BNum} from './BNum.sol';
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/BNum.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BConst} from './BConst.sol';

Expand Down
22 changes: 14 additions & 8 deletions src/contracts/BPool.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BMath} from './BMath.sol';
import {BToken} from './BToken.sol';
Expand Down Expand Up @@ -30,6 +30,10 @@ contract BPool is BToken, BMath, IBPool {
/// @dev Sum of all token weights
uint256 internal _totalWeight;

/// TEST TEST TEST TEST TEST TEST TEST TEST
bytes32 internal _reenteringMutex;
/// TEST TEST TEST TEST TEST TEST TEST TEST
simon-something marked this conversation as resolved.
Show resolved Hide resolved

/// @dev Logs the call data
modifier _logs_() {
emit LOG_CALL(msg.sig, msg.sender, msg.data);
Expand Down Expand Up @@ -148,8 +152,8 @@ contract BPool is BToken, BMath, IBPool {

_pullUnderlying(token, msg.sender, balance);
}

/// @inheritdoc IBPool

wei3erHase marked this conversation as resolved.
Show resolved Hide resolved
function unbind(address token) external _logs_ _lock_ _controller_ _notFinalized_ {
if (!_records[token].bound) {
revert BPool_TokenNotBound();
Expand Down Expand Up @@ -597,9 +601,10 @@ contract BPool is BToken, BMath, IBPool {
* be interpreted as locked
*/
function _setLock(bytes32 value) internal virtual {
assembly ("memory-safe") {
tstore(_MUTEX_TRANSIENT_STORAGE_SLOT, value)
}
// assembly ("memory-safe") {
// tstore(_MUTEX_TRANSIENT_STORAGE_SLOT, value)
// }
_reenteringMutex = value;
}
0xteddybear marked this conversation as resolved.
Show resolved Hide resolved

/**
Expand Down Expand Up @@ -670,8 +675,9 @@ contract BPool is BToken, BMath, IBPool {
* allowing calls
*/
function _getLock() internal view virtual returns (bytes32 value) {
assembly ("memory-safe") {
value := tload(_MUTEX_TRANSIENT_STORAGE_SLOT)
}
// assembly ("memory-safe") {
// value := tload(_MUTEX_TRANSIENT_STORAGE_SLOT)
// }
value = _reenteringMutex;
}
}
2 changes: 1 addition & 1 deletion src/contracts/BToken.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {ERC20} from '@openzeppelin/contracts/token/ERC20/ERC20.sol';

Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IBCoWFactory.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {IBFactory} from 'interfaces/IBFactory.sol';

Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IBCoWPool.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {GPv2Order} from '@cowprotocol/libraries/GPv2Order.sol';
import {IERC1271} from '@openzeppelin/contracts/interfaces/IERC1271.sol';
Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IBFactory.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {IBPool} from 'interfaces/IBPool.sol';

Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/IBPool.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0-or-later
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {IERC20} from '@openzeppelin/contracts/token/ERC20/IERC20.sol';

Expand Down
2 changes: 1 addition & 1 deletion src/interfaces/ISettlement.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-3.0
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {IERC20} from '@cowprotocol/interfaces/IERC20.sol';
import {GPv2Interaction} from '@cowprotocol/libraries/GPv2Interaction.sol';
Expand Down
88 changes: 88 additions & 0 deletions test/SUMMARY.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Tests Summary

There are 9 solidity files, totalling 939 sloc.

| filename | language | code | comment | blank | total |
| :---------------------------- | :------- | :--- | :------ | :---- | :---- |
| src/contracts/BCoWConst.sol | solidity | 4 | 10 | 2 | 16 |
| src/contracts/BCoWFactory.sol | solidity | 20 | 13 | 7 | 40 |
| src/contracts/BCoWPool.sol | solidity | 90 | 41 | 24 | 155 |
| src/contracts/BConst.sol | solidity | 23 | 29 | 9 | 61 |
| src/contracts/BFactory.sol | solidity | 44 | 17 | 11 | 72 |
| src/contracts/BMath.sol | solidity | 128 | 156 | 18 | 302 |
| src/contracts/BNum.sol | solidity | 133 | 40 | 28 | 201 |
| src/contracts/BPool.sol | solidity | 473 | 104 | 107 | 684 |
| src/contracts/BToken.sol | solidity | 24 | 27 | 7 | 58 |


## Interdependencies
BCoWFactory deploys a bcowpool
BCoWPool is a bpool which adds signature validation
Factory deploys a pool and can "collect" from the pool
Pool inherit btoken (which represents a LP) and bmath
Bmath uses bnum

# Unit tests
Current coverage is XXX % for the 9 contracts, accross XXX tests. All tests are passing. Unit tests are writtent using the branched-tree technique and Bulloak as templating tool.
simon-something marked this conversation as resolved.
Show resolved Hide resolved

< TABLE >

# Integration tests

# Property tests
We identified 24 properties. We challenged these either in a long-running fuzzing campaign (targeting 23 of these in XXX runs) or via symbolic execution (for 8 properties).

## Fuzzing campaign

We used echidna to test these 23 properties. In addition to these, another fuzzing campaign as been led against the mathematical contracts (BNum and BMath), insuring the operation properties were holding.

Limitations/future improvements

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be a section?

Currently, the swap logic are tested against the swap in/out functions (and, in a similar way, liquidity management via the join/exit function). The combined equivalent (joinswapExternAmountIn, joinswapPoolAmountOut, etc) should be tested too.

## Formal verification: Symbolic Execution
We managed to test 10 properties out of the 23. Properties not tested are either not easily challenged with symbolic execution (statefullness needed) or limited by Halmos itself (hitting loops in the implementation for instance).

Additional properties from BNum were tested independently too (with severe limitations due to loop unrolling boundaries).


## Notes
The bmath corresponding equations are:

`Spot price:`
$\text{spotPrice} = \frac{\text{tokenBalanceIn}/\text{tokenWeightIn}}{\text{tokenBalanceOut}/\text{tokenWeightOut}} \cdot \frac{1}{1 - \text{swapFee}}$


`Out given in:`
$\text{tokenAmountOut} = \text{tokenBalanceOut} \cdot \left( 1 - \left( \frac{\text{tokenBalanceIn}}{\text{tokenBalanceIn} + \left( \text{tokenAmountIn} \cdot \left(1 - \text{swapFee}\right)\right)} \right)^{\frac{\text{tokenWeightIn}}{\text{tokenWeightOut}}} \right)$

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this expression doesn't seem to be renderable

Also, perhaps expressions as blocks would be easier to read? text in inline expressions gets real small

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true true



`In given out:`
$\text{tokenAmountIn} = \frac{\text{tokenBalanceIn} \cdot \left( \frac{\text{tokenBalanceOut}}{\text{tokenBalanceOut} - \text{tokenAmountOut}} \right)^{\frac{\text{tokenWeightOut}}{\text{tokenWeightIn}}} - 1}{1 - \text{swapFee}}$


`Pool out given single in`
$\text{poolAmountOut} = \left(\frac{\text{tokenAmountIn} \cdot \left(1 - \left(1 - \frac{\text{tokenWeightIn}}{\text{totalWeight}}\right) \cdot \text{swapFee}\right) + \text{tokenBalanceIn}}{\text{tokenBalanceIn}}\right)^{\frac{\text{tokenWeightIn}}{\text{totalWeight}}} \cdot \text{poolSupply} - \text{poolSupply}$


`Single in given pool out`
$\text{tokenAmountIn} = \frac{\left(\frac{\text{poolSupply} + \text{poolAmountOut}}{\text{poolSupply}}\right)^{\frac{1}{\frac{\text{weightIn}}{\text{totalWeight}}}} \cdot \text{balanceIn} - \text{balanceIn}}{\left(1 - \frac{\text{weightIn}}{\text{totalWeight}}\right) \cdot \text{swapFee}}$


`Single out given pool in`
$\text{tokenAmountOut} = \left( \text{tokenBalanceOut} - \left( \frac{\text{poolSupply} - \left(\text{poolAmountIn} \cdot \left(1 - \text{exitFee}\right)\right)}{\text{poolSupply}} \right)^{\frac{1}{\frac{\text{tokenWeightOut}}{\text{totalWeight}}}} \cdot \text{tokenBalanceOut} \right) \cdot \left(1 - \left(1 - \frac{\text{tokenWeightOut}}{\text{totalWeight}}\right) \cdot \text{swapFee}\right)$


`Pool in given single out`
$\text{poolAmountIn} = \frac{\text{poolSupply} - \left( \frac{\text{tokenBalanceOut} - \frac{\text{tokenAmountOut}}{1 - \left(1 - \frac{\text{tokenWeightOut}}{\text{totalWeight}}\right) \cdot \text{swapFee}}}{\text{tokenBalanceOut}} \right)^{\frac{\text{tokenWeightOut}}{\text{totalWeight}}} \cdot \text{poolSupply}}{1 - \text{exitFee}}$


BNum bpow is based on exponentiation by squaring and hold true because (see dapphub dsmath): https://github.com/dapphub/ds-math/blob/e70a364787804c1ded9801ed6c27b440a86ebd32/src/math.sol#L62
```
// If n is even, then x^n = (x^2)^(n/2).
// If n is odd, then x^n = x * x^(n-1),
// and applying the equation for even x gives
// x^n = x * (x^2)^((n-1) / 2).
//
// Also, EVM division is flooring and
// floor[(n-1) / 2] = floor[n / 2].
```
2 changes: 1 addition & 1 deletion test/integration/BCowPool.t.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BPoolIntegrationTest} from './BPool.t.sol';
import {GPv2TradeEncoder} from '@composable-cow/test/vendored/GPv2TradeEncoder.sol';
Expand Down
2 changes: 1 addition & 1 deletion test/integration/BPool.t.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {IERC20} from '@cowprotocol/interfaces/IERC20.sol';
import {BFactory} from 'contracts/BFactory.sol';
Expand Down
2 changes: 1 addition & 1 deletion test/integration/DeploymentGas.t.sol
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;
pragma solidity 0.8.23;

import {BFactory} from 'contracts/BFactory.sol';

Expand Down
Loading
Loading