Skip to content

Commit

Permalink
THE BIG ONE 🐘 💣 🚝 (#501)
Browse files Browse the repository at this point in the history
## Description

As I told some of you at dinner, I've had a bad case of insomnia over
the last week. This PR resulted from a couple of late night coding
sessions and the incessant need to make things nicer in our repos.

It's a big PR but 𝚫code is negative so 🤷 .

What happens in this mondo-PR: 

1. All `celo` contracts are removed form the repo and replaced with the
`@celo/contracts` NPM package. With a small caveat.
2. All `interfaces` are made to work with `0.8` and cover more of the
contract functions.
3. All `tests` contracts are updated to `0.8` and use `deployCode`
helpers to deploy lower-version contracts, and the use the interfaces to
interact with them.
4. ~We make use of this foundry-rs/foundry#8668
to improve compile time.~
5. Update `solhint` and `prettier` to a recent version and fix/ignore
all issues.
6. Fix solc compiler warnings.
7. Fix/ignore slither > informational warnings.
8. Slight Refactor of ForkTests to make them better to work with.
9. Restructuring of the tests folder.

#### `@celo/contracts`

The only caveat of using the `@celo/contracts` package is that
`UsingRegistry.sol` in there needs to import things from `mento-core`
and I didn't manage to get it working ok with remappings, so I kept a
copy of `UsingRegistry.sol` in `contracts/common`. It's only used in
`swap/Reserve.sol` and when we remove it from there we can completely
kill `common`.

#### The foundry WIP PR

A better solution was found here #502, which removes the need for
`via-ir` completely.

~The reason it takes so long to compile our code is because we need
`via-ir` for `Airgrab.sol` and `StableTokenV2.sol`, and `via-ir` is
super slow. But with the compiler restrictions implemented in
foundry-rs/foundry#8668 we can have multiple
compiler profile settings for subgraphs of the source-graph, which
compile in parallel with different settings.~

~You can easily install that version of foundry locally, (you have to
have rust installed tho):~
```
foundryup -P 8668
```

~With this version of foundry and the settings in `foundry.toml`, if
you're not working in a part of the source graph that contains
`Airgrab.sol` or `StableTokenV2.sol`, compilation will happen without
`via-ir` and will be super snappy. However if you do touch that source
graph it will still take noticeably long. Right now on my machine full
clean compilation takes 80seconds. It used to take >3minutes.~

#### ForkTest Refactoring

Our fork tests can get a bit heavy because they're running test
assertions for all the exchanges in a single test call. I've refactor it
a bit and split out exchange assertions into their own tests contracts
that need to be manually created when new exchanges are deployed.
There's a chain-level assertion on the number of exchanges that helps us
catch this and keep them up to date.

This work was continued here #503 for a much cleaner solution.

### Other changes

> _Describe any minor or "drive-by" changes here._

no minor changes here, no :))

### Tested

Tested? Tested!

### Related issues

Fixes the issues in my head.

### Backwards compatibility

What even is that?

### Documentation

Holy Bible

---------

Co-authored-by: Bayological <[email protected]>
Co-authored-by: chapati <[email protected]>
Co-authored-by: philbow61 <[email protected]>
  • Loading branch information
4 people authored Aug 26, 2024
1 parent f0d5316 commit 45a551d
Show file tree
Hide file tree
Showing 211 changed files with 3,754 additions and 11,903 deletions.
21 changes: 19 additions & 2 deletions .github/workflows/echidna.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,32 @@ jobs:
- uses: actions/checkout@v3
with:
submodules: recursive

- name: "Install Node.js"
uses: "actions/setup-node@v3"
with:
cache: "yarn"
node-version: "20"

- name: "Install the Node.js dependencies"
run: "yarn install --immutable"

- name: Install Foundry
uses: foundry-rs/foundry-toolchain@v1

- name: "Build for echidna"
run: forge build --build-info --skip */test/**/*.t.sol */script/**
run: |
forge build --build-info --skip \
"test/fork/**/*" \
"test/integration/**/*" \
"test/unit/**/*" \
"test/utils/**/*" \
"script/**/"
- name: "Run Echidna"
uses: crytic/echidna-action@v2
with:
files: .
solc-version: 0.5.17
contract: ${{ matrix.contract }}
config: echidna.yaml
test-mode: assertion
4 changes: 2 additions & 2 deletions .github/workflows/lint_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ jobs:
run: "yarn install --immutable"

- name: "Lint the contracts"
run: "yarn lint:check"
run: "yarn lint"

- name: "Add lint summary"
run: |
Expand All @@ -50,7 +50,7 @@ jobs:
- name: "Build the contracts"
run: |
forge --version
forge build --sizes
forge build --sizes --skip test/**/*
- name: "Add test summary"
run: |
Expand Down
13 changes: 11 additions & 2 deletions .github/workflows/slither.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,20 @@ jobs:
uses: "actions/checkout@v3"
with:
submodules: "recursive"
- name: "Install Node.js"
uses: "actions/setup-node@v3"
with:
cache: "yarn"
node-version: "20"

- name: "Install the Node.js dependencies"
run: "yarn install --immutable"
- name: Run Slither
uses: crytic/slither-action@v0.3.1
uses: crytic/slither-action@v0.4.0
id: slither
with:
sarif: results.sarif
fail-on: "low"
# continue-on-error: true
# -----------------------
# Ideally, we'd like to continue on error to allow uploading the SARIF file here.
Expand All @@ -36,6 +45,6 @@ jobs:
# know it failed.
# -----------------------
- name: Upload SARIF file
uses: github/codeql-action/upload-sarif@v2
uses: github/codeql-action/upload-sarif@v3
with:
sarif_file: ${{ steps.slither.outputs.sarif }}
12 changes: 12 additions & 0 deletions .github/workflows/storage-layout.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
name: "Storage Layout"

env:
FOUNDRY_PROFILE: "ci"

on:
workflow_dispatch:
push:
Expand Down Expand Up @@ -32,6 +36,14 @@ jobs:
uses: onbjerg/foundry-toolchain@v1
with:
version: "nightly"
- name: "Install Node.js"
uses: "actions/setup-node@v3"
with:
cache: "yarn"
node-version: "20"

- name: "Install the Node.js dependencies"
run: "yarn install --immutable"
- name: Check storage layout
uses: Rubilmax/[email protected]
with:
Expand Down
12 changes: 6 additions & 6 deletions .gitmodules
Original file line number Diff line number Diff line change
@@ -1,18 +1,12 @@
[submodule "lib/openzeppelin-contracts"]
path = lib/openzeppelin-contracts
url = https://github.com/OpenZeppelin/openzeppelin-contracts
[submodule "lib/celo-foundry"]
path = lib/celo-foundry
url = https://github.com/bowd/celo-foundry
[submodule "lib/openzeppelin-contracts-next"]
path = lib/openzeppelin-contracts-next
url = https://github.com/OpenZeppelin/openzeppelin-contracts
[submodule "lib/openzeppelin-contracts-upgradeable"]
path = lib/openzeppelin-contracts-upgradeable
url = https://github.com/OpenZeppelin/openzeppelin-contracts-upgradeable
[submodule "lib/forge-std-next"]
path = lib/forge-std-next
url = https://github.com/foundry-rs/forge-std
[submodule "lib/safe-contracts"]
path = lib/safe-contracts
url = https://github.com/safe-global/safe-contracts
Expand All @@ -23,3 +17,9 @@
branch = "release-v4"
path = lib/prb-math
url = https://github.com/PaulRBerg/prb-math
[submodule "lib/mento-std"]
path = lib/mento-std
url = https://github.com/mento-protocol/mento-std
[submodule "lib/forge-std"]
path = lib/forge-std
url = https://github.com/foundry-rs/forge-std
2 changes: 1 addition & 1 deletion .husky/pre-push
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env sh
. "$(dirname -- "$0")/_/husky.sh"

yarn lint:check
yarn lint
4 changes: 2 additions & 2 deletions .prettierrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ singleQuote: false
tabWidth: 2
trailingComma: all

plugins: ["prettier-plugin-solidity"]

overrides:
- files: ["*.sol"]
options:
compiler: 0.5.17
tabWidth: 2
printWidth: 120
- files: [contracts/tokens/patched/*.sol]
options:
compiler: 0.8.18
Expand Down
18 changes: 15 additions & 3 deletions .solhint.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,17 @@
"extends": "solhint:recommended",
"plugins": ["prettier"],
"rules": {
"no-global-import": "off",
"no-console": "off",
"code-complexity": ["error", 8],
"compiler-version": ["error", ">=0.5.13"],
"func-visibility": ["error", { "ignoreConstructors": true }],
"max-line-length": ["error", 120],
"func-visibility": [
"error",
{
"ignoreConstructors": true
}
],
"max-line-length": ["error", 121],
"not-rely-on-time": "off",
"function-max-lines": ["error", 120],
"no-empty-blocks": "off",
Expand All @@ -15,6 +22,11 @@
"endOfLine": "auto"
}
],
"reason-string": ["warn", { "maxLength": 64 }]
"reason-string": [
"warn",
{
"maxLength": 64
}
]
}
}
22 changes: 18 additions & 4 deletions .solhint.test.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,21 @@
"extends": "solhint:recommended",
"plugins": ["prettier"],
"rules": {
"one-contract-per-file": "off",
"no-global-import": "off",
"no-console": "off",
"code-complexity": ["error", 8],
"compiler-version": ["error", ">=0.5.13"],
"func-visibility": ["error", { "ignoreConstructors": true }],
"max-line-length": ["error", 120],
"func-visibility": [
"error",
{
"ignoreConstructors": true
}
],
"max-line-length": ["error", 121],
"not-rely-on-time": "off",
"function-max-lines": ["error", 120],
"function-max-lines": ["error", 121],
"gas-custom-errors": "off",
"max-states-count": "off",
"var-name-mixedcase": "off",
"func-name-mixedcase": "off",
Expand All @@ -21,6 +30,11 @@
"endOfLine": "auto"
}
],
"reason-string": ["warn", { "maxLength": 64 }]
"reason-string": [
"warn",
{
"maxLength": 64
}
]
}
}
9 changes: 0 additions & 9 deletions contracts/common/CalledByVm.sol

This file was deleted.

27 changes: 0 additions & 27 deletions contracts/common/ExternalCall.sol

This file was deleted.

Loading

0 comments on commit 45a551d

Please sign in to comment.