-
Notifications
You must be signed in to change notification settings - Fork 6
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!: version 3.1 #282
Open
lekhovitsky
wants to merge
34
commits into
main
Choose a base branch
from
next
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat!: version 3.1 #282
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
In this PR: * new `IBotV3` interface that defines bot's required permissions which are checked in the bot list * special permission bots are deprecated * `BotListV3` interface changes: * redundant `creditManager` parameter removed from most functions that operate on `creditAccount` * `EraseBot` event removed in favor of `SetBotPermissions(..., 0)` * minor implementation improvements that make code more consistent * increase `BotListV3` version to `3_10`
In this PR: * Reserve price feed's role is revised: now it's only used in safe pricing and can't be activated. Thus, `setReservePriceFeedStatus` is removed while `setPriceFeed` is now open to controller. * `trusted` flag is removed from price feed params since it was a mess to configure and only provided small gas savings, its effect can be replicated by setting reserve feed equal to the main one. * Add new and cleanup existing getters (`getTokens`, `reservePriceFeedParams`, etc). * Price oracle can now be used to store whitelisted updatable price feeds and consistently update them. * `PriceOracleV3`'s version is increased to `3_10`.
All collateral tokens are now considered quoted (credit configurator forbids adding non-quoted tokens as collateral), while underlying token is always considered enabled. The former allows to make the system stricter and remove a huge pile of dead code related to enabling/disabling tokens, while the latter allows to prevent liquidations during pause by sending underlying directly to credit account. While it's encouraged to use new credit managers going forward, new facade and configurator are compatible with existing ones, with two important remarks: * all non-quoted collateral tokens must be made such **before** upgrading credit facades and configurators; * open credit accounts with disabled underlying will only have it enabled after the first multicall. Before that, sending underlying directly won't prevent liquidation and might result in the loss of funds. Additionally, this PR removes all functionality related to `revokeAdapterAllowances`.
In this PR: * move `setBotPermissions` inside multicall * overall code simplification and improvements * add params in facade exceptions to make them slightly more useful * confusing `IncreaseDebt` and `DecreaseDebt` events are removed in favour of pool's `Borrow` and `Repay`
To make configuration more flexible, it was decided to remove the `AddressProviderV3` from contract constructors and pass all used contract addresses as constructor arguments. On top of that, configurator/controller division was slightly revised (this one is still WIP though). Finally, we're letting go of some v1/v2 mentions in our codebase. For a more broader view on the new configuration system, track upcoming changes in the [governance repo](https://github.com/Gearbox-protocol/governance) by @0xmikko.
This commit focuses on improving project structure. First of all, `core-v2` references are finally removed. Next, `governance/` directory is dissected: * gauge and rate keeper are moved into `pool/` * staking is moved into `core/` * controller timelock is moved into governance repo
feat: reorganize project
In this PR: * simplify `CreditConfiguratorV3` deployment and migration * rename `RateKeeperV3` into `TumblerV3` and separate configurator and controller's functions * allow stateful IRMs in `PoolV3` * a lot of minor changes (cleaner interfaces separation, version bumps, comment fixes, etc.)
That's the very commit one would expect to see the night before the audit kicks in. Here we add the logic from `PartialLiquidationBotV3` to the credit facade. This approach has a few important benefits: * it doesn't trigger safe pricing in the collateral check * it allows partial liquidations during pause * it properly handles fee-on-transfer underlying The PR also contains some general code improvements: * `CreditConfiguratorV3`'s constructor is more consistent * `feeInterest` is made immutable in `CreditManagerV3`, which prevents debt manipulation by controller and improves storage packing * some dead code is removed from libraries
The motivation is to have three singleton contracts with the same ownership model.
…283) Previously, `ACLNonReentrantTrait` was a Swiss knife that combined all traits we commonly use, which had its benefits, but resulted in contracts having functions that they never use (like `setController` in `CreditFacadeV3` or `pause` in `PoolQuotaKeeperV3`), which is not really good considering that contract size limit, well, exists. This PR solves the original issue and makes separation of all functionality much cleaner.
Lossy liquidations no longer pause credit facade. Instead, they must go through a contract that can enforce various policies on them. For more context, an early implementation can be found in the governance repo. `liquidateCreditAccount` now returns reported loss. Access modifiers of some functions are revised.
Notion of phantom tokens is added to the core. When adding a phantom token as collateral, credit configurator checks that its deposited token is already added as collateral. When withdrawing a phantom token in the credit facade, a call to a withdrawer adapter is made first to unwrap phantom token into its deposited token. Number of optimizer runs is decreased to `1000` (which was actually always used in practice) for all contracts to fit into a size limit. --------- Co-authored-by: Van0k <[email protected]>
Having `contractType` return the full contract name makes more sense and doesn't cost anything since it shouldn't really be queried on-chain. The general rule for naming is to use snake-cased capitalized name of the contract without the `V3` prefix. The exceptions are contracts that implement interfaces `IAdapter`, `IPriceFeed`, `IBot`, `IRateKeeper`, `IInterestRateModel` (also `IZapper`, which is not included in this repo), which start with respective abbreviation.
Previously, `TumblerV3`'s constructor was accepting `pool` as argument and reading `poolQuotaKeeper` from there. This caused complications in deployments since it required a configurator-only function `setPoolQuotaKeeper` to be called between contract creations. This PR changes the constructor to accept `poolQuotaKeeper` instead of the pool, which allows to deploy contracts first and configure them later. Note that this is the only occurrence of such issue in core contracts.
* feat: integration test helper updated for v3.1 * fix: add new adapter configs * fix: remove troublesome vm invocations in tests * fix: remove time warp in test helper
- set of emergency liquidators is now stored in ACL instead of credit facades - credit facade's constructor accepts ACL as argument since deployed pools have legacy acl - other contracts don't use roles at all so they can just inherit ACL even from deployed pools
- `IRateKeeper` now declares the signature of `addToken` - `TumblerV3` accepts pool as constructor argument instead of the quota keeper for consistency
- `CreditManagerV3`'s constructor now accepts liquidation fees as parameters - `ltUnderlying` is now `immutable` - `setFees` performs more checks on passed values - `rampDuration` should be at least 2 days - `newExpirationDate` is updated to be at least 2 weeks from the current timestamp
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.
No description provided.