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
68 changes: 68 additions & 0 deletions service_contracts/abi/ServiceProviderRegistry.abi.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,31 @@
],
"stateMutability": "view"
},
{
"type": "function",
"name": "announcePlannedUpgrade",
"inputs": [
{
"name": "plannedUpgrade",
"type": "tuple",
"internalType": "struct ServiceProviderRegistry.PlannedUpgrade",
"components": [
{
"name": "nextImplementation",
"type": "address",
"internalType": "address"
},
{
"name": "afterEpoch",
"type": "uint96",
"internalType": "uint96"
}
]
}
],
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "eip712Domain",
Expand Down Expand Up @@ -794,6 +819,24 @@
"outputs": [],
"stateMutability": "nonpayable"
},
{
"type": "function",
"name": "nextUpgrade",
"inputs": [],
"outputs": [
{
"name": "nextImplementation",
"type": "address",
"internalType": "address"
},
{
"name": "afterEpoch",
"type": "uint96",
"internalType": "uint96"
}
],
"stateMutability": "view"
},
{
"type": "function",
"name": "owner",
Expand Down Expand Up @@ -1304,6 +1347,31 @@
],
"anonymous": false
},
{
"type": "event",
"name": "UpgradeAnnounced",
"inputs": [
{
"name": "plannedUpgrade",
"type": "tuple",
"indexed": false,
"internalType": "struct ServiceProviderRegistry.PlannedUpgrade",
"components": [
{
"name": "nextImplementation",
"type": "address",
"internalType": "address"
},
{
"name": "afterEpoch",
"type": "uint96",
"internalType": "uint96"
}
]
}
],
"anonymous": false
},
{
"type": "event",
"name": "Upgraded",
Expand Down
32 changes: 29 additions & 3 deletions service_contracts/src/ServiceProviderRegistry.sol
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,18 @@ contract ServiceProviderRegistry is
/// @notice Emitted when the contract is upgraded
event ContractUpgraded(string version, address implementation);

// 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;
}

PlannedUpgrade public nextUpgrade;

event UpgradeAnnounced(PlannedUpgrade plannedUpgrade);

/// @notice Ensures the caller is the service provider
modifier onlyServiceProvider(uint256 providerId) {
require(providers[providerId].serviceProvider == msg.sender, "Only service provider can call this function");
Expand Down Expand Up @@ -752,18 +764,32 @@ contract ServiceProviderRegistry is
}
}

/// @notice Announce a planned upgrade
/// @dev Can only be called by the contract owner
/// @param plannedUpgrade The planned upgrade details
function announcePlannedUpgrade(PlannedUpgrade calldata plannedUpgrade) external onlyOwner {
require(plannedUpgrade.nextImplementation.code.length > 3000);
require(plannedUpgrade.afterEpoch > block.number);
nextUpgrade = plannedUpgrade;
emit UpgradeAnnounced(plannedUpgrade);
}

/// @notice Authorizes an upgrade to a new implementation
/// @dev Can only be called by the contract owner
/// @dev Supports both one-step (legacy) and two-step (announcePlannedUpgrade) upgrade mechanisms
/// @param newImplementation Address of the new implementation contract
function _authorizeUpgrade(address newImplementation) internal override onlyOwner {
// Authorization logic is handled by the onlyOwner modifier
// zero address already checked by ERC1967Utils._setImplementation
require(newImplementation == nextUpgrade.nextImplementation);
require(block.number >= nextUpgrade.afterEpoch);
delete nextUpgrade;
}

/// @notice Migration function for contract upgrades
/// @dev This function should be called during upgrades to emit version tracking events
/// Only callable during proxy upgrade process
/// @param newVersion The version string for the new implementation
function migrate(string memory newVersion) public onlyProxy reinitializer(2) {
require(msg.sender == address(this), "Only self can call migrate");
function migrate(string memory newVersion) public onlyProxy onlyOwner reinitializer(2) {
emit ContractUpgraded(newVersion, ERC1967Utils.getImplementation());
}
}
109 changes: 106 additions & 3 deletions service_contracts/test/ServiceProviderRegistry.t.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
pragma solidity ^0.8.20;

import {MockFVMTest} from "@fvm-solidity/mocks/MockFVMTest.sol";
import {Vm} from "forge-std/Test.sol";
import {ERC1967Proxy} from "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
import {IERC20} from "@openzeppelin/contracts/token/ERC20/IERC20.sol";

Expand Down Expand Up @@ -52,6 +53,99 @@ contract ServiceProviderRegistryTest is MockFVMTest {
registry.initialize();
}

function testAnnouncePlannedUpgrade() public {
// Initially, no upgrade is planned
(address nextImplementation, uint96 afterEpoch) = registry.nextUpgrade();
assertEq(nextImplementation, address(0));
assertEq(afterEpoch, uint96(0));

// Deploy new implementation
ServiceProviderRegistry newImplementation = new ServiceProviderRegistry();

// Announce upgrade
ServiceProviderRegistry.PlannedUpgrade memory plan;
plan.nextImplementation = address(newImplementation);
plan.afterEpoch = uint96(vm.getBlockNumber()) + 2000;

vm.expectEmit(false, false, false, true);
emit ServiceProviderRegistry.UpgradeAnnounced(plan);
registry.announcePlannedUpgrade(plan);

// Verify upgrade plan is stored
(nextImplementation, afterEpoch) = registry.nextUpgrade();
assertEq(nextImplementation, plan.nextImplementation);
assertEq(afterEpoch, plan.afterEpoch);

// Cannot upgrade before afterEpoch
bytes memory migrateData =
abi.encodeWithSelector(ServiceProviderRegistry.migrate.selector, newImplementation.VERSION());
vm.expectRevert();
registry.upgradeToAndCall(plan.nextImplementation, migrateData);

// Still cannot upgrade at afterEpoch - 1
vm.roll(plan.afterEpoch - 1);
vm.expectRevert();
registry.upgradeToAndCall(plan.nextImplementation, migrateData);

// Can upgrade at afterEpoch
vm.roll(plan.afterEpoch);
// Note: reinitializer(2) emits Initialized event first, then ContractUpgraded
// We use recordLogs to capture all events and verify ContractUpgraded is present
vm.recordLogs();
registry.upgradeToAndCall(plan.nextImplementation, migrateData);

// Verify ContractUpgraded event was emitted
Vm.Log[] memory logs = vm.getRecordedLogs();
bytes32 expectedTopic = keccak256("ContractUpgraded(string,address)");
bool foundEvent = false;
for (uint256 i = 0; i < logs.length; i++) {
if (logs[i].topics[0] == expectedTopic) {
(string memory version, address impl) = abi.decode(logs[i].data, (string, address));
assertEq(version, newImplementation.VERSION(), "Version should match");
assertEq(impl, plan.nextImplementation, "Implementation should match");
foundEvent = true;
break;
}
}
assertTrue(foundEvent, "ContractUpgraded event should be emitted");

// After upgrade, nextUpgrade should be cleared
(nextImplementation, afterEpoch) = registry.nextUpgrade();
assertEq(nextImplementation, address(0));
assertEq(afterEpoch, uint96(0));
}

function testAnnouncePlannedUpgradeOnlyOwner() public {
ServiceProviderRegistry newImplementation = new ServiceProviderRegistry();
ServiceProviderRegistry.PlannedUpgrade memory plan;
plan.nextImplementation = address(newImplementation);
plan.afterEpoch = uint96(vm.getBlockNumber()) + 2000;

// Non-owner cannot announce upgrade
vm.prank(user1);
vm.expectRevert();
registry.announcePlannedUpgrade(plan);
}

function testAnnouncePlannedUpgradeInvalidImplementation() public {
ServiceProviderRegistry.PlannedUpgrade memory plan;
plan.nextImplementation = address(0x123); // Invalid address with no code
plan.afterEpoch = uint96(vm.getBlockNumber()) + 2000;

vm.expectRevert();
registry.announcePlannedUpgrade(plan);
}

function testAnnouncePlannedUpgradeInvalidEpoch() public {
ServiceProviderRegistry newImplementation = new ServiceProviderRegistry();
ServiceProviderRegistry.PlannedUpgrade memory plan;
plan.nextImplementation = address(newImplementation);
plan.afterEpoch = uint96(vm.getBlockNumber()); // Must be in the future

vm.expectRevert();
registry.announcePlannedUpgrade(plan);
}

function testIsRegisteredProviderReturnsFalse() public view {
// Should return false for unregistered addresses
assertFalse(registry.isRegisteredProvider(user1), "Should return false for unregistered address");
Expand Down Expand Up @@ -241,13 +335,22 @@ contract ServiceProviderRegistryTest is MockFVMTest {
// Deploy new implementation
ServiceProviderRegistry newImplementation = new ServiceProviderRegistry();

// Non-owner cannot upgrade
// Non-owner cannot upgrade (will fail in _authorizeUpgrade due to onlyOwner)
vm.prank(user1);
vm.expectRevert();
registry.upgradeToAndCall(address(newImplementation), "");

// Owner can upgrade
registry.upgradeToAndCall(address(newImplementation), "");
// Owner can upgrade (but needs to announce first or it will fail in _authorizeUpgrade)
// Since we're testing the onlyOwner check, we need to announce the upgrade first
ServiceProviderRegistry.PlannedUpgrade memory plan;
plan.nextImplementation = address(newImplementation);
plan.afterEpoch = uint96(vm.getBlockNumber()) + 1;
registry.announcePlannedUpgrade(plan);

vm.roll(plan.afterEpoch);
bytes memory migrateData =
abi.encodeWithSelector(ServiceProviderRegistry.migrate.selector, newImplementation.VERSION());
registry.upgradeToAndCall(address(newImplementation), migrateData);
}

function testTransferOwnership() public {
Expand Down
77 changes: 77 additions & 0 deletions service_contracts/tools/announce-planned-upgrade-registry.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
#!/bin/bash

# announce-planned-upgrade-registry.sh: Announces a planned upgrade for ServiceProviderRegistry
# Required args: ETH_RPC_URL, REGISTRY_PROXY_ADDRESS, ETH_KEYSTORE, PASSWORD, NEW_REGISTRY_IMPLEMENTATION_ADDRESS, AFTER_EPOCH

if [ -z "$ETH_RPC_URL" ]; then
echo "Error: ETH_RPC_URL is not set"
exit 1
fi

if [ -z "$ETH_KEYSTORE" ]; then
echo "Error: ETH_KEYSTORE is not set"
exit 1
fi

if [ -z "$PASSWORD" ]; then
echo "Error: PASSWORD is not set"
exit 1
fi

if [ -z "$CHAIN" ]; then
CHAIN=$(cast chain-id)
if [ -z "$CHAIN" ]; then
echo "Error: Failed to detect chain ID from RPC"
exit 1
fi
fi

if [ -z "$NEW_REGISTRY_IMPLEMENTATION_ADDRESS" ]; then
echo "NEW_REGISTRY_IMPLEMENTATION_ADDRESS is not set"
exit 1
fi

if [ -z "$AFTER_EPOCH" ]; then
echo "AFTER_EPOCH is not set"
exit 1
fi

CURRENT_EPOCH=$(cast block-number 2>/dev/null)

if [ "$CURRENT_EPOCH" -gt "$AFTER_EPOCH" ]; then
echo "Already past AFTER_EPOCH ($CURRENT_EPOCH > $AFTER_EPOCH)"
exit 1
else
echo "Announcing planned upgrade after $(($AFTER_EPOCH - $CURRENT_EPOCH)) epochs"
fi


ADDR=$(cast wallet address --password "$PASSWORD")
echo "Sending announcement from owner address: $ADDR"

# Get current nonce
NONCE=$(cast nonce "$ADDR")

if [ -z "$REGISTRY_PROXY_ADDRESS" ]; then
echo "Error: REGISTRY_PROXY_ADDRESS is not set"
exit 1
fi

PROXY_OWNER=$(cast call -f 0x0000000000000000000000000000000000000000 "$REGISTRY_PROXY_ADDRESS" "owner()(address)" 2>/dev/null)
if [ "$PROXY_OWNER" != "$ADDR" ]; then
echo "Supplied ETH_KEYSTORE ($ADDR) is not the proxy owner ($PROXY_OWNER)."
exit 1
fi

TX_HASH=$(cast send "$REGISTRY_PROXY_ADDRESS" "announcePlannedUpgrade((address,uint96))" "($NEW_REGISTRY_IMPLEMENTATION_ADDRESS,$AFTER_EPOCH)" \
--password "$PASSWORD" \
--nonce "$NONCE" \
--json | jq -r '.transactionHash')

if [ -z "$TX_HASH" ]; then
echo "Error: Failed to send announcePlannedUpgrade transaction"
exit 1
fi

echo "announcePlannedUpgrade transaction sent: $TX_HASH"

1 change: 1 addition & 0 deletions service_contracts/tools/announce-planned-upgrade.sh
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ TX_HASH=$(cast send "$WARM_STORAGE_PROXY_ADDRESS" "announcePlannedUpgrade((addre

if [ -z "$TX_HASH" ]; then
echo "Error: Failed to send announcePlannedUpgrade transaction"
exit 1
fi

echo "announcePlannedUpgrade transaction sent: $TX_HASH"
Loading
Loading