Release v2.6.0 #564
Merged
Release v2.6.0 #564
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
### Description The verbosity setting for CI tests was set to `4`, which outputs full stack traces for all (even passed) tests. This leads to extremely verbose output that doesn't get fully displayed in GitHub UI (need to download a 52MB, 350k line log file). This makes it more time consuming to pinpoint what specific tests are causing the workflow to fail. [Example](https://github.com/mento-protocol/mento-core/actions/runs/9779856065/job/27000074200?pr=456). I propose we set verbosity to `3`, which should still output stack traces for failing tests. The one thing missing will be setup traces for failing tests, but this should still be a quality of life improvement. ### Other changes Nothing, this is a one byte change. ### Tested Will need to look at CI output for this PR.
### Description Adds a ChainlinkAdapter contract which can relay anwers from a Chainlink AggregatorV3 to SortedOracles. It assumes it is the only reporter for the given rate feed in SortedOracles (e.g. `greater`/`lesserKey` are not needed). Every rate feed that relies on this mechanism should have a separate ChainlinkAdapter deployed, with the appropriate parameters passed to the constructor. To save gas, the adapter is stateless, unupgradeable, and relies on immutable parameters set in the constructor. Next steps: * Integration test of the contract. * Off-chain component that can ping this adapter contract. * Deployment tooling/docs for setting up a rate feed with the ChainlinkAdapter. ### Tested Unit tests. ### Related issues - Fixes #458 --------- Co-authored-by: chapati <philip.paetz@me.com>
### Description This implements a factory contract to deploy Chainlink relayers (see #456). TODO: - [x] Make the factory Ownable and set permissions for functions that affect state - [x] Add NatSpec documentation LATER (follow-up PR, see #481 ): - [x] Use a proxy for upgradability - [x] Add integration tests that test the Factory -> Relayer -> SortedOracles -> BreakerBox flow ### Other changes * export a `contractPath` helper function from `test/utils/Factory.sol`, it's useful for computing the `CREATE2` address in tests ### Tested Unit tests. ### Related issues - Fixes #459 --------- Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: bowd <dumbogdan@gmail.com>
### Description This PR sets up upgradeability for the ChainlinkRelayerFactory (see #476). TODO: * [x] integration test of the ChainlinkRelayerFactory ### Tested Integration tests that: - Setup a proxied ChainlinkRelayerFactory - Ensure the TransparentUpgradeableProxy/ProxyAdmin pattern works correctly - Sets up a new rate feed with an enabled circuit breaker - Tests that price reports coming from the relayer correctly trigger/reset the circuit breaker. ### Related issues - Fixes #459 --------- Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: bowd <dumbogdan@gmail.com> Co-authored-by: boqdan <304771+bowd@users.noreply.github.com> Co-authored-by: baroooo <baranseltekin@gmail.com>
adds the PSO Proxy contract + removes PHP
### Description Late last week it became apparent that Chainlink won't provide all derived rates that we need, and that we will need to compute implicit rates on-chain. This PR extends the relayer such that it can aggregate up to 4 chainlink date feeds into a single price. Each data feed can be inverted (1 / p) to make the dominos line up. ### Other changes Upgraded the tests to solidity 0.8 and fixed the issue with deploying SortedOracles via the factory by pre-deploying libraries as well because then factory links them automatically when calling `vm.getCode`. ### Tested Tests have been extended, and I've also used a slightly new pattern that I didn't use before which I like, and might not be evident from reading the code. Specifically for the relayer we have 4 scenarios depending on how many price feeds it aggregates, so I wrote test contracts that extend each other in such a way where test cases from N-1 price feeds get executed also in the test for N price feeds. ### Related issues N/A ### Backwards compatibility Not backwards compatible but not deployed yet. ### Documentation N/A --------- Co-authored-by: Marcin Chrzanowski <martin@clabs.co> Co-authored-by: Martin <marcin.j.chrzanowski@gmail.com> Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: Nelson Taveras <4562733+nvtaveras@users.noreply.github.com> Co-authored-by: baroooo <baranseltekin@gmail.com>
### Description When we restructured the repo a while ago I moved the proxy contracts to the legacy folder thinking that we might not deploy named proxies in the future, and maybe upgrade to a more modern Proxy implementation, but we have continued to add the proxies inside the `legacy` folder which doesn't feel great as that folder is meant to be removed (and probably can be at this point). So in order to embrace what we're doing I propose collocating the proxy contracts to the `contracts/tokens` directory. And as an addition to that, I propose collocating all proxies with their contracts like Martin also did in the `contracts/oracles` for the new ChainlinkRelayerFactory contract and proxy. So I also moved around the proxies in `contracts/proxies`. This shouldn't really effect anything else. ### Other changes - Renamed stabletokenPSO to stabletokenPHP ### Tested Ran tests, all is well. ### Related issues N/A ### Backwards compatibility YES ### Documentation N/A --------- Co-authored-by: Bayological <6872903+bayological@users.noreply.github.com> Co-authored-by: denviljclarke <60730266+denviljclarke@users.noreply.github.com>
### Description We realised that it would be better operationally to separate the proxy ownership ACL from the deployer ACL, that way we can keep using the `mento-deployer` account for relayer deploy, and use the multisig only for proxy updates. ### Other changes N/A ### Tested Tests added ### Related issues N/A ### Backwards compatibility No, but no mainnet deployment yet. ### Documentation N/A
This reverts commit ac1c540. ### Description Brings back the maxTimestampSpread setting. A visual explanation here: <img width="918" alt="image" src="https://github.com/user-attachments/assets/02dfaadb-f029-4f38-8bd5-965644b4a4b5"> We need it because CELO/USD has a heartbeat of 24hr and PHP/USD 5minutes, therefore we could be in a situation where the spread between reports is quite large, and if we rely only on max timestamp spread we could be reporting with stale data. The rule of thumb is: - Set `maxTimestampSpread` by considering the largest heartbeat frequency. - Set `tokenExpiry` in SortedOracles by considering the shortest heartbeat frequency. ### Other changes N/A ### Tested Yup ### Related issues N/A ### Backwards compatibility N/A ### Documentation N/A --------- Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: philbow61 <80156619+philbow61@users.noreply.github.com>
## 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 <6872903+bayological@users.noreply.github.com> Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: philbow61 <80156619+philbow61@users.noreply.github.com>
### Description After getting more and more annoyed with the CI time and other small issues around this I decided to really understand how big is our need for `--via-ir` compilation. Turns out it's really small. I mean really small. There are just a couple of places where this is causing issues: 1. `Airgrab#claim` has too many variables. I fixed this with a `struct` for the fractal proof, and an internal function to do the lock. Changing it is ok in my opinion. If we want to reuse this contract we'll have to redeploy it anyway, we have a tag for the version when it was deployed, all is well. 2. `GovernanceFactory#createGovernance` has too many variables. I just broke it down in internal functions. Again, I don't see a problem in changing this, even if it's deployed and etc. 3. `StableTokenV2#initialize` has too many arguments. When we made StableTokenV2 we wanted to keep the same interface for the `initialize` method to keep the interfaces between stable tokens consistent, but it was actually a bad call. There's no good reason why we have it that way and it's a method that can anyway only be called once. I would say it's ok to also change this without making a StableTokenV3 for it. Basically we will have a few contracts which will have a different signature for the initialize method that anyway can't be called anymore. But this is the one change the feels least nice. 4. A couple of tests with many variables where I used structs for grouping or broke down into internal functions. What happens afterwards, well: ``` ╰─ forge test [⠊] Compiling... [⠒] Compiling 64 files with Solc 0.5.17 [⠢] Compiling 55 files with Solc 0.8.26 [⠒] Compiling 223 files with Solc 0.8.18 [⠒] Solc 0.5.17 finished in 307.90ms [⠘] Solc 0.8.26 finished in 1.75s [⠒] Solc 0.8.18 finished in 5.50s Compiler run successful with warnings: ``` The whole fucking shabang runs in 5.5s. Jesus. CI takes ~1minute instead of ~15minutes. P.S. Thank you Claude for the bash script. ### Other changes It changed my life. ### Tested Thoroughly. ### Related issues Sleep deprivation. ### Backwards compatibility Who are you calling backward? ### Documentation I read it, yes. --------- Co-authored-by: Bayological <6872903+bayological@users.noreply.github.com> Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: philbow61 <80156619+philbow61@users.noreply.github.com> Co-authored-by: baroooo <baranseltekin@gmail.com>
### Description Another branch PR on THE BIG ONE. This is a long-overdue cleanup and improvement of our fork tests structure. These tests are important and we will want to maintain and grow them, but the structure they were in made this very hard. They were also terribly brittle and fleaky. The [new structure](https://github.com/mento-protocol/mento-core/blob/81bd18666ea088d6df1cfa461d5f902e36a2c49a/test/fork/README.md) I'm proposing aims to tackle two main issues: - Fork tests were executing assertions for all exchanges in a single test making it hard to debug and follow. - Helpers and utility functions weren't structured and were hard to debug and maintain. Note for reviewers: 99% of the code inside of the functions is just copied from the old structure so I wouldn't do a full review of everything, I would focus on the structure itself. I did make some small tweaks to improve flakiness, for example skipping some of the tests when the limits prevent them, which I think is a fair thing to do, as long as the tests run periodically. ### Other changes I added a CI job for running fork-tests daily on `develop`. ### Tested Yeah, they are, after all, tests. ### Related issues N/A ### Backwards compatibility N/A ### Documentation N/A --------- Co-authored-by: Bayological <6872903+bayological@users.noreply.github.com> Co-authored-by: chapati <philip.paetz@me.com> Co-authored-by: philbow61 <80156619+philbow61@users.noreply.github.com> Co-authored-by: baroooo <baranseltekin@gmail.com>
…checks (#506) ### Description CI is currently red on `develop` because I forgot to remove these old contracts from the storage-layout check in CI, and that only happens on push to develop and PR to main (for artifact rate limiting reasons). This PR removes them from the job. ### Other changes I've also reenabled `continue-on-error` on slither because it makes it nicer to work with via the sarif file. In the past we've disabled it because we were hitting the rate-limit on artifact upload and that ended up being a false-nagative failure mode, but that was before we moved the storage-layout check to develop and main only and that was the main hog. We didn't reenable it after doing that but I think now would be a good time to see if it works. If you need to bring it back revert this commit: 754c8be ### Tested Yes ### Related issues N/A ### Backwards compatibility YES ### Documentation N/A
### Description Context: We create 50k locks to check how much gas is used by governance and locking operations for users with high number of locks. It was working fine until a couple of weeks ago. Then we disabled the gas tests to make CI happy. The problem was related to a [recent change on foundry](https://github.com/foundry-rs/foundry/pull/8274/files). They reduced the default gas limit for tests from `9223372036854775807` to `1073741824` which is not enough to create as many locks as we want. This PR fixes the issue by setting the gas limit to the previous default value of `9223372036854775807` or `i64::MAX / 2` and re-enables the gas tests. I don't think this will cause any problems as the only downside of this is if we have an infinite loop in our tests, it will take some time to consume all the available gas before the test fails. ### Other changes - Used a common var `viewCallGas` instead of creating one for each test - Increased the expected view call gas usages from 20k to 30k as they consume slightly higher gas now, probably thats because we removed the gas optimization across repo recently ### Tested ✅ ### Related issues - Fixes [#495](mento-protocol/mento-general#495)
### Description Adds the proxy for GHS which will be used for both cGHS(pilot) and the actual cGHS in future ### Other changes no ### Related issues - mento-protocol/mento-general#576
7704a9b chore: reserve storage gap in new contracts 040b7fe GoodDollar Fork Tests (#530) 2ee14b4 Feat(454): Exchange provider tests (#529) 578747c test: add BancorExchangeProvider pricing tests (#532) fcd3d02 feat: improve tests around the expansion controller (#533) d3a31fa Streamline comments and errors (#528) ec64f0a fix: ☦ pray to the lords of echidna 13d3a98 fix: add back forge install to the CI step c236009 fix: echidna tests c50a198 feat: add Broker liquidity check (#523) 914ba7d Address Slither Issues in GoodDollar/Bancor Contracts (#526) 8bcf3fd Feat/change gdollar modifiers (#524) 379a511 feat: make validate() internal bcbd9aa chore: added "yarn todo" task to log out open todos in the code 0631c60 refactor: renamed SafeERC20 to SafeERC20MintableBurnable 2800297 feat: simplified SafeERC20 e858391 feat: extend open zeppelin's IERC20 instead of duplicating it 196f6be chore: fixed linter and compiler warnings 096efe2 test: fix fork integration test e5308d9 feat: add GoodDollar contracts + tests 3135626 feat: update Broker + TradingLimits to 0.8 and make G$ changes
### Description Fixes the audit finding https://audits.sherlock.xyz/contests/598/voting/44?dashboard_id=14327c9c7bc843a43c46c4efd899f4c1 ### Other changes no ### Tested Unit test ### Related issues - Fixes [#597](mento-protocol/mento-general#597)
### Description This PR adds scaling to the reserve balance read in mintUBIFromReserveBalance in Case the token has less than 18 decimals. ### Other changes ### Tested - added test verifying expected behavior for token with 6 decimals ### Related issues ### Backwards compatibility ### Documentation
### Description fixes issue in how lastUpdated was set: before if 3.5 days passed and a daily expansion was configured, 3 expansions were calculated and the last expansion timestamp was updated to now essentially losing half a day. Over time this could have resulted in the yearly expansion rate not being achieved. ### Other changes ### Tested - added tests for this behavior ### Related issues mento-protocol/mento-general#592 ### Backwards compatibility ### Documentation
### Description Adds slippage protection to `updateRatioForReward()` ### Other changes Simplifies the new ratio calculation formula ### Tested Unit tests ### Related issues - Fixes [#594](mento-protocol/mento-general#594)
### Description Fixes Sherlock issue 46 by using `type(int48).min` ### Other changes updates test for withOverflowOnAdd to test the correct limit ### Tested unit tests ### Related issues - Fixes [#598](mento-protocol/mento-general#598)
### Description This PR tries to solve the discontinued curve problem that was causing the same sell amount when executed in two swaps resulting in a larger amountOut than when sold in one swap. The idea is to apply the exitContribution on the G$ amount being sold and burning that amount. In order to not move on the curve by this burn we increase the reserve ratio in order to keep the price before the exitContribution burn equal to the price after. ### Other changes Added requires that ensure the token Supply always stays at a minimum of 1 wei ### Tested - new tests added ensuring the difference in splitting sells into multiple swaps stays super small compared to the gas costs ### Related issues mento-protocol/mento-general#596 #555 ### Backwards compatibility ### Documentation --------- Co-authored-by: baroooo <baranseltekin@gmail.com>
### Description Scales down the current price to correct decimals dividing it by the reserve asset precision multiplier ### Other changes added some extra tests for swapIn and swapOut with non standard token decimals ### Tested Unit test ### Related issues - Fixes #559
chore: resolve merge conflicts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description