Merge branch 'delta' into vg/feat/treasury-contribution

livepeer · Aug 25, 2023 · 48a9230 · 48a9230
2 parents a65a5b0 + ad48642
commit 48a9230
Show file tree

Hide file tree

Showing 9 changed files with 807 additions and 68 deletions.
diff --git a/contracts/bonding/BondingManager.sol b/contracts/bonding/BondingManager.sol
@@ -555,6 +555,17 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         // Current bonded amount
         uint256 currentBondedAmount = del.bondedAmount;
 
+        // Requirements for a third party caller that is not the L2Migrator
+        if (msg.sender != _owner && msg.sender != l2Migrator()) {
+            // Does not trigger self-delegation
+            // Does not change the delegate if it is already non-null
+            if (delegatorStatus(_owner) == DelegatorStatus.Unbonded) {
+                require(_to != _owner, "INVALID_DELEGATE");
+            } else {
+                require(currentDelegate == _to, "INVALID_DELEGATE_CHANGE");
+            }
+        }
+
         if (delegatorStatus(_owner) == DelegatorStatus.Unbonded) {
             // New delegate
             // Set start round
@@ -563,8 +574,6 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
             // Unbonded state = no existing delegate and no bonded stake
             // Thus, delegation amount = provided amount
         } else if (currentBondedAmount > 0 && currentDelegate != _to) {
-            // Prevents third-party caller to change the delegate of a delegator
-            require(msg.sender == _owner || msg.sender == l2Migrator(), "INVALID_CALLER");
             // A registered transcoder cannot delegate its bonded stake toward another address
             // because it can only be delegated toward itself
             // In the future, if delegation towards another registered transcoder as an already
@@ -678,14 +687,13 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
         // the `autoClaimEarnings` modifier has been replaced with its internal function as a `Stack too deep` error work-around
         _autoClaimEarnings(msg.sender);
         Delegator storage oldDel = delegators[msg.sender];
+        Delegator storage newDel = delegators[_delegator];
         // Cache delegate address of caller before unbondWithHint because
         // if unbondWithHint is for a full unbond the caller's delegate address will be set to null
         address oldDelDelegate = oldDel.delegateAddress;
 
         unbondWithHint(_amount, _oldDelegateNewPosPrev, _oldDelegateNewPosNext);
 
-        Delegator storage newDel = delegators[_delegator];
-
         uint256 oldDelUnbondingLockId = oldDel.nextUnbondingLockId.sub(1);
         uint256 withdrawRound = oldDel.unbondingLocks[oldDelUnbondingLockId].withdrawRound;
 
@@ -709,6 +717,10 @@ contract BondingManager is ManagerProxyTarget, IBondingManager {
 
         // Rebond lock for new owner
         if (newDel.delegateAddress == address(0) && newDel.bondedAmount == 0) {
+            // Requirements for caller
+            // Does not trigger self-delegation
+            require(oldDelDelegate != _delegator, "INVALID_DELEGATOR");
+
             newDel.delegateAddress = oldDelDelegate;
         }
 

diff --git a/deployments/arbitrumMainnet/BondingManagerTarget.json b/deployments/arbitrumMainnet/BondingManagerTarget.json
diff --git a/deployments/arbitrumMainnet/solcInputs/b4307db432f8a65a8c8ad6b3664fc085.json b/deployments/arbitrumMainnet/solcInputs/b4307db432f8a65a8c8ad6b3664fc085.json
diff --git a/doc/upgrade.md b/doc/upgrade.md
@@ -0,0 +1,71 @@
+# Upgrade ManagerProxy Contract
+
+A `ManagerProxy` proxy contract uses the `delegatecall` opcode to forward all function calls to a target implementation contract that is registered with the `Controller` with state managed by the proxy contract. Thus, the proxy contract can be upgraded by registering a new target implementation contract with the `Controller` by following the steps below.
+
+Note: The addresses of all deployed contracts can be found [here](https://docs.livepeer.org/reference/deployed-contract-addresses).
+
+## Deploy Target Implementation Contract
+
+Set the Infura API key environment variable.
+
+```
+export INFURA_KEY=<INFURA_KEY>
+```
+
+Deploy the target implementation contract by using the `--tags` flag to specify the tag associated with the relevant deploy script.
+
+```
+PRIVATE_KEY=$(cat <PATH_TO_PRIVATE_KEY_FILE>) npx hardhat deploy --tags <TAGS> --network arbitrumMainnet
+```
+
+For example, `deploy/deploy_bonding_manager.ts` is the deploy script for the `BondingManager` target implementation contract and the following command would just run that specific deploy script.
+
+```
+PRIVATE_KEY=$(cat ~/path-to-private-key-file) npx hardhat deploy --tags BONDING_MANAGER --network arbitrumMainnet
+```
+
+After deployment, a file in the `deployments` directory containing the latest addresses of deployed contracts will be updated. The JSON file for the proxy will use the name of the contract i.e. `BondingManager` proxy -> `deployments/<NETWORK>/BondingManager.json` and the target implementation will use the name of the contract with the `Target` suffix i.e. `BondingManager` target implementation -> `deployments/<NETWORK>/BondingManagerTarget.json`. By default, the proxy file i.e. `deployments/<NETWORK>/BondingManager.json` will be updated as well even if we only want to update the target implementation file i.e. `deployments/<NETWORK>/BondingManagerTarget.json` to be updated. We can omit this change from the Git history just by running `git checkout -- deployments/<NETWORK>/BondingManager.json`.
+
+## Verify Contract Code
+
+Verify the contract code on arbiscan.io.
+
+```
+npx hardhat etherscan-verify --network arbitrumMainnet --license MIT --sleep
+```
+
+The `etherscan-verify` task might return an error for certain contracts. If this happens, an alternative approach is to generate a single "flattened" (contains code from all files that the contract depends on) `.sol` file that can be manually submitted on arbiscan.io.
+
+```
+npx hardhat flatten contracts/bonding/BondingManager.sol > flattened.sol
+```
+
+You can use https://arbiscan.io/verifyContract to manually submit contract code for public verification.
+
+- The compiler config (i.e. version, optimizer runs, etc.) can be found in `hardhat.config.ts`  under `solidity.compilers`.
+- For Compiler Type, select "Solidity (Single File)".
+- For Open Source License Type, select "MIT"
+
+When prompted for the code, you can copy and paste the contents of `flattened.sol`.
+
+You can also use Tenderly to manually submit contract code for [private verification](https://docs.tenderly.co/monitoring/smart-contract-verification/verifying-a-smart-contract).
+
+If you see an error related to multiple SPDX license identifiers, remove all SPDX license identifiers from `flattened.sol` except for a single one.
+
+## View Contract Diff
+
+Use the contract diff checker at https://arbiscan.io/contractdiffchecker to view the code diff between the current and new target implementation contracts in order to check that the verified code at address of the new target implementation contract contains the expected changes.
+
+If the contract code is only privately verified in Tenderly, you can view the contract diff by copying the current target implementation contract code from arbiscan.io and the new target implementation contract code from Tenderly to local files and then running `diff current.sol new.sol`.
+
+## Create Protocol Governor Update
+
+Use [governor-scripts](https://github.com/livepeer/governor-scripts) to generate the update to be staged and executed by the protocol `Governor` that will register the target implementation contract with the `Controller`.
+
+## Run Upgrade Simulation
+
+Use [Tenderly](https://tenderly.co/) and/or Hardhat/Foundry to simulate the upgrade by creating a fork, staging/executing the protocol `Governor` update and verifying that the registration of the new target implementation contract is executed as expected.
+
+## Stage and Execute Protocol Governor Update
+
+The owner of the protocol `Governor` needs to submit a `stage()` transaction with the update and then after the update's delay is over (if the delay is 0 then the update is immediately executable) any address can submit an `execute()` transaction to execute the update to complete the registration of the new target implementation contract.
diff --git a/src/test/BondingManagerForceSelfDelegationFix.sol b/src/test/BondingManagerForceSelfDelegationFix.sol
@@ -0,0 +1,170 @@
+pragma solidity ^0.8.9;
+
+import "ds-test/test.sol";
+import "./base/GovernorBaseTest.sol";
+import "contracts/bonding/BondingManager.sol";
+import "contracts/rounds/RoundsManager.sol";
+import "contracts/token/LivepeerToken.sol";
+import "contracts/snapshots/MerkleSnapshot.sol";
+import "./interfaces/ICheatCodes.sol";
+import "./interfaces/IL2Migrator.sol";
+
+// forge test --match-contract BondingManagerForceSelfDelegationFix --fork-url https://arbitrum-mainnet.infura.io/v3/<INFURA_KEY> -vvv --fork-block-number 104182839
+contract BondingManagerForceSelfDelegationFix is GovernorBaseTest {
+    BondingManager public constant BONDING_MANAGER = BondingManager(0x35Bcf3c30594191d53231E4FF333E8A770453e40);
+    MerkleSnapshot public constant MERKLE_SNAPSHOT = MerkleSnapshot(0x10736ffaCe687658F88a46D042631d182C7757f7);
+    IL2Migrator public constant L2_MIGRATOR = IL2Migrator(0x148D5b6B4df9530c7C76A810bd1Cdf69EC4c2085);
+    RoundsManager public constant ROUNDS_MANAGER = RoundsManager(0xdd6f56DcC28D3F5f27084381fE8Df634985cc39f);
+    LivepeerToken public constant TOKEN = LivepeerToken(0x289ba1701C2F088cf0faf8B3705246331cB8A839);
+
+    address public constant MINTER = 0xc20DE37170B45774e6CD3d2304017fc962f27252;
+    address public constant L1_MIGRATOR = 0x21146B872D3A95d2cF9afeD03eE5a783DaE9A89A;
+
+    bytes32 public constant BONDING_MANAGER_TARGET_ID = keccak256("BondingManagerTarget");
+
+    BondingManager public newBondingManagerTarget;
+
+    event Bond(
+        address indexed newDelegate,
+        address indexed oldDelegate,
+        address indexed delegator,
+        uint256 additionalAmount,
+        uint256 bondedAmount
+    );
+
+    uint160 internal constant OFFSET = uint160(0x1111000000000000000000000000000000001111);
+
+    function applyL1ToL2Alias(address _l1Address) internal pure returns (address l2Address) {
+        l2Address = address(uint160(_l1Address) + OFFSET);
+    }
+
+    function setUp() public {
+        newBondingManagerTarget = new BondingManager(address(CONTROLLER));
+
+        (, gitCommitHash) = CONTROLLER.getContractInfo(BONDING_MANAGER_TARGET_ID);
+
+        stageAndExecuteOne(
+            address(CONTROLLER),
+            0,
+            abi.encodeWithSelector(
+                CONTROLLER.setContractInfo.selector,
+                BONDING_MANAGER_TARGET_ID,
+                address(newBondingManagerTarget),
+                gitCommitHash
+            )
+        );
+    }
+
+    function testUpgrade() public {
+        // Check that new BondingManagerTarget is registered
+        (address infoAddr, bytes20 infoGitCommitHash) = fetchContractInfo(BONDING_MANAGER_TARGET_ID);
+        assertEq(infoAddr, address(newBondingManagerTarget));
+        assertEq(infoGitCommitHash, gitCommitHash);
+    }
+
+    // A bondForWithHint() call from a third party that sets an unbonded delegator's delegate to self should fail after the upgrade
+    function testThirdPartyBondForWithHintInvalidDelegate() public {
+        address thirdParty = CHEATS.addr(1);
+        address delegator = CHEATS.addr(2);
+
+        CHEATS.prank(thirdParty);
+        CHEATS.expectRevert("INVALID_DELEGATE");
+        BONDING_MANAGER.bondForWithHint(1, delegator, delegator, address(0), address(0), address(0), address(0));
+    }
+
+    // A bondForWithHint() call from a third party that changes a delegator's non-null delegate should fail after the upgrade
+    function testThirdPartyBondForWithHintInvalidDelegateChange() public {
+        address thirdParty = CHEATS.addr(1);
+        // Has a non-null delegate as of fork block
+        address delegator = 0xed89FFb5F4a7460a2F9B894b494db4F5e431f842;
+        // Is a transcoder as of fork block
+        address transcoder = 0x5D98F8d269C94B746A5c3C2946634dCfc75E5E60;
+
+        CHEATS.prank(thirdParty);
+        CHEATS.expectRevert("INVALID_DELEGATE_CHANGE");
+        BONDING_MANAGER.bondForWithHint(0, delegator, transcoder, address(0), address(0), address(0), address(0));
+    }
+
+    // A transferBond() call that setes an unbonded delegator's delegate to self should fail after the upgrade
+    function testTransferBondInvalidDelegator() public {
+        address sender = CHEATS.addr(1);
+        address receiver = CHEATS.addr(2);
+
+        uint256 mockAllow = 1000;
+
+        CHEATS.startPrank(MINTER);
+        TOKEN.mint(sender, mockAllow);
+
+        CHEATS.startPrank(sender);
+        TOKEN.approve(address(BONDING_MANAGER), mockAllow);
+        BONDING_MANAGER.bond(1, receiver);
+        CHEATS.stopPrank();
+
+        // Sender needs to wait 1 round before it can call transferBond()
+        // This is the next round start block assuming 104182839 is the fork block number
+        uint256 nextRoundStartBlock = 17545330;
+        CHEATS.roll(nextRoundStartBlock);
+        ROUNDS_MANAGER.initializeRound();
+
+        CHEATS.prank(sender);
+        CHEATS.expectRevert("INVALID_DELEGATOR");
+        BONDING_MANAGER.transferBond(receiver, 1, address(0), address(0), address(0), address(0));
+    }
+
+    // A bondForWithHint() call from L2Migrator.finalizeMigrateDelegator() to set a migrating transcoder's delegate to self
+    // should still succeed after the upgrade
+    function testTranscoderFinalizeMigrateDelegator() public {
+        address transcoder = CHEATS.addr(1);
+        address l1MigratorL2Alias = applyL1ToL2Alias(L1_MIGRATOR);
+
+        uint256 stake = 500000000000000000000;
+        uint256 delegatedStake = 1000000000000000000000;
+        IL2Migrator.MigrateDelegatorParams memory params = IL2Migrator.MigrateDelegatorParams({
+            l1Addr: transcoder,
+            l2Addr: transcoder,
+            stake: stake,
+            delegatedStake: delegatedStake,
+            fees: 0,
+            delegate: transcoder
+        });
+
+        CHEATS.prank(l1MigratorL2Alias);
+        CHEATS.expectEmit(true, true, true, true);
+        emit Bond(transcoder, address(0), transcoder, stake, stake);
+        L2_MIGRATOR.finalizeMigrateDelegator(params);
+
+        (uint256 bondedAmount, , address delegateAddress, uint256 delegatedAmount, , , ) = BONDING_MANAGER.getDelegator(
+            transcoder
+        );
+        assertEq(bondedAmount, stake);
+        assertEq(delegateAddress, transcoder);
+        assertEq(delegatedAmount, delegatedStake);
+        assertTrue(BONDING_MANAGER.isRegisteredTranscoder(transcoder));
+    }
+
+    // A bondForWithHint() call from L2Migrator.claimStake() to set a migrating delegator's delegate to a transcoder
+    // should still succeed after the upgrade
+    function testDelegatorClaimStake() public {
+        address delegator = CHEATS.addr(1);
+        address delegate = CHEATS.addr(2);
+
+        // Allow arbitrary proof to pass verification in L2Migrator.claimStake()
+        CHEATS.mockCall(
+            address(MERKLE_SNAPSHOT),
+            abi.encodeWithSelector(MERKLE_SNAPSHOT.verify.selector),
+            abi.encode(true)
+        );
+
+        uint256 stake = 500000000000000000000;
+        bytes32[] memory proof;
+
+        CHEATS.prank(delegator);
+        CHEATS.expectEmit(true, true, true, true);
+        emit Bond(delegate, address(0), delegator, stake, stake);
+        L2_MIGRATOR.claimStake(delegate, stake, 0, proof, address(0));
+
+        (uint256 bondedAmount, , address delegateAddress, , , , ) = BONDING_MANAGER.getDelegator(delegator);
+        assertEq(bondedAmount, stake);
+        assertEq(delegateAddress, delegate);
+    }
+}