feat: announcePlannedUpgrade (#260)

Reviewer @rvagg
Per a discussion with @jennijuju, we want to make the upgrade process
more transparent.
This changes upgrades to a two step process.
First, we announce the planned upgrade.
Then, after the designated time, we can complete the upgrade.
The delay gives us time to check the pending implementation address. It
also gives users notice even if we are not in communication with them.
I also fix #259 in
this change, because I was already adding tests for `upgradeToAndCall`.
I can fix it separately if this change is rejected.
After validating that this is a good design that the team likes, I can
replicate it for the other proxies in separate PRs.
#### Test Plan
I tested the new deployment scripts by running them on calibnet.
*
[announcePlannedUpgrade](https://calibration.filfox.info/en/message/bafy2bzaceb6vpcsy563vhpwig5g6jy7gb6fr2ssxhjgwiagj7k3vbjrg3dg56?t=3)
*
[upgradeToAndCall](https://calibration.filfox.info/en/message/bafy2bzaceadjmlemjyrv7nmwh5lzavdbntjmywvblrzmtuh3wsyldqlvxbcyo?t=3)
#### Changes
* announcePlannedUpgrade
* update _authorizeUpgrade
* new view method for pending upgrade
* make gen
* add and fix tests covering upgradeToAndCall with migrate
* separate announce and upgrade scripts
* update implementation-only script

---------

Co-authored-by: Rod Vagg <[email protected]>

Author: wjmelements

service_contracts/abi/FilecoinWarmStorageService.abi.json

@@ -74,6 +74,31 @@
     "outputs": [],
     "stateMutability": "nonpayable"
   },
+  {
+    "type": "function",
+    "name": "announcePlannedUpgrade",
+    "inputs": [
+      {
+        "name": "plannedUpgrade",
+        "type": "tuple",
+        "internalType": "struct FilecoinWarmStorageService.PlannedUpgrade",
+        "components": [
+          {
+            "name": "nextImplementation",
+            "type": "address",
+            "internalType": "address"
+          },
+          {
+            "name": "afterEpoch",
+            "type": "uint96",
+            "internalType": "uint96"
+          }
+        ]
+      }
+    ],
+    "outputs": [],
+    "stateMutability": "nonpayable"
+  },
   {
     "type": "function",
     "name": "calculateRatesPerEpoch",
@@ -627,19 +652,6 @@
     "outputs": [],
     "stateMutability": "nonpayable"
   },
-  {
-    "type": "function",
-    "name": "serviceCommissionBps",
-    "inputs": [],
-    "outputs": [
-      {
-        "name": "",
-        "type": "uint256",
-        "internalType": "uint256"
-      }
-    ],
-    "stateMutability": "view"
-  },
   {
     "type": "function",
     "name": "serviceProviderRegistry",
@@ -1411,6 +1423,31 @@
     ],
     "anonymous": false
   },
+  {
+    "type": "event",
+    "name": "UpgradeAnnounced",
+    "inputs": [
+      {
+        "name": "plannedUpgrade",
+        "type": "tuple",
+        "indexed": false,
+        "internalType": "struct FilecoinWarmStorageService.PlannedUpgrade",
+        "components": [
+          {
+            "name": "nextImplementation",
+            "type": "address",
+            "internalType": "address"
+          },
+          {
+            "name": "afterEpoch",
+            "type": "uint96",
+            "internalType": "uint96"
+          }
+        ]
+      }
+    ],
+    "anonymous": false
+  },
   {
     "type": "event",
     "name": "Upgraded",
@@ -1994,22 +2031,6 @@
       }
     ]
   },
-  {
-    "type": "error",
-    "name": "OnlySelf",
-    "inputs": [
-      {
-        "name": "expected",
-        "type": "address",
-        "internalType": "address"
-      },
-      {
-        "name": "actual",
-        "type": "address",
-        "internalType": "address"
-      }
-    ]
-  },
   {
     "type": "error",
     "name": "OwnableInvalidOwner",

service_contracts/abi/FilecoinWarmStorageServiceStateLibrary.abi.json

@@ -588,6 +588,30 @@
     ],
     "stateMutability": "view"
   },
+  {
+    "type": "function",
+    "name": "nextUpgrade",
+    "inputs": [
+      {
+        "name": "service",
+        "type": "FilecoinWarmStorageService",
+        "internalType": "contract FilecoinWarmStorageService"
+      }
+    ],
+    "outputs": [
+      {
+        "name": "nextImplementation",
+        "type": "address",
+        "internalType": "address"
+      },
+      {
+        "name": "afterEpoch",
+        "type": "uint96",
+        "internalType": "uint96"
+      }
+    ],
+    "stateMutability": "view"
+  },
   {
     "type": "function",
     "name": "provenPeriods",
@@ -713,6 +737,25 @@
     ],
     "stateMutability": "view"
   },
+  {
+    "type": "function",
+    "name": "serviceCommissionBps",
+    "inputs": [
+      {
+        "name": "service",
+        "type": "FilecoinWarmStorageService",
+        "internalType": "contract FilecoinWarmStorageService"
+      }
+    ],
+    "outputs": [
+      {
+        "name": "",
+        "type": "uint256",
+        "internalType": "uint256"
+      }
+    ],
+    "stateMutability": "view"
+  },
   {
     "type": "error",
     "name": "ProvingPeriodNotInitialized",

service_contracts/abi/FilecoinWarmStorageServiceStateView.abi.json

@@ -509,6 +509,24 @@
     ],
     "stateMutability": "view"
   },
+  {
+    "type": "function",
+    "name": "nextUpgrade",
+    "inputs": [],
+    "outputs": [
+      {
+        "name": "nextImplementation",
+        "type": "address",
+        "internalType": "address"
+      },
+      {
+        "name": "afterEpoch",
+        "type": "uint96",
+        "internalType": "uint96"
+      }
+    ],
+    "stateMutability": "view"
+  },
   {
     "type": "function",
     "name": "provenPeriods",
@@ -622,6 +640,19 @@
     ],
     "stateMutability": "view"
   },
+  {
+    "type": "function",
+    "name": "serviceCommissionBps",
+    "inputs": [],
+    "outputs": [
+      {
+        "name": "",
+        "type": "uint256",
+        "internalType": "uint256"
+      }
+    ],
+    "stateMutability": "view"
+  },
   {
     "type": "error",
     "name": "ProvingPeriodNotInitialized",

service_contracts/src/FilecoinWarmStorageService.sol

@@ -151,6 +151,14 @@ contract FilecoinWarmStorageService is
         uint256 epochsPerMonth; // Number of epochs in a month
     }
 
+    // Used for announcing upgrades, packed into one slot
+    struct PlannedUpgrade {
+        // Address of the new implementation contract
+        address nextImplementation;
+        // Upgrade will not occur until at least this epoch
+        uint96 afterEpoch;
+    }
+
     // =========================================================================
     // Constants
 
@@ -229,7 +237,7 @@ contract FilecoinWarmStorageService is
     uint256 private challengeWindowSize;
 
     // Commission rate
-    uint256 public serviceCommissionBps;
+    uint256 private serviceCommissionBps;
 
     // Track which proving periods have valid proofs with bitmap
     mapping(uint256 dataSetId => mapping(uint256 periodId => uint256)) private provenPeriods;
@@ -266,6 +274,10 @@ contract FilecoinWarmStorageService is
     // The address allowed to terminate CDN services
     address private filBeamControllerAddress;
 
+    PlannedUpgrade private nextUpgrade;
+
+    event UpgradeAnnounced(PlannedUpgrade plannedUpgrade);
+
     // =========================================================================
 
     // Modifier to ensure only the PDP verifier contract can call certain functions
@@ -374,7 +386,19 @@ contract FilecoinWarmStorageService is
         serviceCommissionBps = 0; // 0%
     }
 
-    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {}
+    function announcePlannedUpgrade(PlannedUpgrade calldata plannedUpgrade) external onlyOwner {
+        require(plannedUpgrade.nextImplementation.code.length > 3000);
+        require(plannedUpgrade.afterEpoch > block.number);
+        nextUpgrade = plannedUpgrade;
+        emit UpgradeAnnounced(plannedUpgrade);
+    }
+
+    function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
+        // zero address already checked by ERC1967Utils._setImplementation
+        require(newImplementation == nextUpgrade.nextImplementation);
+        require(block.number >= nextUpgrade.afterEpoch);
+        delete nextUpgrade;
+    }
 
     /**
      * @notice Sets new proving period parameters
@@ -398,9 +422,7 @@ contract FilecoinWarmStorageService is
      * Only callable during proxy upgrade process
      * @param _viewContract Address of the view contract (optional, can be address(0))
      */
-    function migrate(address _viewContract) public onlyProxy reinitializer(4) {
-        require(msg.sender == address(this), Errors.OnlySelf(address(this), msg.sender));
-
+    function migrate(address _viewContract) public onlyProxy onlyOwner reinitializer(4) {
         // Set view contract if provided
         if (_viewContract != address(0)) {
             viewContractAddress = _viewContract;

service_contracts/src/FilecoinWarmStorageServiceStateView.sol

@@ -131,6 +131,10 @@ contract FilecoinWarmStorageServiceStateView is IPDPProvingSchedule {
         return service.nextPDPChallengeWindowStart(setId);
     }
 
+    function nextUpgrade() external view returns (address nextImplementation, uint96 afterEpoch) {
+        return service.nextUpgrade();
+    }
+
     function provenPeriods(uint256 dataSetId, uint256 periodId) external view returns (bool) {
         return service.provenPeriods(dataSetId, periodId);
     }
@@ -150,4 +154,8 @@ contract FilecoinWarmStorageServiceStateView is IPDPProvingSchedule {
     function railToDataSet(uint256 railId) external view returns (uint256) {
         return service.railToDataSet(railId);
     }
+
+    function serviceCommissionBps() external view returns (uint256) {
+        return service.serviceCommissionBps();
+    }
 }

service_contracts/src/lib/FilecoinWarmStorageServiceLayout.sol

@@ -24,3 +24,4 @@ bytes32 constant APPROVED_PROVIDERS_SLOT = bytes32(uint256(15));
 bytes32 constant APPROVED_PROVIDER_IDS_SLOT = bytes32(uint256(16));
 bytes32 constant VIEW_CONTRACT_ADDRESS_SLOT = bytes32(uint256(17));
 bytes32 constant FIL_BEAM_CONTROLLER_ADDRESS_SLOT = bytes32(uint256(18));
+bytes32 constant NEXT_UPGRADE_SLOT = bytes32(uint256(19));

service_contracts/src/lib/FilecoinWarmStorageServiceStateInternalLibrary.sol

@@ -203,6 +203,10 @@ library FilecoinWarmStorageServiceStateInternalLibrary {
         initChallengeWindowStart = block.number + maxProvingPeriod - challengeWindowSize;
     }
 
+    function serviceCommissionBps(FilecoinWarmStorageService service) internal view returns (uint256) {
+        return uint256(service.extsload(StorageLayout.SERVICE_COMMISSION_BPS_SLOT));
+    }
+
     /**
      * @notice Returns the start of the next challenge window for a data set
      * @param service The service contract
@@ -490,4 +494,20 @@ library FilecoinWarmStorageServiceStateInternalLibrary {
     function filBeamControllerAddress(FilecoinWarmStorageService service) internal view returns (address) {
         return address(uint160(uint256(service.extsload(StorageLayout.FIL_BEAM_CONTROLLER_ADDRESS_SLOT))));
     }
+
+    /**
+     * @notice Get information about the next contract upgrade
+     * @param service The service contract
+     * @return nextImplementation The next code for the contract
+     * @return afterEpoch The earliest the upgrade may complete
+     */
+    function nextUpgrade(FilecoinWarmStorageService service)
+        internal
+        view
+        returns (address nextImplementation, uint96 afterEpoch)
+    {
+        bytes32 upgradeInfo = service.extsload(StorageLayout.NEXT_UPGRADE_SLOT);
+        nextImplementation = address(uint160(uint256(upgradeInfo)));
+        afterEpoch = uint96(uint256(upgradeInfo) >> 160);
+    }
 }

service_contracts/src/lib/FilecoinWarmStorageServiceStateLibrary.sol

@@ -199,6 +199,10 @@ library FilecoinWarmStorageServiceStateLibrary {
         initChallengeWindowStart = block.number + maxProvingPeriod - challengeWindowSize;
     }
 
+    function serviceCommissionBps(FilecoinWarmStorageService service) public view returns (uint256) {
+        return uint256(service.extsload(StorageLayout.SERVICE_COMMISSION_BPS_SLOT));
+    }
+
     /**
      * @notice Returns the start of the next challenge window for a data set
      * @param service The service contract
@@ -486,4 +490,20 @@ library FilecoinWarmStorageServiceStateLibrary {
     function filBeamControllerAddress(FilecoinWarmStorageService service) public view returns (address) {
         return address(uint160(uint256(service.extsload(StorageLayout.FIL_BEAM_CONTROLLER_ADDRESS_SLOT))));
     }
+
+    /**
+     * @notice Get information about the next contract upgrade
+     * @param service The service contract
+     * @return nextImplementation The next code for the contract
+     * @return afterEpoch The earliest the upgrade may complete
+     */
+    function nextUpgrade(FilecoinWarmStorageService service)
+        public
+        view
+        returns (address nextImplementation, uint96 afterEpoch)
+    {
+        bytes32 upgradeInfo = service.extsload(StorageLayout.NEXT_UPGRADE_SLOT);
+        nextImplementation = address(uint160(uint256(upgradeInfo)));
+        afterEpoch = uint96(uint256(upgradeInfo) >> 160);
+    }
 }