Resupply Update Report

The sanity checks over amountToBurn and minShareBurnAmount applied in addSupportedProtocol() are missing while config is updated in updateSupportedProtocol().

Informational.

Add the sanity checks to updateSupportedProtocol().

Fixed in f6da20f.

The current deployer contract isn't registered in the main registry, skipping the migration logic in the new deployer.

The new ResupplyPairDeployer contract performs a state migration from the previous deployer during construction.

139:         address _previousPairDeployer = IResupplyRegistry(registry).getAddress("DEPLOYER");
140:         if(_previousPairDeployer != address(0)) {
141:             _migrateState(_previousPairDeployer);
142:         }

Currently, the registry (0x10101010E0C3171D894B71B3400668aF311e7D94) doesn't contain the "DEPLOYER" key, causing the migration logic to be skipped.

Informational.

Ensure the current deployer contract is registered to enable the migration mechanism when the new deployer contract is created.

Pair deployer registry check updated to look for PAIR_DEPLOYER key: 7445002.

PAIR_DEPLOYER key set on registry via this transaction: 0xe644dc0e44ec6d4f51ccf9d8966d63f0bcc3edfe26ab93e499d87169d63331f7.

Inside _migrateState() the function loops through supportedProtocols and run getBorrowAndCollateralTokens() to find which protocol id was used for that pair, then it sets the storage mapping to _deployer.collateralId().

However if two supportedProtocols have the same methods to query the _borrowToken and _collateralToken then the loop will exit early and use the first matching protocol's id to get _deployer.collateralId() and store it.

Currently only Frax and Curve are being used and their methods are different so this will happen only if new protocols are supported in the future.

Informational. If the deployer is migrated again with new supported protocols then the mapping may be incorrectly migrated.

Check that _deployer.collateralId() > 0 inside the if(_borrowToken != address(0) && _collateralToken != address(0)) condition.

Agree, this could be an issue if a future supported protocol was added with conflicting selectors. That will not happen before we upgrade the deployer, but nonetheless have re-worked the _migrateState() flow a bit: 7445002.

Instead of hardcoding an arbitrary limit in the pair implementation, it could be more flexible to delegate this logic to the Oracle implementation.

The updated implementation of _updateExchangeRate() implements a hardcoded limit to detect Oracle manipulation attempts.

571:         // Get the latest exchange rate from the oracle
572:         uint256 priceFromOracle = IOracle(_exchangeRateInfo.oracle).getPrices(address(collateral));
573:         //all prices should *normally* be within the 1e18 (or 1e15 for curvelend) range
574:         //reject any prices that are well beyond that
575:         if(priceFromOracle > 1e22){
576:             revert InvalidOraclePrice();
577:         }

While this threshold has been acknowledged by the team as a reasonable limit for the share price, moving this check to the Oracle would improve flexibility as this implementation can be eventually updated in the pair.

Note that this check is not present in other occurrences in the codebase where getPrices() is used, such as previewRedeem() in RedemptionHandler or isSolvent() in the Utilities contract.

Informational.

Delegating this logic to the Oracle would allow different strategies, for example:

• Create an Oracle with a configurable limit.
• Define an Oracle for each protocol to adapt to potential pricing particularities (e.g. account for the 1e15 / 1e18 difference in Curve and Frax).
• Implement more advance strategies such a time-weighted Oracle or a dual Oracle.

Updated Core and Basic Vault Oracle here: c6a80f0.