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: halmos symbolic tests #21

Merged
merged 64 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
4394925
feat: introduce OptimismSuperchainERC20
agusduha Jul 25, 2024
bb247fa
fix: contract fixes
agusduha Jul 25, 2024
feea60d
feat: add snapshots and semver
agusduha Jul 25, 2024
c23e226
test: add supports interface tests
agusduha Jul 26, 2024
61d10d9
test: add invariant test
agusduha Jul 26, 2024
d1d2f28
feat: add parameters to the RelayERC20 event
agusduha Jul 26, 2024
7dd28e1
fix: typo
agusduha Jul 26, 2024
68bcdca
fix: from param description
agusduha Jul 26, 2024
ca487dc
fix: event signature and interface pragma
0xng Jul 29, 2024
475d83d
feat: add initializer
agusduha Jul 31, 2024
a7fdcf6
feat: use unstructured storage and OZ v5
agusduha Aug 1, 2024
392c49a
Merge branch 'develop' into sc/superchain-erc20
agusduha Aug 2, 2024
1ed5895
feat: update superchain erc20 interfaces
agusduha Aug 2, 2024
8de4f01
fix: adapt storage to ERC7201
agusduha Aug 6, 2024
d167dce
test: add initializable OZ v5 test
agusduha Aug 6, 2024
176d232
Merge branch 'develop' into sc/superchain-erc20
agusduha Aug 6, 2024
e0e5910
fix: invariant docs
agusduha Aug 6, 2024
5514694
fix: ERC165 implementation
agusduha Aug 6, 2024
b3813f8
Merge branch 'develop' into sc/superchain-erc20
agusduha Aug 7, 2024
eac9884
test: improve superc20 invariant (#11)
0xDiscotech Aug 8, 2024
97e08de
Merge branch 'develop' into sc/superchain-erc20
agusduha Aug 8, 2024
97377fe
fix: gas snapshot
agusduha Aug 8, 2024
f6c3f90
chore: configure medusa with basic supERC20 self-bridging
0xteddybear Aug 8, 2024
fcbf7fe
fix: delete dead code
0xteddybear Aug 13, 2024
473a0bb
test: give the fuzzer a head start
0xteddybear Aug 13, 2024
c8ccdc7
feat: create suite for sybolic tests with halmos
0xDiscotech Aug 14, 2024
5f125fa
chore: remove todo comment
0xDiscotech Aug 14, 2024
ca1f668
docs: fix properties order
0xteddybear Aug 14, 2024
8331006
test: document & implement assertions 22, 23 and 24
0xteddybear Aug 13, 2024
466c605
fix: fixes from self-review
0xteddybear Aug 14, 2024
d8ad2b1
test: guide the fuzzer a little bit less
0xteddybear Aug 14, 2024
1a0c46e
feat: add property for burn
0xDiscotech Aug 14, 2024
91d378e
feat: checkpoint
0xDiscotech Aug 14, 2024
7157066
chore: set xdomain sender on failing test
0xDiscotech Aug 14, 2024
5dcb3a8
chore: enhance mocks
0xDiscotech Aug 14, 2024
945d6b6
Merge branch 'chore/setup-medusa' into feat/halmos-symbolic-tests
0xDiscotech Aug 14, 2024
15c04d5
Revert "Merge branch 'chore/setup-medusa' into feat/halmos-symbolic-t…
0xDiscotech Aug 14, 2024
13fc70a
refactor: remove symbolic addresses to make all of the test work
0xDiscotech Aug 15, 2024
4a99ffd
chore: remove console logs
0xDiscotech Aug 15, 2024
741971a
feat: add properties file
0xDiscotech Aug 15, 2024
066a22f
refactor: enhance test on property 7 using direct try catch (now works)
0xDiscotech Aug 15, 2024
2640556
fix: review comments
0xDiscotech Aug 15, 2024
3b831a6
refactor: add symbolic addresses on test functions
0xDiscotech Aug 15, 2024
b65259c
chore: update property
0xDiscotech Aug 15, 2024
22d15bf
Merge branch 'chore/setup-medusa' into feat/halmos-symbolic-tests
0xDiscotech Aug 15, 2024
bc49b4e
refactor: move symbolic folder into properties one
0xDiscotech Aug 15, 2024
cc66e13
refactor: enhance tests using symbolic addresses instead of concrete …
0xDiscotech Aug 15, 2024
0032eb6
chore: remove 0 property natspec
0xDiscotech Aug 15, 2024
cb36b0c
feat: add halmos profile and just script
0xDiscotech Aug 16, 2024
a982ccb
feat: add halmos commands to justfile
0xDiscotech Aug 16, 2024
a83b29e
chore: reorder assertions on one test
0xDiscotech Aug 16, 2024
be57ac8
refactor: complete test property seven
0xDiscotech Aug 16, 2024
787c2d7
chore: mark properties as completed
0xDiscotech Aug 16, 2024
313429f
Merge branch 'chore/setup-medusa' into feat/halmos-symbolic-tests
0xDiscotech Aug 16, 2024
98557e0
chore: add halmos-cheatcodes dependency
0xteddybear Aug 22, 2024
4945501
chore: rename advancedtest->halmosbase
0xteddybear Aug 22, 2024
0fcf844
chore: minimize mocked messenger
0xteddybear Aug 22, 2024
a974787
chore: delete empty halmos file
0xteddybear Aug 22, 2024
4ec3c30
chore: revert changes to medusa.json
0xteddybear Aug 22, 2024
bf49cdf
docs: update changes to PROPERTIES.md from base branch
0xteddybear Aug 22, 2024
50a8577
test: sendERC20 destination fix
0xteddybear Aug 22, 2024
2928e5a
chore: natspec fixes
0xteddybear Aug 22, 2024
99b26fa
Merge pull request #32 from defi-wonderland/chore/feedback-on-halmos-…
0xteddybear Aug 23, 2024
542b539
chore: merge base branch & fix conflicts
0xteddybear Aug 23, 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 .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,6 @@
[submodule "packages/contracts-bedrock/lib/openzeppelin-contracts-v5"]
path = packages/contracts-bedrock/lib/openzeppelin-contracts-v5
url = https://github.com/OpenZeppelin/openzeppelin-contracts
[submodule "packages/contracts-bedrock/lib/halmos-cheatcodes"]
path = packages/contracts-bedrock/lib/halmos-cheatcodes
url = https://github.com/a16z/halmos-cheatcodes
8 changes: 7 additions & 1 deletion packages/contracts-bedrock/foundry.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ remappings = [
'ds-test/=lib/forge-std/lib/ds-test/src',
'safe-contracts/=lib/safe-contracts/contracts',
'kontrol-cheatcodes/=lib/kontrol-cheatcodes/src',
'gelato/=lib/automate/contracts'
'gelato/=lib/automate/contracts',
'halmos-cheatcodes/=lib/halmos-cheatcodes/'
]
extra_output = ['devdoc', 'userdoc', 'metadata', 'storageLayout']
bytecode_hash = 'none'
Expand Down Expand Up @@ -101,3 +102,8 @@ script = 'test/kontrol/proofs'
src = 'test/properties/medusa/'
test = 'test/properties/medusa/'
script = 'test/properties/medusa/'

[profile.halmos]
src = 'test/properties/halmos/'
test = 'test/properties/halmos/'
script = 'test/properties/halmos/'
15 changes: 15 additions & 0 deletions packages/contracts-bedrock/halmos.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# Halmos configuration file

## The version needed is `halmos 0.1.15.dev2+gc3f45dd`
## Just running `halmos` will run the tests with the default configuration

[global]
# Contract to test
match-contract = "SymTest_"

# Path to the Forge artifacts directory
forge_build_out = "./forge-artifacts"


# Storage layout
storage_layout = "generic"
7 changes: 7 additions & 0 deletions packages/contracts-bedrock/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ test-kontrol:
test-medusa timeout='100':
FOUNDRY_PROFILE=medusa medusa fuzz --timeout {{timeout}}


test-halmos-all VERBOSE="-v":
FOUNDRY_PROFILE=halmos halmos {{VERBOSE}}

test-halmos TEST VERBOSE="-v":
FOUNDRY_PROFILE=halmos halmos --function {{TEST}} {{VERBOSE}}

Choose a reason for hiding this comment

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


test-rerun: build-go-ffi
forge test --rerun -vvv

Expand Down
1 change: 1 addition & 0 deletions packages/contracts-bedrock/lib/halmos-cheatcodes
Submodule halmos-cheatcodes added at c0d865
73 changes: 73 additions & 0 deletions packages/contracts-bedrock/test/invariants/PROPERTIES.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# supertoken properties

legend:

- `[ ]`: property not yet tested
- `**[ ]**`: property not yet tested, dev/research team has asked for extra focus on it
- `[X]`: tested/proven property
- `[~]`: partially tested/proven property
- `:(`: property won't be tested due to some limitation

## Unit test

| id | description | halmos | medusa |
| --- | ---------------------------------------------------------------------------------- | ------ | ------ |
| 0 | supertoken token address does not depend on the executing chain’s chainID | [ ] | [ ] |
| 1 | supertoken token address depends on name, remote token, address and decimals | [ ] | [ ] |
| 2 | convert() should only allow converting legacy tokens to supertoken and viceversa | [ ] | [ ] |
| 3 | convert() only allows migrations between tokens representing the same remote asset | [ ] | [ ] |
| 4 | convert() only allows migrations from tokens with the same decimals | [ ] | [ ] |
| 5 | convert() burns the same amount of one token that it mints of the other | [ ] | [ ] |

## Valid state

| id | description | halmos | medusa |
| --- | ------------------------------------------------------------------------------------------ | ------- | ------ |
| 6 | calls to sendERC20 succeed as long as caller has enough balance | [x] | [ ] |
| 7 | calls to relayERC20 always succeed as long as the sender and cross-domain caller are valid | **[~]** | [ ] |

## Variable transition

| id | description | halmos | medusa |
| --- | ------------------------------------------------------------------------------------------------- | ------ | ------ |
| 8 | sendERC20 with a value of zero does not modify accounting | [x] | [ ] |
| 9 | relayERC20 with a value of zero does not modify accounting | [x] | [ ] |
| 10 | sendERC20 decreases the token's totalSupply in the source chain exactly by the input amount | [x] | [ ] |
| 11 | relayERC20 increases the token's totalSupply in the destination chain exactly by the input amount | [x] | [ ] |
| 12 | supertoken total supply only increases on calls to mint() by the L2toL2StandardBridge | [x] | [ ] |
| 13 | supertoken total supply only decreases on calls to burn() by the L2toL2StandardBridge | [x] | [ ] |
| 14 | supertoken total supply starts at zero | [x] | [ ] |
| 15 | deploying a supertoken registers its remote token in the factory | [ ] | [ ] |
| 16 | deploying an OptimismMintableERC20 registers its remote token in the factory | [ ] | [ ] |

## High level

| id | description | halmos | medusa |
| --- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ------ |
| 17 | only calls to convert(legacy, super) can increase a supertoken’s total supply across chains | [ ] | [ ] |
| 18 | only calls to convert(super, legacy) can decrease a supertoken’s total supply across chains | [ ] | [ ] |
| 19 | sum of total supply across all chains is always <= to convert(legacy, super)- convert(super, legacy) | [ ] | [ ] |
| 20 | tokens sendERC20-ed on a source chain to a destination chain can be relayERC20-ed on it as long as the source chain is in the dependency set of the destination chain | [ ] | [ ] |
| 21 | sum of supertoken total supply across all chains is = to convert(legacy, super)- convert(super, legacy) when all cross-chain messages are processed | [ ] | [ ] |

## Atomic bridging pseudo-properties

As another layer of defense, the following properties are defined which assume bridging operations to be atomic (that is, the sequencer and L2Inbox and CrossDomainMessenger contracts are fully abstracted away, `sendERC20` triggering the `relayERC20` call on the same transaction)
It’s worth noting that these properties will not hold for a live system

| id | description | halmos | medusa |
| --- | ---------------------------------------------------------------------------------------------------------------------------------- | ------ | ------ |
| 22 | sendERC20 decreases sender balance in source chain and increases receiver balance in destination chain exactly by the input amount | [ ] | [x] |
| 23 | sendERC20 decreases total supply in source chain and increases it in destination chain exactly by the input amount | [ ] | [x] |
| 24 | sum of supertoken total supply across all chains is always equal to convert(legacy, super)- convert(super, legacy) | [ ] | [~] |

# Expected external interactions

- regular ERC20 operations between any accounts on the same chain, provided by [crytic ERC20 properties](https://github.com/crytic/properties?tab=readme-ov-file#erc20-tests)

# Invariant-breaking candidates (brain dump)

here we’ll list possible interactions that we intend the fuzzing campaign to support in order to help break invariants

- [ ] changing the decimals of tokens after deployment
- [ ] `convert()` ing between multiple (3+) representations of the same remote token, by having different names/symbols
23 changes: 12 additions & 11 deletions packages/contracts-bedrock/test/properties/PROPERTIES.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ Given the [OP monorepo](https://github.com/ethereum-optimism/optimism) already h
# Ecosystem properties

legend:

- `[ ]`: property not yet tested
- `**[ ]**`: property not yet tested, dev/research team has asked for extra focus on it
- `[X]`: tested/proven property
Expand All @@ -73,22 +74,22 @@ legend:

## Valid state

| id | milestone | description | halmos | medusa |
| --- | --- | --- | --- | --- |
| 6 | SupERC20 | calls to sendERC20 succeed as long as caller has enough balance | [ ] | [ ] |
| 7 | SupERC20 | calls to relayERC20 always succeed as long as the cross-domain caller is valid | **[ ]** | [ ] |
| id | milestone | description | halmos | medusa |
| --- | --- | --- | --- | --- |
| 6 | SupERC20 | calls to sendERC20 succeed as long as caller has enough balance | [x] | [ ] |
| 7 | SupERC20 | calls to relayERC20 always succeed as long as the sender and cross-domain caller are valid | **[~]** | [ ] |

## Variable transition

| id | milestone | description | halmos | medusa |
| --- | --- | --- | --- | --- |
| 8 | SupERC20 | sendERC20 with a value of zero does not modify accounting | [ ] | [ ] |
| 9 | SupERC20 | relayERC20 with a value of zero does not modify accounting | [ ] | [ ] |
| 10 | SupERC20 | sendERC20 decreases the token's totalSupply in the source chain exactly by the input amount | [ ] | [ ] |
| 11 | SupERC20 | relayERC20 increases the token's totalSupply in the destination chain exactly by the input amount | [ ] | [ ] |
| 12 | Liquidity Migration | supertoken total supply only increases on calls to mint() by the L2toL2StandardBridge | [ ] | [~] |
| 13 | Liquidity Migration | supertoken total supply only decreases on calls to burn() by the L2toL2StandardBridge | [ ] | [ ] |
| 14 | SupERC20 | supertoken total supply starts at zero | [ ] | [x] |
| 8 | SupERC20 | sendERC20 with a value of zero does not modify accounting | [x] | [ ] |
| 9 | SupERC20 | relayERC20 with a value of zero does not modify accounting | [x] | [ ] |
| 10 | SupERC20 | sendERC20 decreases the token's totalSupply in the source chain exactly by the input amount | [x] | [ ] |
| 11 | SupERC20 | relayERC20 increases the token's totalSupply in the destination chain exactly by the input amount | [x] | [ ] |
| 12 | Liquidity Migration | supertoken total supply only increases on calls to mint() by the L2toL2StandardBridge | [x] | [~] |
| 13 | Liquidity Migration | supertoken total supply only decreases on calls to burn() by the L2toL2StandardBridge | [x] | [ ] |
| 14 | SupERC20 | supertoken total supply starts at zero | [x] | [x] |
| 15 | Factories | deploying a supertoken registers its remote token in the factory | [ ] | [ ] |
| 16 | Factories | deploying an OptimismMintableERC20 registers its remote token in the factory | [ ] | [ ] |

Expand Down

Choose a reason for hiding this comment

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

I managed to get the contract down to this:

contract MockL2ToL2Messenger {
    // Setting the current cross domain sender for the check of sender address equals the supertoken address
    address internal immutable CROSS_DOMAIN_SENDER;
    constructor(address _xDomainSender) {
        CROSS_DOMAIN_SENDER = _xDomainSender;
    }
    function sendMessage(uint256 , address , bytes calldata) external payable {
    }
    function crossDomainMessageSource() external view returns (uint256 _source) {
        _source = block.chainid + 1;
    }
    function crossDomainMessageSender() external view returns (address _sender) {
        _sender = CROSS_DOMAIN_SENDER;
    }
}

with all tests passing. I believe the extra code makes it a bit confusing for the reader

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// SPDX-License-Identifier: MIT
pragma solidity 0.8.25;


// TODO: Try to merge to a single mocked contract used by fuzzing and symbolic invariant tests - only if possible
// and low priorty
contract MockL2ToL2Messenger {
// Setting the current cross domain sender for the check of sender address equals the supertoken address
address internal immutable CROSS_DOMAIN_SENDER;

constructor(address _xDomainSender) {
CROSS_DOMAIN_SENDER = _xDomainSender;
}

function sendMessage(uint256 , address , bytes calldata) external payable {
}

function crossDomainMessageSource() external view returns (uint256 _source) {
_source = block.chainid + 1;
}

function crossDomainMessageSender() external view returns (address _sender) {
_sender = CROSS_DOMAIN_SENDER;
}
}
Loading