Add fix tool to upgrade for reverse pubkey mapping and include tests

contracts/contract/minipool/RocketMinipoolManager.sol

@@ -454,9 +454,12 @@ contract RocketMinipoolManager is RocketBase, RocketMinipoolManagerInterface {
         // Remove from vacant set
         AddressSetStorageInterface addressSetStorage = AddressSetStorageInterface(getContractAddress("addressSetStorage"));
         addressSetStorage.removeItem(keccak256(abi.encodePacked("minipools.vacant.index")), msg.sender);
-        // Remove mapping of pubkey to minipool to allow NO to try again in future
-        bytes memory pubkey = getMinipoolPubkey(msg.sender);
-        deleteAddress(keccak256(abi.encodePacked("validator.minipool", pubkey)));
+        // If minipool was dissolved, remove mapping of pubkey to minipool to allow NO to try again in future
+        RocketMinipoolInterface minipool = RocketMinipoolInterface(msg.sender);
+        if (minipool.getStatus() == MinipoolStatus.Dissolved) {
+            bytes memory pubkey = getMinipoolPubkey(msg.sender);
+            deleteAddress(keccak256(abi.encodePacked("validator.minipool", pubkey)));
+        }
     }
 
     /// @notice Returns the number of minipools in the vacant minipool set

contracts/contract/upgrade/RocketUpgradeOneDotThree.sol

@@ -6,6 +6,7 @@ import "../../interface/network/RocketNetworkSnapshotsInterface.sol";
 import "../../interface/network/RocketNetworkPricesInterface.sol";
 import "../../interface/dao/protocol/settings/RocketDAOProtocolSettingsNodeInterface.sol";
 import "../../interface/util/AddressSetStorageInterface.sol";
+import "../../interface/minipool/RocketMinipoolManagerInterface.sol";
 
 /// @notice Transient contract to upgrade Rocket Pool with the Houston set of contract upgrades
 contract RocketUpgradeOneDotThree is RocketBase {
@@ -212,6 +213,7 @@ contract RocketUpgradeOneDotThree is RocketBase {
         setUint(keccak256(abi.encodePacked(settingNameSpace, "proposal.quorum")), 0.51 ether);            // The quorum required to pass a proposal
         setUint(keccak256(abi.encodePacked(settingNameSpace, "proposal.veto.quorum")), 0.51 ether);       // The quorum required to veto a proposal
         setUint(keccak256(abi.encodePacked(settingNameSpace, "proposal.max.block.age")), 1024);           // The maximum age of a block a proposal can be raised at
+        setBool(keccak256(abi.encodePacked(settingNameSpace, "deployed")), true);
 
         // pDAO network settings
         settingNameSpace = keccak256(abi.encodePacked("dao.protocol.setting.", "network"));
@@ -229,6 +231,7 @@ contract RocketUpgradeOneDotThree is RocketBase {
         setUint(keccak256(abi.encodePacked(settingNameSpace, "proposal.vote.time")), 2 weeks);      // How long a proposal can be voted on
         setUint(keccak256(abi.encodePacked(settingNameSpace, "proposal.execute.time")), 4 weeks);   // How long a proposal can be executed after its voting period is finished
         setUint(keccak256(abi.encodePacked(settingNameSpace, "proposal.action.time")), 4 weeks);    // Certain proposals require a secondary action to be run after the proposal is successful (joining, leaving etc). This is how long until that action expires
+        setBool(keccak256(abi.encodePacked(settingNameSpace, "deployed")), true);
 
         // Default permissions for security council
         setBool(keccak256(abi.encodePacked("dao.security.allowed.setting", "deposit", "deposit.enabled")), true);
@@ -261,6 +264,21 @@ contract RocketUpgradeOneDotThree is RocketBase {
         setString(keccak256(abi.encodePacked("protocol.version")), "1.3.0");
     }
 
+    /// @notice Used to fix incorrect mapping of pubkey to minipool caused by a bug in previous release
+    function fixPubkeys(address[] calldata _minipoolAddresses) external onlyGuardian {
+        for (uint256 i = 0; i < _minipoolAddresses.length; ++i) {
+            address minipoolAddress = _minipoolAddresses[i];
+            // Require minipool exists
+            require(getBool(keccak256(abi.encodePacked("minipool.exists", minipoolAddress))), "Minipool does not exist");
+            // Check the minipool hasn't been dissolved as this could prevent a failed vacant minipool from trying again
+            RocketMinipoolInterface minipool = RocketMinipoolInterface(minipoolAddress);
+            require(minipool.getStatus() != MinipoolStatus.Dissolved, "Minipool was dissolved");
+            // Update reverse lookup
+            bytes memory pubkey = getBytes(keccak256(abi.encodePacked("minipool.pubkey", minipoolAddress)));
+            setAddress(keccak256(abi.encodePacked("validator.minipool", pubkey)), minipoolAddress);
+        }
+    }
+
     /// @dev Add a new network contract
     function _addContract(string memory _name, address _contractAddress, string memory _contractAbi) internal {
         // Check contract name

test/_helpers/minipool.js

@@ -242,6 +242,13 @@ export async function stakeMinipool(minipool, txOptions) {
 // Promote a minipool to staking
 export async function promoteMinipool(minipool, txOptions) {
     await minipool.promote(txOptions);
+    if (await upgradeExecuted()) {
+        // Expect pubkey -> minipool mapping still exists
+        const rocketMinipoolManager = await RocketMinipoolManager.deployed();
+        const actualPubKey = await rocketMinipoolManager.getMinipoolPubkey(minipool.address);
+        const reverseAddress = await rocketMinipoolManager.getMinipoolByPubkey(actualPubKey);
+        assert.equal(reverseAddress, minipool.address);
+    }
 }
 
 

test/dao/dao-node-trusted-tests.js

@@ -11,6 +11,7 @@ import { daoNodeTrustedExecute, getDAOMemberIsValid, daoNodeTrustedPropose, daoN
 import { proposalStates, getDAOProposalState, getDAOProposalStartTime, getDAOProposalEndTime, getDAOProposalExpires } from './scenario-dao-proposal';
 import { assertBN } from '../_helpers/bn';
 import { RocketDAONodeTrusted, RocketDAONodeTrustedActions, RocketDAONodeTrustedSettingsMembers, RocketDAONodeTrustedSettingsProposals, RocketTokenRPL, RocketMinipoolManager, RocketDAONodeTrustedUpgrade, RocketStorage } from '../_utils/artifacts';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 
 export default function() {
@@ -69,6 +70,9 @@ export default function() {
         let rocketDAONodeTrustedUpgradeNew;
 
         before(async () => {
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
             // Load contracts
             // Get RocketStorage
             const rocketStorage = await RocketStorage.deployed();

test/dao/dao-security-tests.js

@@ -39,6 +39,7 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             await userDeposit({ from: random, value: '320'.ether });

test/deposit/deposit-pool-tests.js

@@ -15,6 +15,7 @@ import { setDAOProtocolBootstrapSetting } from '../dao/scenario-dao-protocol-boo
 import { setDAONodeTrustedBootstrapSetting } from '../dao/scenario-dao-node-trusted-bootstrap'
 import { assignDepositsV2 } from './scenario-assign-deposits-v2';
 import { assertBN } from '../_helpers/bn';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketDepositPool', async (accounts) => {
@@ -32,6 +33,9 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
             // Register node
             await registerNode({from: node});
 

test/minipool/minipool-scrub-tests.js

@@ -45,6 +45,7 @@ export default function() {
         let prelaunchMinipool;
 
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             // Register node & set withdrawal address

test/minipool/minipool-status-tests.js

@@ -11,6 +11,7 @@ import { daoNodeTrustedExecute, daoNodeTrustedMemberLeave, daoNodeTrustedPropose
 import { getDAOProposalEndTime, getDAOProposalStartTime } from '../dao/scenario-dao-proposal'
 import { setDAONodeTrustedBootstrapSetting } from '../dao/scenario-dao-node-trusted-bootstrap'
 import { assertBN } from '../_helpers/bn';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketMinipoolStatus', async (accounts) => {
@@ -39,6 +40,9 @@ export default function() {
         let stakingMinipool2;
         let stakingMinipool3;
         before(async () => {
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
             // Register node
             await registerNode({from: node});
 

test/minipool/minipool-tests.js

@@ -84,6 +84,7 @@ export default function() {
         const halfDepositNodeAmount = '16'.ether;
 
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             oldDelegateAddress = (await RocketMinipoolDelegate.deployed()).address;

test/minipool/minipool-vacant-tests.js

@@ -1,7 +1,7 @@
 import {
     RocketDAOProtocolSettingsMinipool,
     RocketDAOProtocolSettingsNetwork,
-    RocketDAONodeTrustedSettingsMinipool, RocketNodeStaking,
+    RocketDAONodeTrustedSettingsMinipool, RocketNodeStaking, RocketNodeStakingNew,
 } from '../_utils/artifacts';
 import { increaseTime } from '../_utils/evm';
 import { printTitle } from '../_utils/formatting';
@@ -26,6 +26,7 @@ import { voteScrub } from './scenario-scrub';
 import { assertBN } from '../_helpers/bn';
 import { refund } from './scenario-refund';
 import { getValidatorPubkey } from '../_utils/beacon';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketMinipool', async (accounts) => {
@@ -53,7 +54,10 @@ export default function() {
         let rocketNodeStaking;
 
         before(async () => {
-            rocketNodeStaking = await RocketNodeStaking.deployed();
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
+            rocketNodeStaking = await RocketNodeStakingNew.deployed();
 
             // Register node & set withdrawal address
             await registerNode({from: node});

test/minipool/minipool-withdrawal-tests.js

@@ -21,6 +21,7 @@ import {
 } from '../dao/scenario-dao-node-trusted-bootstrap';
 import { submitPrices } from '../_helpers/network';
 import { assertBN } from '../_helpers/bn';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketMinipool', async (accounts) => {
@@ -46,6 +47,15 @@ export default function() {
         let userDistributeLength = (60 * 60);
 
         before(async () => {
+            // Hard code fee to 50%
+            const fee = '0.5'.ether;
+            await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.minimum', fee, {from: owner});
+            await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.target', fee, {from: owner});
+            await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.maximum', fee, {from: owner});
+
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
             // Register node & set withdrawal address
             await registerNode({from: node});
             await setNodeWithdrawalAddress(node, nodeWithdrawalAddress, {from: node});
@@ -80,12 +90,6 @@ export default function() {
             const rocketMinipoolPenalty = await RocketMinipoolPenalty.deployed();
             await rocketMinipoolPenalty.setMaxPenaltyRate(maxPenaltyRate, {from: owner})
 
-            // Hard code fee to 50%
-            const fee = '0.5'.ether;
-            await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.minimum', fee, {from: owner});
-            await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.target', fee, {from: owner});
-            await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.maximum', fee, {from: owner});
-
             // Deposit some user funds to assign to pools
             let userDepositAmount = '16'.ether;
             await userDeposit({from: random, value: userDepositAmount});

test/minipool/scenario-scrub.js

@@ -1,8 +1,13 @@
 // Dissolve a minipool
 import {
     RocketDAONodeTrusted,
-    RocketDAONodeTrustedSettingsMinipool, RocketDAOProtocolSettingsNode, RocketNetworkPrices,
-    RocketNodeStaking, RocketNodeStakingNew,
+    RocketDAONodeTrustedSettingsMinipool,
+    RocketDAOProtocolSettingsNode,
+    RocketMinipoolManager,
+    RocketMinipoolManagerNew,
+    RocketNetworkPrices,
+    RocketNodeStaking,
+    RocketNodeStakingNew,
     RocketTokenRPL,
     RocketVault,
 } from '../_utils/artifacts';
@@ -32,7 +37,7 @@ export async function voteScrub(minipool, txOptions) {
             rocketNodeStaking.getNodeRPLStake.call(nodeAddress),
             rocketVault.balanceOfToken('rocketAuctionManager', rocketTokenRPL.address),
             rocketDAONodeTrustedSettingsMinipool.getScrubPenaltyEnabled(),
-            minipool.getVacant.call()
+            minipool.getVacant.call(),
         ]).then(
             ([status, userDepositBalance, votes, nodeRPLStake, auctionBalance, penaltyEnabled, vacant]) =>
             ({status, userDepositBalance, votes, nodeRPLStake, auctionBalance, penaltyEnabled, vacant})
@@ -71,6 +76,13 @@ export async function voteScrub(minipool, txOptions) {
             assertBN.equal(slashAmountEth, expectedSlash, 'Amount of RPL slashed is incorrect');
             assertBN.equal(details2.auctionBalance.sub(details1.auctionBalance), slashAmount, 'RPL was not sent to auction manager');
         }
+        if (details1.vacant) {
+            // Expect pubkey -> minipool mapping to be removed
+            const rocketMinipoolManager = await RocketMinipoolManager.deployed();
+            const actualPubKey = await rocketMinipoolManager.getMinipoolPubkey(minipool.address);
+            const reverseAddress = await rocketMinipoolManager.getMinipoolByPubkey(actualPubKey);
+            assert.equal(reverseAddress, "0x0000000000000000000000000000000000000000");
+        }
     } else {
         assertBN.equal(details2.votes.sub(details1.votes), 1, 'Vote count not incremented');
         assertBN.notEqual(details2.status, minipoolStates.Dissolved, 'Incorrect updated minipool status');

test/network/network-balances-tests.js

@@ -33,6 +33,7 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             // Register node

test/network/network-fees-tests.js

@@ -3,6 +3,7 @@ import { getNodeFeeByDemand } from '../_helpers/network';
 import { RocketDAOProtocolSettingsNetwork } from '../_utils/artifacts';
 import { setDAOProtocolBootstrapSetting } from '../dao/scenario-dao-protocol-bootstrap';
 import { assertBN } from '../_helpers/bn';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketNetworkFees', async (accounts) => {
@@ -26,6 +27,9 @@ export default function() {
             await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.target', targetNodeFee, {from: owner});
             await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.maximum', maxNodeFee, {from: owner});
             await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNetwork, 'network.node.fee.demand.range', demandRange, {from: owner});
+
+            // Upgrade to Houston
+            await upgradeOneDotThree();
         });
 
 

test/network/network-penalties-tests.js

@@ -10,6 +10,7 @@ import { createMinipool, getMinipoolMinimumRPLStake } from '../_helpers/minipool
 import { submitPenalty } from './scenario-submit-penalties';
 import { mintRPL } from '../_helpers/tokens';
 import { userDeposit } from '../_helpers/deposit';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketNetworkPenalties', async (accounts) => {
@@ -35,6 +36,9 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
             // Register node
             await registerNode({from: node});
 

test/network/network-prices-tests.js

@@ -37,6 +37,7 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             // Register node

test/network/network-snapshots-tests.js

@@ -23,6 +23,7 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             // Add penalty helper contract

test/network/network-voting-tests.js

@@ -30,6 +30,7 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             // Get contracts

test/node/node-deposit-tests.js

@@ -39,6 +39,7 @@ export default function() {
         let halfDepositNodeAmount;
 
         before(async () => {
+            // Upgrade to Houston
             await upgradeOneDotThree();
 
             // Set settings

test/node/node-distributor-tests.js

@@ -2,7 +2,7 @@ import { printTitle } from '../_utils/formatting';
 import {
     RocketNodeManager,
     RocketDAONodeTrustedSettingsMinipool,
-    RocketNodeDistributorFactory
+    RocketNodeDistributorFactory, RocketNodeManagerNew,
 } from '../_utils/artifacts';
 import {
     createMinipool,
@@ -16,6 +16,7 @@ import { increaseTime } from '../_utils/evm';
 import { setDAONodeTrustedBootstrapSetting } from '../dao/scenario-dao-node-trusted-bootstrap';
 import { shouldRevert } from '../_utils/testing';
 import { userDeposit } from '../_helpers/deposit';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 export default function() {
     contract('RocketNodeDistributor', async (accounts) => {
@@ -36,6 +37,8 @@ export default function() {
         let rplStake;
 
         before(async () => {
+            // Upgrade to Houston
+            await upgradeOneDotThree();
             // Get contracts
             const rocketNodeDistributorFactory = await RocketNodeDistributorFactory.deployed();
             // Set settings
@@ -60,15 +63,15 @@ export default function() {
             await registerNode({from: node2});
             await nodeStakeRPL(rplStake, {from: node2});
             // Get contracts
-            const rocketNodeManager = await RocketNodeManager.deployed();
+            const rocketNodeManager = await RocketNodeManagerNew.deployed();
             // Attempt to initialise
             await shouldRevert(rocketNodeManager.initialiseFeeDistributor({from: node2}), 'Was able to initialise again', 'Already initialised');
         });
 
 
         it(printTitle('node operator', 'can not initialise fee distributor if already initialised'), async () => {
             // Attempt to initialise a second time
-            const rocketNodeManager = await RocketNodeManager.deployed();
+            const rocketNodeManager = await RocketNodeManagerNew.deployed();
             await shouldRevert(rocketNodeManager.initialiseFeeDistributor({from: node1}), 'Was able to initialise again', 'Already initialised');
         });
 

test/node/node-manager-tests.js

@@ -8,6 +8,7 @@ import { setTimezoneLocation } from './scenario-set-timezone';
 import { setWithdrawalAddress, confirmWithdrawalAddress } from './scenario-set-withdrawal-address';
 import { setSmoothingPoolRegistrationState } from './scenario-register-smoothing-pool';
 import { increaseTime } from '../_utils/evm';
+import { upgradeOneDotThree } from '../_utils/upgrade';
 
 
 export default function() {
@@ -35,6 +36,9 @@ export default function() {
 
         // Setup
         before(async () => {
+            // Upgrade to Houston
+            await upgradeOneDotThree();
+
             // Enable smoothing pool registrations
             await setDAOProtocolBootstrapSetting(RocketDAOProtocolSettingsNode, 'node.smoothing.pool.registration.enabled', true, {from: owner});