Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 50 additions & 29 deletions service_contracts/abi/FilecoinWarmStorageService.abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -627,19 +652,6 @@
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "serviceCommissionBps",
"inputs": [],
"outputs": [
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "view"
},
{
"type": "function",
"name": "serviceProviderRegistry",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
31 changes: 31 additions & 0 deletions service_contracts/abi/FilecoinWarmStorageServiceStateView.abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -622,6 +640,19 @@
],
"stateMutability": "view"
},
{
"type": "function",
"name": "serviceCommissionBps",
"inputs": [],
"outputs": [
{
"name": "",
"type": "uint256",
"internalType": "uint256"
}
],
"stateMutability": "view"
},
{
"type": "error",
"name": "ProvingPeriodNotInitialized",
Expand Down
32 changes: 27 additions & 5 deletions service_contracts/src/FilecoinWarmStorageService.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also require a minimum delay, but in the event of an emergency we might want a shorter delay. I can add it if someone else wants it. Mainly if we discovered a problem right after a deployment a mandatory delay would become mandatory downtime.

Such a minimum delay would help facilitate security by allowing time to check, for example, that the bytecode matches the source code. It would enforce transparency by mandating a notice period for service providers, who can watch for the UpgradeAnnounced event log.

However such a delay could also just be enforced off-chain by best-practices. This is why I don't enforce it in the code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

nextUpgrade = plannedUpgrade;
emit UpgradeAnnounced(plannedUpgrade);
}

function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
// zero address already checked by ERC1967Utils._setImplementation
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also test for this case in testUpgrade.

        (address nextImplementation, uint96 afterEpoch) = viewContract.nextUpgrade();
        assertEq(nextImplementation, address(0));
        assertEq(afterEpoch, uint96(0));

        // Do not allow upgrade to zero address even if it's the nextImplementation
        vm.expectRevert();
        service.upgradeToAndCall(nextImplementation, migrateData);

require(newImplementation == nextUpgrade.nextImplementation);
require(block.number >= nextUpgrade.afterEpoch);
delete nextUpgrade;
}

/**
* @notice Sets new proving period parameters
Expand All @@ -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));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for #259


function migrate(address _viewContract) public onlyProxy onlyOwner reinitializer(4) {
// Set view contract if provided
if (_viewContract != address(0)) {
viewContractAddress = _viewContract;
Expand Down
8 changes: 8 additions & 0 deletions service_contracts/src/FilecoinWarmStorageServiceStateView.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}
}
Loading
Loading