test: Revise on-chain governance architectures to allow more flexible Yearn Governance designs. Revise README

Author: Detoo

README-yearnBorg.md

@@ -79,22 +79,77 @@ all coming operations listed above will require approval of both `ychad.eth` and
 
 ### Future On-chain Governance Transition
 
-Yearn's Snapshot governance will be replaced with an on-chain governance at some point (ex. `YearnGovExecutor`).
-`YearnGovExecutor` (or its adapter) must satisfy the following requirements to integrate with the co-approval process:
-- Each proposal must include generic transaction fields (`target`, `value`, `calldata` or their equivalents) to enable `YearnGovExecutor` to execute the proposal upon approval
-- Proposals involving `ychad.eth` [Restricted Admin Operations](#restricted-admin-operations) must be executed solely by `ychad.eth` to enforce co-approval requirements
+Yearn's Snapshot-based governance will transition to an on-chain governance system (ex. `YearnGovernance`). 
+An adapter (`YearnGovernanceAdapter`) will be implemented by MetaLex to manage the implementation details on co-approval process. 
+To integrate successfully, `YearnGovernance` must meet the following requirements:
+
+- Each proposal has an unique ID (ex. `proposalId`)
+- `YearnGovernanceAdapter` can read the proposal's voting result and verify it is passed
+- `YearnGovernanceAdapter` can extract the admin operation (ex. `target`, `value`, `calldata` or equivalent) from the proposal
 
 The transition process from Snapshot to on-chain governance is listed as follows:
 
-1. A final Snapshot proposal will be submitted to replace `Snapshot Executor` with `YearnGovExecutor` by transferring ownership of `SudoImplant` and `EjectImplant` to `YearnGovExecutor`
-2. Once co-approved and executed by `ychad.eth`, the transition process is complete
+1. A final Snapshot proposal will be submitted to grant `YearnGovernanceAdapter` ownership of the implants 
+2. `ychad.eth` to co-approved and executed the proposal
+3. The first on-chain proposal will be submitted to revoke `SnapShotExecutor` ownership of the implants
+4. `ychad.eth` to co-approved and executed the proposal. The transition is now complete
 
 After the transition, the co-approval process will become:
 
 1. Operation is initiated on the MetaLeX OS webapp
 2. An on-chain proposal will be submitted to `YearnGovExecutor`
 3. Once the vote passed, `ychad.eth` will co-approve it by executing the operation through the MetaLeX OS webapp
 
+Below shows the changes of BORG architectures before/after on-chain governance transition:
+
+```mermaid
+graph TD
+    ychad[ychad.eth<br/>6/9 signers]
+    
+    subgraph offChainGovernance["Snapshot Governance (before)"]
+        yearnDaoVoting[Yearn DAO Voting Snapshot]
+        oracleAddr[oracle]
+        snapshotExecutor[Snapshot Executor]
+    end
+    
+    subgraph onChainGovernance["On-chain Governance (after)"]
+        yearnGovernance[Yearn Governance]
+        yearnGovernanceAdapter[Yearn Governance Adapter]
+    end
+    
+    subgraph implants["Implants (Modules)"]
+        ejectImplant{{Eject Implant}}
+        sudoImplant{{Sudo Implant}}
+    end
+    
+    ychad -->|"owner<br>execute(proposalId)"| snapshotExecutor
+    
+    oracleAddr -->|"oracle<br>propose(admin operation)"| snapshotExecutor      
+    oracleAddr -->|monitor| yearnDaoVoting
+    
+    snapshotExecutor -->|"owner<br>admin operation()"| implants
+    
+    ychad -->|"owner<br>execute(proposalId)"| yearnGovernanceAdapter
+    
+    yearnGovernanceAdapter -->|"verify voting results of proposalId<br>and extract the admin operation"| yearnGovernance
+    yearnGovernanceAdapter -->|"owner<br>admin operation()"| implants
+    
+    %% Styling (optional, Mermaid supports limited styling)
+    classDef default fill:#191918,stroke:#fff,stroke-width:2px,color:#fff;
+    classDef borg fill:#191918,stroke:#E1FE52,stroke-width:2px,color:#E1FE52;
+    classDef yearn fill:#191918,stroke:#2C68DB,stroke-width:2px,color:#2C68DB;
+    classDef safe fill:#191918,stroke:#76FB8D,stroke-width:2px,color:#76FB8D;
+    classDef todo fill:#191918,stroke:#F09B4A,stroke-width:2px,color:#F09B4A;
+    class ejectImplant borg;
+    class sudoImplant borg;
+    class snapshotExecutor borg;
+    class oracleAddr borg;
+    class yearnGovernanceAdapter borg;
+    class ychad yearn;
+    class yearnDaoVoting yearn;
+    class yearnGovernance yearn;
+```
+
 ### Module Addition
 
 New Modules grant `ychad.eth` privileges to bypass Guards restrictions, therefore it requires DAO co-approval via [Co-approval Workflows](#co-approval-workflows).

test/yearnBorgAcceptance.t.sol

@@ -3,39 +3,67 @@ pragma solidity 0.8.20;
 
 import "forge-std/Test.sol";
 import "solady/tokens/ERC20.sol";
-import {Ownable} from "openzeppelin/contracts/access/Ownable.sol";
 import {borgCore} from "../src/borgCore.sol";
 import {ejectImplant} from "../src/implants/ejectImplant.sol";
 import {sudoImplant} from "../src/implants/sudoImplant.sol";
-import {BorgAuth} from "../src/libs/auth.sol";
+import {BorgAuth, BorgAuthACL} from "../src/libs/auth.sol";
 import {SnapShotExecutor} from "../src/libs/governance/snapShotExecutor.sol";
 import {SafeTxHelper} from "./libraries/safeTxHelper.sol";
 import {IGnosisSafe, GnosisTransaction, IMultiSendCallOnly} from "../test/libraries/safe.t.sol";
 
-contract YearnGovExecutor is Ownable {
-    struct proposal {
+contract MockYearnGovernance {
+    struct Proposal {
         address target;
         uint256 value;
         bytes cdata;
         string description;
     }
 
-    mapping(bytes32 => proposal) public pendingProposals;
-    
-    constructor(address owner) Ownable(owner) {}
+    mapping(bytes32 => Proposal) public proposals;
+
+    // Assume this is how to get a proposal's content (including admin operation's data)
+    function getProposal(bytes32 proposalId) external returns (Proposal memory) {
+        return proposals[proposalId];
+    }
+
+    // Assume this is how to verify a proposal is passed
+    function isProposalPassed(bytes32 proposalId) external returns (bool) {
+        return true;
+    }
 
-    // Propose for voting
-    function propose(address target, uint256 value, bytes calldata cdata, string memory description) external returns (bytes32) {
-        bytes32 proposalId = keccak256(abi.encodePacked(target, value, cdata, description));
-        pendingProposals[proposalId] = proposal(target, value, cdata, description);
+    // Assume this is how to propose an admin operation
+    function propose(Proposal calldata p) external returns (bytes32) {
+        bytes32 proposalId = keccak256(abi.encodePacked(p.target, p.value, p.cdata, p.description));
+        proposals[proposalId] = p;
         return proposalId;
     }
+}
+
+contract MockYearnGovernanceAdapter is BorgAuthACL {
+    error YearnGovernanceAdapter_ProposalNotPassed(bytes32 proposalId);
+    error YearnGovernanceAdapter_ProposalAlreadyExecuted(bytes32 proposalId);
+
+    MockYearnGovernance yearnGovernance;
+    mapping(bytes32 => bool) public proposalExecuted;
 
-    // Execute passed proposal (for testing we assume it always passes)
+    constructor(BorgAuth _auth, MockYearnGovernance _yearnGovernance) BorgAuthACL(_auth) {
+        yearnGovernance = _yearnGovernance;
+    }
+
+    // Only owner (ychad.eth) is allowed to execute the admin operation. This is part of the co-approval process.
     function execute(bytes32 proposalId) payable external onlyOwner() {
-        proposal memory p = pendingProposals[proposalId];
+        if (!yearnGovernance.isProposalPassed(proposalId)) {
+            revert YearnGovernanceAdapter_ProposalNotPassed(proposalId);
+        }
+
+        if (proposalExecuted[proposalId]) {
+            revert YearnGovernanceAdapter_ProposalAlreadyExecuted(proposalId);
+        }
+
+        MockYearnGovernance.Proposal memory p = yearnGovernance.getProposal(proposalId);
+        proposalExecuted[proposalId] = true;
+
         (bool success, ) = p.target.call{value: p.value}(p.cdata);
-        delete pendingProposals[proposalId];
     }
 }
 
@@ -269,28 +297,32 @@ contract YearnBorgAcceptanceTest is Test {
 
     /// @dev Transition to on-chain governance should be successful with co-approval
     function testOnChainGovernanceTransition() public {
-        YearnGovExecutor yearnGovExecutor = new YearnGovExecutor(address(ychadSafe));
+        // Yearn to deploy on-chain governance contract
+        MockYearnGovernance yearnGovernance = new MockYearnGovernance();
+
+        // MetaLeX to deploy adapter
+        MockYearnGovernanceAdapter yearnGovernanceAdapter = new MockYearnGovernanceAdapter(snapShotExecutor.AUTH(), yearnGovernance);
 
         BorgAuth implantAuth = eject.AUTH();
         uint256 ownerRole = implantAuth.OWNER_ROLE();
 
         // Should not be owner yet
-        vm.expectRevert(abi.encodeWithSelector(BorgAuth.BorgAuth_NotAuthorized.selector, ownerRole, address(yearnGovExecutor)));
-        implantAuth.onlyRole(ownerRole, address(yearnGovExecutor));
+        vm.expectRevert(abi.encodeWithSelector(BorgAuth.BorgAuth_NotAuthorized.selector, ownerRole, address(yearnGovernanceAdapter)));
+        implantAuth.onlyRole(ownerRole, address(yearnGovernanceAdapter));
 
         // Simulate on-chain governance transition
         {
-            // SnapShotExecutor to add YearnGovExecutor as owner
+            // SnapShotExecutor to add yearnGovernanceAdapter as owner
             vm.prank(oracle);
             bytes32 proposalIdAddOwner = snapShotExecutor.propose(
                 address(implantAuth), // target
                 0, // value
                 abi.encodeWithSelector(
                     implantAuth.updateRole.selector,
-                    address(yearnGovExecutor),
+                    address(yearnGovernanceAdapter),
                     ownerRole
                 ), // cdata
-                "Add yearnGovExecutor as owner"
+                "Add yearnGovernanceAdapter as owner"
             );
 
             // After waiting period
@@ -306,28 +338,28 @@ contract YearnBorgAcceptanceTest is Test {
                 )
             }));
 
-            // YearnGovExecutor should be an owner now
-            implantAuth.onlyRole(ownerRole, address(yearnGovExecutor));
+            // yearnGovernanceAdapter should be an owner now
+            implantAuth.onlyRole(ownerRole, address(yearnGovernanceAdapter));
 
-            // YearnGovExecutor to remove SnapShotExecutor's ownership
-            bytes32 proposalIdRemoveOwner = yearnGovExecutor.propose(
-                address(implantAuth), // target
-                0, // value
-                abi.encodeWithSelector(
+            // YearnGovernance to revoke SnapShotExecutor ownership
+            bytes32 proposalIdRevokeOwner = yearnGovernance.propose(MockYearnGovernance.Proposal({
+                target: address(implantAuth),
+                value: 0,
+                cdata: abi.encodeWithSelector(
                     implantAuth.updateRole.selector,
                     address(snapShotExecutor),
                     0
-                ), // cdata
-                "Remove snapShotExecutor ownership"
-            );
+                ),
+                description: "Revoke snapShotExecutor ownership"
+            }));
 
-            // Execute the proposal
+            // Execute the passed proposal
             safeTxHelper.executeSingle(GnosisTransaction({
-                to: address(yearnGovExecutor),
+                to: address(yearnGovernanceAdapter),
                 value: 0,
                 data: abi.encodeWithSelector(
-                    yearnGovExecutor.execute.selector,
-                    proposalIdRemoveOwner
+                    MockYearnGovernanceAdapter.execute.selector,
+                    proposalIdRevokeOwner
                 )
             }));
 
@@ -341,26 +373,26 @@ contract YearnBorgAcceptanceTest is Test {
             vm.assertFalse(ychadSafe.isOwner(alice), "Should not be Safe signer");
 
             // Simulate a proposal (and it is immediately passed)
-            bytes32 proposalId = yearnGovExecutor.propose(
-                address(eject), // target
-                0, // value
-                abi.encodeWithSelector(
+            bytes32 proposalId = yearnGovernance.propose(MockYearnGovernance.Proposal({
+                target: address(eject),
+                value: 0,
+                cdata: abi.encodeWithSelector(
                     bytes4(keccak256("addOwner(address)")),
                     alice // newOwner
-                ), // cdata
-                "Add Alice as new signer"
-            );
+                ),
+                description: "Add Alice as new signer"
+            }));
 
             // Should fail if not executed from ychad.eth
-            vm.expectRevert(abi.encodeWithSelector(Ownable.OwnableUnauthorizedAccount.selector, address(this)));
-            yearnGovExecutor.execute(proposalId);
+            vm.expectRevert(abi.encodeWithSelector(BorgAuth.BorgAuth_NotAuthorized.selector, ownerRole, address(this)));
+            yearnGovernanceAdapter.execute(proposalId);
 
             // Should succeed if executed from ychad.eth
             safeTxHelper.executeSingle(GnosisTransaction({
-                to: address(yearnGovExecutor),
+                to: address(yearnGovernanceAdapter),
                 value: 0,
                 data: abi.encodeWithSelector(
-                    yearnGovExecutor.execute.selector,
+                    MockYearnGovernanceAdapter.execute.selector,
                     proposalId
                 )
             }));