BaseVault multiple fixes

[0xdcota]

This PR addresses:

• the issue with the first deposit small amount that leads to DOS #214
• add version to vaults and a setter function for vault's ratings in the Chief.sol #224
• out of date provider when blindly calling setActiveProvider() #192

[0xdcota]

Notes:

In forking setup, I split the forking logic from the deploy logic. This is because in Foundry anything done prior to forking does not persist in state. Forge has a workaround with vm.makePersistent() but there is not need to overcomplicate it. The result could be achieved by simply splitting these two tasks.

[0xdcota]

Self note: Need to understand why these are failing: This is a problem with Foundry most likely.

Failing tests:

Encountered 1 failing test in test/forking/mainnet/CompoundV3.t.sol:CompoundV3ForkingTest [FAIL. Reason: Setup failed: Contract 0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9 does not exist and is not marked as persistent, see vm.makePersistent()] setUp() (gas: 0)

Encountered 1 failing test in test/forking/polygon/DForcePolygon.t.sol:DForcePolygonForkingTest [FAIL. Reason: Setup failed: Revert] setUp() (gas: 0)

Encountered 1 failing test in test/forking/polygon/FlasherAaveV3.t.sol:FlasherAaveV3ForkingTest [FAIL. Reason: Setup failed: Revert] setUp() (gas: 0)

Encountered 1 failing test in test/forking/polygon/WePiggyPolygon.t.sol:WePiggyPolygonForkingTest [FAIL. Reason: Setup failed: Revert] setUp() (gas: 0)

---

After commits: 981b788 and 16311eb I was able to make all test pass. There was an issue in where from the constructor, trying to call a function method from an external contract when contract is being deployed, throws an error. Therefore the ILendingProvider method approvedOperator was changed to not call IVault.asset() or IVault.debtAsset().

In addition also note that some test don't consistently pass, but I think this is due to Foundry error.

Optimism tests also fail due to missing OPCODE in foundry forks that mimic optimism L2-evm.

[0xdcota]

@brozorec this is ready for review. Please see notes above.

Nevermind just realized I am missing the editable security rating in the Chief 🤡🫣

[brozorec]

Hey @DaigaroCota I just had a look at this PR. I think I understand why you brought back the debtAsset in the BorrowingVault. It's because it's needed in setProviders, right? If this is the only reason, I might suggest we turn _setProviders(providers) into an abstract method in BaseVault. That will allow the BorrowingVault to implement it by using both _asset and _debtAsset w/o checking nullity and the YieldVault only using _asset.

[brozorec]

Regarding the ratings, I suggest we switch from a string-based rating to a number-based one (1-100). That's easier to handle and far more scalable, at least from an SC perspective.

How this number gets interpreted on the client side is another question. We can display whatever we want: "A+", "A-", ... or 10-base or 20-base. A rating with a range 1-100 gives us enough flexibility.

[0xdcota]

Regarding the ratings, I suggest we switch from a string-based rating to a number-based one (1-100). That's easier to handle and far more scalable, at least from an SC perspective.

How this number gets interpreted on the client side is another question. We can display whatever we want: "A+", "A-", ... or 10-base or 20-base. A rating with a range 1-100 gives us enough flexibility.

Agree. I will simply assign it a uint256, and restrict range [1,100]

[0xdcota]

Hey @DaigaroCota I just had a look at this PR. I think I understand why you brought back the debtAsset in the BorrowingVault. It's because it's needed in setProviders, right? If this is the only reason, I might suggest we turn _setProviders(providers) into an abstract method in BaseVault. That will allow the BorrowingVault to implement it by using both _asset and _debtAsset w/o checking nullity and the YieldVault only using _asset.

Implemented as discussed.

[0xdcota]

@brozorec, this should be ready to merge once approved.

[0xdcota]

@brozorec review comments have been addressed.