Conversation
LucasCambon
commented
Dec 30, 2025
- Upgrade Solidity version to 0.8.30
- Upgrade EVM version to praga (same as OZ)
- Upgrade @ensuro/utils to latest
- Upgrade @ensuro/swaplibrary to latest
- Upgrade OZ to 5.5 & remove dead code contracts
- Move strategies to contracts/strategies
- Upgrade to Hardhat 3
…ate @ensuro utils & swaplibrary, upgrade OZ to 5.5 & remove dead code contracts
gnarvaja
left a comment
There was a problem hiding this comment.
Revisé algunas cosas, después sigo con el resto.
| string memory name_, | ||
| string memory symbol_, | ||
| address admin_, | ||
| address, |
There was a problem hiding this comment.
Cambialo para que ya no reciba este parámetro. Sería simplemente cambiar los tests, este contrato no se usa más que en este repo.
There was a problem hiding this comment.
Entiendo que ahora este contrato queda sin ningún permiso, todos los métodos abiertos. Tiene sentido para un contrato que se usa para tests.
hardhat.config.js
Outdated
| dependencyCompiler: { | ||
| paths: [ | ||
| "@ensuro/utils/contracts/TestCurrency.sol", | ||
| "@ensuro/utils/contracts/TestCurrencyAC.sol", |
There was a problem hiding this comment.
Creo que sería mejor usar el TestCurrency a secas, sin access control. No me parece que necesitemos para los tests que tenga access control y los hace más complicados.
Lo mismo en el repo de CashFlowLender.
| }, | ||
| }); | ||
| const strategy = await CompoundV3InvestStrategy.deploy(ADDRESSES.cUSDCv3, ADDRESSES.REWARDS); | ||
| const MultiStrategyERC4626 = await ethers.getContractFactory("MultiStrategyERC4626"); |
There was a problem hiding this comment.
Esta variante, en lugar de borrarla, habría que hacer que sea usando el AccessManagedMSV. Salvo que eso ya esté probado más abajo en otra variante.
There was a problem hiding this comment.
Ah, esta creo que ya está cubierta en CompoundV3Strategy+AccessManaged
| name: "AAVEV3Strategy", | ||
| cToken: ADDRESSES.aUSDCv3, | ||
| supplyToken: ADDRESSES.USDC, | ||
| fixture: async () => { |
There was a problem hiding this comment.
Esta variante, en lugar de borrarla, habría que hacer que sea usando el AccessManagedMSV. Salvo que eso ya esté probado más abajo en otra variante.
There was a problem hiding this comment.
Esta me parece que no. Habría que hacer una AAVEV3Strategy+AccessManaged
| const specificRole = await vault.getForwardToStrategyRole(0, operation); | ||
| await grantRole(hre, vault.connect(admin), specificRole, user); | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Idem, no borrar las variantes (ni los tests que testean cosas específicas de estas), adaptarlas para que trabajen con AccessManagedMSV.
- Improve coverage % & branches % - Add deleted variants tests from AAVE & SwapStable strategies - Delete admin from SingleStrategyERC4626.sol
34c4d4a to
ba548f3
Compare
Avoids duplication of code between the variants. Removes complexitity that came when multiple heterogenous variants were active, now that all are AccessManaged, we don't need to mention it.