[executor] implement upgradability (#1198)
* get all test to work * executor upgradable * update comments * address feedback * fix tests
This commit is contained in:
parent
cd604d56f2
commit
1816838c8a
|
@ -42,13 +42,13 @@ contract Executor {
|
|||
uint16 private ownerEmitterChainId;
|
||||
bytes32 private ownerEmitterAddress;
|
||||
|
||||
constructor(
|
||||
function _initialize(
|
||||
address _wormhole,
|
||||
uint64 _lastExecutedSequence,
|
||||
uint16 _chainId,
|
||||
uint16 _ownerEmitterChainId,
|
||||
bytes32 _ownerEmitterAddress
|
||||
) {
|
||||
) internal {
|
||||
require(_wormhole != address(0), "_wormhole is zero address");
|
||||
|
||||
wormhole = IWormhole(_wormhole);
|
||||
|
|
|
@ -16,4 +16,6 @@ library ExecutorErrors {
|
|||
error InvalidGovernanceTarget();
|
||||
// The target address for the contract call is not a contract
|
||||
error InvalidContractTarget();
|
||||
// The governance message is not valid
|
||||
error InvalidMagicValue();
|
||||
}
|
||||
|
|
|
@ -0,0 +1,95 @@
|
|||
// SPDX-License-Identifier: Apache 2
|
||||
pragma solidity ^0.8.0;
|
||||
|
||||
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
|
||||
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
|
||||
import "@openzeppelin/contracts-upgradeable/access/Ownable2StepUpgradeable.sol";
|
||||
|
||||
import "./Executor.sol";
|
||||
import "./ExecutorErrors.sol";
|
||||
|
||||
contract ExecutorUpgradable is
|
||||
Initializable,
|
||||
Ownable2StepUpgradeable,
|
||||
UUPSUpgradeable,
|
||||
Executor
|
||||
{
|
||||
event ContractUpgraded(
|
||||
address oldImplementation,
|
||||
address newImplementation
|
||||
);
|
||||
|
||||
function initialize(
|
||||
address wormhole,
|
||||
uint64 lastExecutedSequence,
|
||||
uint16 chainId,
|
||||
uint16 ownerEmitterChainId,
|
||||
bytes32 ownerEmitterAddress
|
||||
) public initializer {
|
||||
require(wormhole != address(0), "wormhole is zero address");
|
||||
|
||||
__Ownable_init();
|
||||
__UUPSUpgradeable_init();
|
||||
|
||||
Executor._initialize(
|
||||
wormhole,
|
||||
lastExecutedSequence,
|
||||
chainId,
|
||||
ownerEmitterChainId,
|
||||
ownerEmitterAddress
|
||||
);
|
||||
|
||||
// Transfer ownership to the contract itself.
|
||||
_transferOwnership(address(this));
|
||||
}
|
||||
|
||||
/// Ensures the contract cannot be uninitialized and taken over.
|
||||
/// @custom:oz-upgrades-unsafe-allow constructor
|
||||
constructor() initializer {}
|
||||
|
||||
// Only allow the owner to upgrade the proxy to a new implementation.
|
||||
function _authorizeUpgrade(address) internal override onlyOwner {}
|
||||
|
||||
// Upgrade the contract to the given newImplementation. The `newImplementation`
|
||||
// should implement the method `entropyUpgradableMagic`, see below. If the method
|
||||
// is not implemented or if the magic is different from the current contract, this call
|
||||
// will revert.
|
||||
function upgradeTo(address newImplementation) external override onlyProxy {
|
||||
address oldImplementation = _getImplementation();
|
||||
_authorizeUpgrade(newImplementation);
|
||||
_upgradeToAndCallUUPS(newImplementation, new bytes(0), false);
|
||||
|
||||
magicCheck();
|
||||
|
||||
emit ContractUpgraded(oldImplementation, _getImplementation());
|
||||
}
|
||||
|
||||
// Upgrade the contract to the given newImplementation and call it with the given data.
|
||||
// The `newImplementation` should implement the method `entropyUpgradableMagic`, see
|
||||
// below. If the method is not implemented or if the magic is different from the current
|
||||
// contract, this call will revert.
|
||||
function upgradeToAndCall(
|
||||
address newImplementation,
|
||||
bytes memory data
|
||||
) external payable override onlyProxy {
|
||||
address oldImplementation = _getImplementation();
|
||||
_authorizeUpgrade(newImplementation);
|
||||
_upgradeToAndCallUUPS(newImplementation, data, true);
|
||||
|
||||
magicCheck();
|
||||
|
||||
emit ContractUpgraded(oldImplementation, _getImplementation());
|
||||
}
|
||||
|
||||
function magicCheck() internal view {
|
||||
// Calling a method using `this.<method>` will cause a contract call that will use
|
||||
// the new contract. This call will fail if the method does not exists or the magic
|
||||
// is different.
|
||||
if (this.entropyUpgradableMagic() != 0x66697288)
|
||||
revert ExecutorErrors.InvalidMagicValue();
|
||||
}
|
||||
|
||||
function entropyUpgradableMagic() public pure returns (uint32) {
|
||||
return 0x66697288;
|
||||
}
|
||||
}
|
|
@ -4,7 +4,7 @@ pragma solidity ^0.8.0;
|
|||
|
||||
import "./utils/EntropyTestUtils.t.sol";
|
||||
import "../contracts/entropy/EntropyUpgradable.sol";
|
||||
import "./utils/EntropyTestContracts/EntropyDifferentMagic.t.sol";
|
||||
import "./utils/InvalidMagic.t.sol";
|
||||
import "@openzeppelin/contracts/proxy/ERC1967/ERC1967Proxy.sol";
|
||||
import "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol";
|
||||
|
||||
|
@ -12,7 +12,7 @@ contract EntropyAuthorized is Test, EntropyTestUtils {
|
|||
ERC1967Proxy public proxy;
|
||||
EntropyUpgradable public random;
|
||||
EntropyUpgradable public random2;
|
||||
EntropyDifferentMagic public randomDifferentMagic;
|
||||
InvalidMagic public randomDifferentMagic;
|
||||
|
||||
address public owner = address(1);
|
||||
address public admin = address(2);
|
||||
|
@ -36,7 +36,7 @@ contract EntropyAuthorized is Test, EntropyTestUtils {
|
|||
random = EntropyUpgradable(address(proxy));
|
||||
// to test for upgrade
|
||||
random2 = new EntropyUpgradable();
|
||||
randomDifferentMagic = new EntropyDifferentMagic();
|
||||
randomDifferentMagic = new InvalidMagic();
|
||||
|
||||
random.initialize(owner, admin, pythFeeInWei, provider1, false);
|
||||
}
|
||||
|
|
|
@ -4,13 +4,16 @@ pragma solidity ^0.8.0;
|
|||
|
||||
import "forge-std/Test.sol";
|
||||
import "@pythnetwork/entropy-sdk-solidity/EntropyStructs.sol";
|
||||
import "../contracts/executor/Executor.sol";
|
||||
import "../contracts/executor/ExecutorUpgradable.sol";
|
||||
import "./utils/WormholeTestUtils.t.sol";
|
||||
import "./utils/InvalidMagic.t.sol";
|
||||
|
||||
contract ExecutorTest is Test, WormholeTestUtils {
|
||||
Wormhole public wormhole;
|
||||
Executor public executor;
|
||||
ExecutorUpgradable public executor;
|
||||
ExecutorUpgradable public executor2;
|
||||
TestCallable public callable;
|
||||
InvalidMagic public executorInvalidMagic;
|
||||
|
||||
uint16 OWNER_CHAIN_ID = 7;
|
||||
bytes32 OWNER_EMITTER = bytes32(uint256(1));
|
||||
|
@ -19,7 +22,13 @@ contract ExecutorTest is Test, WormholeTestUtils {
|
|||
|
||||
function setUp() public {
|
||||
address _wormhole = setUpWormholeReceiver(NUM_SIGNERS);
|
||||
executor = new Executor(
|
||||
ExecutorUpgradable _executor = new ExecutorUpgradable();
|
||||
ERC1967Proxy _proxy = new ERC1967Proxy(address(_executor), "");
|
||||
executor = ExecutorUpgradable(payable(address(_proxy)));
|
||||
executor2 = new ExecutorUpgradable();
|
||||
executorInvalidMagic = new InvalidMagic();
|
||||
|
||||
executor.initialize(
|
||||
_wormhole,
|
||||
0,
|
||||
CHAIN_ID,
|
||||
|
@ -56,6 +65,51 @@ contract ExecutorTest is Test, WormholeTestUtils {
|
|||
executor.execute(vaa);
|
||||
}
|
||||
|
||||
function getTestUpgradeVaa(
|
||||
address newImplementation
|
||||
) internal returns (bytes memory vaa) {
|
||||
bytes memory payload = abi.encodePacked(
|
||||
uint32(0x5054474d),
|
||||
PythGovernanceInstructions.GovernanceModule.EvmExecutor,
|
||||
Executor.ExecutorAction.Execute,
|
||||
CHAIN_ID,
|
||||
address(executor),
|
||||
address(executor),
|
||||
abi.encodeWithSelector(
|
||||
ExecutorUpgradable.upgradeTo.selector,
|
||||
newImplementation
|
||||
)
|
||||
);
|
||||
|
||||
vaa = generateVaa(
|
||||
uint32(block.timestamp),
|
||||
OWNER_CHAIN_ID,
|
||||
OWNER_EMITTER,
|
||||
1,
|
||||
payload,
|
||||
NUM_SIGNERS
|
||||
);
|
||||
}
|
||||
|
||||
function testUpgradeCallSucceedsForContractWithCorrectMagic() public {
|
||||
bytes memory vaa = getTestUpgradeVaa(address(executor2));
|
||||
executor.execute(vaa);
|
||||
}
|
||||
|
||||
function testUpgradeCallFailsForNotUUPSContract() public {
|
||||
bytes memory vaa = getTestUpgradeVaa(address(callable));
|
||||
|
||||
vm.expectRevert("ERC1967Upgrade: new implementation is not UUPS");
|
||||
executor.execute(vaa);
|
||||
}
|
||||
|
||||
function testUpgradeCallFailsForInvalidMagic() public {
|
||||
bytes memory vaa = getTestUpgradeVaa(address(executorInvalidMagic));
|
||||
|
||||
vm.expectRevert(ExecutorErrors.InvalidMagicValue.selector);
|
||||
executor.execute(vaa);
|
||||
}
|
||||
|
||||
function testCallSucceeds() public {
|
||||
callable.reset();
|
||||
|
||||
|
|
|
@ -4,13 +4,8 @@ pragma solidity ^0.8.0;
|
|||
import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol";
|
||||
import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
|
||||
import "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol";
|
||||
import "@pythnetwork/entropy-sdk-solidity/EntropyErrors.sol";
|
||||
|
||||
contract EntropyDifferentMagic is
|
||||
Initializable,
|
||||
OwnableUpgradeable,
|
||||
UUPSUpgradeable
|
||||
{
|
||||
contract InvalidMagic is Initializable, OwnableUpgradeable, UUPSUpgradeable {
|
||||
function initialize() public initializer {
|
||||
__Ownable_init();
|
||||
__UUPSUpgradeable_init();
|
||||
|
@ -23,15 +18,7 @@ contract EntropyDifferentMagic is
|
|||
// // Only allow the owner to upgrade the proxy to a new implementation.
|
||||
function _authorizeUpgrade(address) internal override onlyOwner {}
|
||||
|
||||
function magicCheck() internal view {
|
||||
// Calling a method using `this.<method>` will cause a contract call that will use
|
||||
// the new contract. This call will fail if the method does not exists or the magic
|
||||
// is different.
|
||||
if (this.entropyUpgradableMagic() != 0x666972)
|
||||
revert EntropyErrors.InvalidUpgradeMagic();
|
||||
}
|
||||
|
||||
function entropyUpgradableMagic() public pure returns (uint32) {
|
||||
return 0x666972;
|
||||
return 0x000000;
|
||||
}
|
||||
}
|
Loading…
Reference in New Issue