Feat/governance wireup v2

[sajanrajdev]

The following changes were introduced:

• feeRecipientAddress was made immutable everywhere: As per the new governance design, it was already intended to burn the capability of changing the fee recipient address at launch. Instead, the variable was made immutable all together:
  • Testes were adapted accordingly.
  • Setters and update events were removed.
  • This addresses the issue found on the CollSurplusPool where this address was inherited from the ActivePool and made immutable. This is now proper and by design.
• Multiple changes were made to the governance wiring:
  • Roles were redefined with further separation in order to introduce higher flexibility and the naming convention was made more cohesive.
  • User roles wiring was modified in accordance to the latest design. Supporting notes for the new structure are being finalized and will be shared soon.
• EcosystemMultisig name was changed for SecurityMultisig in accordance to latest design.

NOTE: Making the fee recipient immutable means, of course, that the address used upon deployment will not be changeable. An opportunity exists for this recipient to be a proxy of some sorts instead of a multisig to allow for a certain degree of flexibility. In the same way as the multisig, any alternative contract should be owned by the Treasury. For this reason, this should be decided by the Treasury.

[getrecon-bot]

Job ID	7a489f4c-6a43-40a3-9ad8-d75dc5cbd7dc
Command	yarn && git submodule init && git submodule update && solc-select use 0.8.17 && cd packages/contracts/ && yarn echidna --test-mode assertion --test-limit 600000
Instance ID	i-0b73b64e762bbf5cf
Instance Type	c5.2xlarge
Status	Started
Elapsed

[getrecon-bot]

Job ID	7a489f4c-6a43-40a3-9ad8-d75dc5cbd7dc
Command	yarn && git submodule init && git submodule update && solc-select use 0.8.17 && cd packages/contracts/ && yarn echidna --test-mode assertion --test-limit 600000
Instance ID	i-0b73b64e762bbf5cf
Instance Type	c5.2xlarge
Status	Running
Elapsed	37 seconds

[getrecon-bot]

Job ID	7a489f4c-6a43-40a3-9ad8-d75dc5cbd7dc
Command	yarn && git submodule init && git submodule update && solc-select use 0.8.17 && cd packages/contracts/ && yarn echidna --test-mode assertion --test-limit 600000
Instance ID	i-0b73b64e762bbf5cf
Instance Type	c5.2xlarge
Status	Running
Elapsed	38 seconds

[CodingNameKiki]

Hey @sajanrajdev.

In the constructor of coll surplus pool, the feeRecipientAddress is fetched from the active pool storage.

CollSurplusPool.sol

        feeRecipientAddress = IActivePool(activePoolAddress).feeRecipientAddress();

The question is - can we refactor the constructor of active pool to fetch the fee recipient address from BorrowerOperations as well. If l understood correctly BorrowerOperations will be deployed before both pools so we can do this change instead of setting the fee recipient two times in both contracts.

Final result - borrower operations is deployed with a particular fee recipient address and both active & surplus pools use this address as fee recipient.

ActivePool.sol

    constructor(
        address _borrowerOperationsAddress,
        address _cdpManagerAddress,
        address _collTokenAddress,
        address _collSurplusAddress,
        address _feeRecipientAddress
    ) TwapWeightedObserver(0) {
        borrowerOperationsAddress = _borrowerOperationsAddress;
        cdpManagerAddress = _cdpManagerAddress;
        collateral = ICollateralToken(_collTokenAddress);
        collSurplusPoolAddress = _collSurplusAddress;
-       feeRecipientAddress = _feeRecipientAddress;
+       feeRecipientAddress = IBorrowerOperations(borrowerOperationsAddress).feeRecipientAddress();;

        // TEMP: read authority to avoid signature change
        address _authorityAddress = address(AuthNoOwner(cdpManagerAddress).authority());
        if (_authorityAddress != address(0)) {
            _initializeAuthority(_authorityAddress);
        }

        require(systemDebt == 0, "ActivePool: systemDebt should be 0 for TWAP initialization");
    }

[sajanrajdev]

Good call @CodingNameKiki, might as well have only one input of this variable into the system and remove the room for error at setup. Will make the change.