-
Notifications
You must be signed in to change notification settings - Fork 7
Add Migration.sol action upgrade contract #15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
cdf86ff
d59a3b6
0565459
c99df92
59d425d
7d8ed6b
6a783c7
1d791eb
78808a5
6a632fd
e40d1ab
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
pragma solidity ^0.8.9; | ||
|
||
import "../challenge/IChallengeManager.sol"; | ||
import "../rollup/IRollupAdmin.sol"; | ||
|
||
import "@openzeppelin/contracts/proxy/transparent/ProxyAdmin.sol"; | ||
import "@openzeppelin/contracts/utils/Address.sol"; | ||
|
||
contract EspressoMigration{ | ||
function perform( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are some "constants" that are set in the constructor instead of having everything passed to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree that we should put as much into the constructor as possible. I suppose the question then becomes what constants we can set in the constructor? To me it seems like we could set the new OSP implememtation address in the constructor, and I'm tempted to say the wasmModuleRoot. However, there is a case that has me concerned. What if an orbit chain has some pre-existing changes to their STF that they want to merge with ours? If we declare an immutable wasmModuleRoot in the constructor, suddenly we can't use the deployed contract to help upgrade this hypothetical orbit chain as the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That rollup would then have to deploy their own upgrade action where they set the root as needed. |
||
address rollup, | ||
address proxyAdmin, | ||
bytes32 newWasmModuleRoot, | ||
bytes32 currentWasmModuleRoot, | ||
address newOspEntry, | ||
address currentOspEntry | ||
) public{ | ||
//Handle assertions in the perform functoin as we shouldn't be storing local state for delegated calls. | ||
require(newWasmModuleRoot != bytes32(0), "_newWasmModuleRoot cannot be empty"); | ||
|
||
require(currentWasmModuleRoot != bytes32(0), "_currentWasmModuleRoot cannot be empty"); | ||
|
||
require(Address.isContract(newOspEntry), "_newOsp must be a contract"); | ||
|
||
require(Address.isContract(currentOspEntry), "_currentOsp must be a contract"); | ||
|
||
// set the new challenge manager impl | ||
TransparentUpgradeableProxy challengeManager = | ||
TransparentUpgradeableProxy(payable(address(IRollupCore(rollup).challengeManager()))); | ||
address chalManImpl = ProxyAdmin(proxyAdmin).getProxyImplementation(challengeManager); | ||
ProxyAdmin(proxyAdmin).upgradeAndCall( | ||
challengeManager, | ||
chalManImpl, // Use the rollups current challenge manager as we only need to upgrade the OSP | ||
abi.encodeWithSelector(IChallengeManager.postUpgradeInit.selector, IOneStepProofEntry(newOspEntry), currentWasmModuleRoot, IOneStepProofEntry(currentOspEntry)) | ||
); | ||
|
||
require(IChallengeManager(address(challengeManager)).osp() == IOneStepProofEntry(newOspEntry), "new OSP not set"); | ||
|
||
IRollupAdmin(rollup).setWasmModuleRoot(newWasmModuleRoot); | ||
|
||
require(IRollupCore(rollup).wasmModuleRoot() == newWasmModuleRoot, "newWasmModuleRoot not set"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
pragma solidity ^0.8.0; | ||
// Example action contract to upgrade chain config to turn on espresso mode | ||
|
||
import "../precompiles/ArbOwner.sol"; | ||
|
||
contract ScheduleEspressoUpgrade { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you considered re-using what's here: https://github.com/OffchainLabs/orbit-actions/tree/main/scripts/foundry/arbos-upgrades/at-timestamp? I wonder if it would make sense to fork the orbit-actions repo instead of putting the contracts into this repo. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it might be beneficial to have a fork of the orbit-actions repo for the migration. I don't believe we need to include them in the integration repo like I had originally expected. If we do decide to move these contracts to a fork of the orbit-action repo we can just close this PR correct? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made a fork here: https://github.com/EspressoSystems/orbit-actions i think you can make a PR in that repo and reference this PR in the new one. |
||
function perform(uint64 timestamp) public { | ||
//The ArbOwner precompile always lives at this address. | ||
ArbOwner arbOwner = ArbOwner(0x0000000000000000000000000000000000000070); | ||
//This call must come from an address designated as an Owner by the ArbOwner contract. | ||
arbOwner.scheduleArbOSUpgrade(35, timestamp); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,220 @@ | ||
pragma solidity ^0.8.0; | ||
|
||
import "forge-std/Test.sol"; | ||
import "./util/TestUtil.sol"; | ||
import "../../src/challenge/IChallengeManager.sol"; | ||
import "../../src/osp/OneStepProver0.sol"; | ||
import "../../src/osp/OneStepProverMemory.sol"; | ||
import "../../src/osp/OneStepProverMath.sol"; | ||
import "../../src/osp/OneStepProverHostIo.sol"; | ||
import "../../src/osp/OneStepProofEntry.sol"; | ||
import "../../src/mocks/UpgradeExecutorMock.sol"; | ||
import "../../src/rollup/RollupCore.sol"; | ||
import "../../src/rollup/RollupCreator.sol"; | ||
import "../../test/foundry/RollupCreator.t.sol"; | ||
import "../../src/espresso-migration/EspressoMigration.sol"; | ||
|
||
contract MigrationTest is Test{ | ||
|
||
IOneStepProofEntry originalOspEntry; | ||
IOneStepProofEntry newOspEntry = IOneStepProofEntry( | ||
new OneStepProofEntry( | ||
new OneStepProver0(), | ||
new OneStepProverMemory(), | ||
new OneStepProverMath(), | ||
new OneStepProverHostIo(address(hotshot)) | ||
) | ||
); | ||
|
||
RollupCreator public rollupCreator; | ||
address public rollupAddress; | ||
address public rollupOwner = makeAddr("rollupOwner"); | ||
address public deployer = makeAddr("deployer"); | ||
IRollupAdmin public rollupAdmin; | ||
IRollupUser public rollupUser; | ||
DeployHelper public deployHelper; | ||
IReader4844 dummyReader4844 = IReader4844(address(137)); | ||
MockHotShot public hotshot = new MockHotShot(); | ||
IUpgradeExecutor upgradeExecutor; | ||
|
||
// 1 gwei | ||
uint256 public constant MAX_FEE_PER_GAS = 1_000_000_000; | ||
uint256 public constant MAX_DATA_SIZE = 117_964; | ||
|
||
BridgeCreator.BridgeContracts public ethBasedTemplates = | ||
BridgeCreator.BridgeContracts({ | ||
bridge: new Bridge(), | ||
sequencerInbox: new SequencerInbox(MAX_DATA_SIZE, dummyReader4844, false), | ||
inbox: new Inbox(MAX_DATA_SIZE), | ||
rollupEventInbox: new RollupEventInbox(), | ||
outbox: new Outbox() | ||
}); | ||
BridgeCreator.BridgeContracts public erc20BasedTemplates = | ||
BridgeCreator.BridgeContracts({ | ||
bridge: new ERC20Bridge(), | ||
sequencerInbox: new SequencerInbox(MAX_DATA_SIZE, dummyReader4844, true), | ||
inbox: new ERC20Inbox(MAX_DATA_SIZE), | ||
rollupEventInbox: new ERC20RollupEventInbox(), | ||
outbox: new ERC20Outbox() | ||
}); | ||
|
||
|
||
/* solhint-disable func-name-mixedcase */ | ||
//create items needed for a rollup and deploy it. This code is lovingly borrowed from the rollupcreator.t.sol foundry test. | ||
function setUp() public { | ||
//// deploy rollup creator and set templates | ||
vm.startPrank(deployer); | ||
rollupCreator = new RollupCreator(); | ||
deployHelper = new DeployHelper(); | ||
|
||
for (uint256 i = 1; i < 10; i++) { | ||
hotshot.setCommitment(uint256(i), uint256(i)); | ||
} | ||
|
||
// deploy BridgeCreators | ||
BridgeCreator bridgeCreator = new BridgeCreator(ethBasedTemplates, erc20BasedTemplates); | ||
|
||
IUpgradeExecutor upgradeExecutorLogic = new UpgradeExecutorMock(); | ||
upgradeExecutor = upgradeExecutorLogic; | ||
|
||
( | ||
IOneStepProofEntry ospEntry, | ||
IChallengeManager challengeManager, | ||
IRollupAdmin _rollupAdmin, | ||
IRollupUser _rollupUser | ||
) = _prepareRollupDeployment(); | ||
|
||
originalOspEntry = ospEntry; | ||
|
||
rollupAdmin = _rollupAdmin; | ||
rollupUser = _rollupUser; | ||
|
||
//// deploy creator and set logic | ||
rollupCreator.setTemplates( | ||
bridgeCreator, | ||
ospEntry, | ||
challengeManager, | ||
_rollupAdmin, | ||
_rollupUser, | ||
upgradeExecutorLogic, | ||
address(new ValidatorUtils()), | ||
address(new ValidatorWalletCreator()), | ||
deployHelper | ||
); | ||
|
||
// deployment params | ||
ISequencerInbox.MaxTimeVariation memory timeVars = ISequencerInbox.MaxTimeVariation( | ||
((60 * 60 * 24) / 15), | ||
12, | ||
60 * 60 * 24, | ||
60 * 60 | ||
); | ||
Config memory config = Config({ | ||
confirmPeriodBlocks: 20, | ||
extraChallengeTimeBlocks: 200, | ||
stakeToken: address(0), | ||
baseStake: 1000, | ||
wasmModuleRoot: keccak256("wasm"), | ||
owner: rollupOwner, | ||
loserStakeEscrow: address(200), | ||
chainId: 1337, | ||
chainConfig: "abc", | ||
genesisBlockNum: 15_000_000, | ||
sequencerInboxMaxTimeVariation: timeVars | ||
}); | ||
|
||
// prepare funds | ||
uint256 factoryDeploymentFunds = 1 ether; | ||
vm.deal(deployer, factoryDeploymentFunds); | ||
uint256 balanceBefore = deployer.balance; | ||
|
||
/// deploy rollup | ||
address[] memory batchPosters = new address[](1); | ||
batchPosters[0] = makeAddr("batch poster 1"); | ||
address batchPosterManager = makeAddr("batch poster manager"); | ||
address[] memory validators = new address[](2); | ||
validators[0] = makeAddr("validator1"); | ||
validators[1] = makeAddr("validator2"); | ||
|
||
RollupCreator.RollupDeploymentParams memory deployParams = RollupCreator | ||
.RollupDeploymentParams({ | ||
config: config, | ||
batchPosters: batchPosters, | ||
validators: validators, | ||
maxDataSize: MAX_DATA_SIZE, | ||
nativeToken: address(0), | ||
deployFactoriesToL2: true, | ||
maxFeePerGasForRetryables: MAX_FEE_PER_GAS, | ||
batchPosterManager: batchPosterManager | ||
}); | ||
rollupAddress = rollupCreator.createRollup{value: factoryDeploymentFunds}( | ||
deployParams | ||
); | ||
|
||
vm.stopPrank(); | ||
} | ||
|
||
function _prepareRollupDeployment() | ||
internal | ||
returns ( | ||
IOneStepProofEntry ospEntry, | ||
IChallengeManager challengeManager, | ||
IRollupAdmin rollupAdminLogic, | ||
IRollupUser rollupUserLogic | ||
) | ||
{ | ||
//// deploy challenge stuff | ||
ospEntry = new OneStepProofEntry( | ||
new OneStepProver0(), | ||
new OneStepProverMemory(), | ||
new OneStepProverMath(), | ||
new OneStepProverHostIo(address(hotshot)) | ||
); | ||
challengeManager = new ChallengeManager(); | ||
|
||
//// deploy rollup logic | ||
rollupAdminLogic = IRollupAdmin(new RollupAdminLogic()); | ||
rollupUserLogic = IRollupUser(new RollupUserLogic()); | ||
|
||
return (ospEntry, challengeManager, rollupAdminLogic, rollupUserLogic); | ||
} | ||
|
||
function _getProxyAdmin(address proxy) internal view returns (address) { | ||
bytes32 adminSlot = bytes32(uint256(keccak256("eip1967.proxy.admin")) - 1); | ||
return address(uint160(uint256(vm.load(proxy, adminSlot)))); | ||
} | ||
|
||
function test_migrateToEspresso() public{ | ||
IRollupCore rollup = IRollupCore(rollupAddress); | ||
// IRollupAdmin rollupAdministrator = IRollupAdmin(rollupAdmin); | ||
address migration = address(new EspressoMigration()); | ||
address upgradeExecutorExpectedAddress = computeCreateAddress(address(rollupCreator), 4); | ||
assertEq( | ||
ProxyAdmin(_getProxyAdmin(address(rollup.sequencerInbox()))).owner(), | ||
upgradeExecutorExpectedAddress, | ||
"Invalid proxyAdmin's owner" | ||
); | ||
IUpgradeExecutor _upgradeExecutor = IUpgradeExecutor( | ||
upgradeExecutorExpectedAddress | ||
); | ||
|
||
|
||
bytes memory data = abi.encodeWithSelector( | ||
EspressoMigration.perform.selector, | ||
rollup, | ||
computeCreateAddress(address(rollupCreator), 1), // the address that the rollup creator would deploy based on the nonce. For the arbitrum rollup creator this will be the first deployment. | ||
bytes32(uint256(keccak256("newRoot"))), // create a new dummy root. | ||
bytes32(uint256(keccak256("wasm"))), // current wasm module root as defined in the config | ||
newOspEntry, | ||
originalOspEntry | ||
); | ||
|
||
vm.prank(rollupOwner); | ||
_upgradeExecutor.execute(migration, data); | ||
vm.stopPrank(); | ||
assertEq(address(rollup.challengeManager().getOsp(bytes32(uint256(keccak256("wasm"))))), address(originalOspEntry), "CondOsp at original root is not what was expected."); | ||
assertEq(address(rollup.challengeManager().getOsp(bytes32(uint256(keccak256("newRoot"))))), address(newOspEntry), "CondOsp at new root is not what was expected."); | ||
assertEq(rollup.wasmModuleRoot(), bytes32(uint256(keccak256("newRoot"))), "Rollup's wasmModuleRoot was not changed by migration"); | ||
} | ||
|
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should also follow the upstream governance guidelines as much as possible.
https://github.com/ArbitrumFoundation/governance/tree/main/src/gov-action-contracts#governance-action-contract-standards-and-guidelines