Market setters should only be called once
For consistency, all setters should only be called once. This is not the case for the registerDebt
, registerCollateral
, and more functions, which can be called multiple times. This can lead to unexpected behavior if the credit or collateral for a market are changed in the meanwhile, after accounting already existed for the previous credit and collateral.
function registerDebt(address debt_) external originIs(Role.ADMIN) {
_debt = IDebt(payable(debt_));
}
function registerCollateral(
address collateral_
) external originIs(Role.ADMIN) {
_collateral = ICollateral(payable(collateral_));
}
// ... rest of the setters
Moreover, we recommend that an additional check is performed so that all contracts are in sync. For example, ensure that the new debt's market address points to the same market as the collateral's market address and that they both point to address(this)
, the address of the Market contract.
function registerDebt(
address debt_
) external originIs(Role.ADMIN) {
+ require(_collateral.market() == IMarket(debt_).market(), "not same market");
+ require(IMarket(debt_).market() == address(this), "not same market");
_debt = IDebt(payable(debt_));
}
function registerCollateral(
address collateral_
) external originIs(Role.ADMIN) {
+ require(_debt.market() == ICollateral(collateral_).market(), "not same market");
+ require(ICollateral(collateral_).market() == address(this), "not same market");
_collateral = ICollateral(payable(collateral_));
}
// ... likewise for the rest of the setters