search

Gearbox-protocol / core-v3

Other
28 stars 4 forks source link

feat: adapters return `useSafePrices` flag #268

Closed lekhovitsky closed 1 month ago

lekhovitsky commented 1 month ago

Fixes https://github.com/spearbit-audits/review-gearbox/issues/107

StErMi commented 1 month ago

@lekhovitsky as far as I can see from the current implementation of the existing adapters (see https://github.com/Gearbox-protocol/integrations-v3/blob/main/contracts/adapters)

1) they are not upgradable 2) each adapter's function signature has the return (uint256 tokensToEnable, uint256 tokensToDisable) which is incompatible with the new code that expects at minimum to have a bool returned as the first part of the returned data. The new CreditFacadeV3 contract will always revert when it calls an adapter with the "old" signature.

How do you plan to manage this? This would mean that you would need to re-deploy each adapter with the new return signature structure and that new CreditFacadeV3 contracts (with v3.1) will not be able to support existing (old) adapters.

lekhovitsky commented 1 month ago

Please switch to the @next branch, it's updated to return this flag. The migration process is trivial, forbid old adapters and connect new ones.

Redeploying is totally fine, adapters are almost stateless, so that's a common operation for us.

StErMi commented 1 month ago

The changes made to the CreditFacadeV3 will enable both the USE_SAFE_PRICES_FLAG and REVERT_ON_FORBIDDEN_TOKENS_FLAG flags if/when needed (depending on the adapter and the underlying wrapped function called on the target contract). The changes satisfy the recommendations provided in the finding.

Gearbox should consider clarifying and documenting how they plan the deployment and configuration of all the new system given that the current CreditFacade v3.0 is incompatible with the new adapters (that needs to return a bool) and the new CreditFacade v3.1 will be incompatible with the old ones (that return 2 uint256)