search

Gearbox-protocol / core-v3

Other
28 stars 4 forks source link

fix: adapter configuration fixes #257

Closed lekhovitsky closed 1 month ago

lekhovitsky commented 1 month ago

Fixes:

Acknowledges:

StErMi commented 1 month ago

@lekhovitsky

allowAdapter natspec should be updated because now the function does not (or at least not directly) reverts if adapter or adapter's targetContract are equal to the creditManager because this check is performed directly inside the CreditManagerV3(creditManager).contractToAdapter(...) call

cryptarasecurity commented 1 month ago

@lekhovitsky

allowAdapter natspec should be updated because now the function does not (or at least not directly) reverts if adapter or adapter's targetContract are equal to the creditManager because this check is performed directly inside the CreditManagerV3(creditManager).contractToAdapter(...) call

I do agree that natspec could be updated to at least state that the check is performed indireclty as the check is now only performed inside the CreditManagerV3(creditManager).setContractAllowance function which does follow the recommendation of issue https://github.com/spearbit-audits/review-gearbox/issues/61 by removing the check under the configurator due to duplications on the CreditManagerV3.

lekhovitsky commented 1 month ago

@StErMi @cryptarasecurity It's fine for user-facing contracts like credit configurator or credit facade to make comments about checks or other actions performed in the credit manager, so decided not to update the comment.

StErMi commented 1 month ago

@lekhovitsky the TargetContractNotAllowedException used in allowAdapter is not entirely correct because the function reverts if the target contract OR the adapter are equal to the CreditFacade. The same would be for the setCreditFacade function.

StErMi commented 1 month ago

For the rest, the PR seems to fix/address all the linked issues linked above.

lekhovitsky commented 1 month ago

@StErMi Exception names are quite a mess but fixing them all consistently again creates a huge diff which nobody would want to review, so keeping those as is