-
Notifications
You must be signed in to change notification settings - Fork 125
[wip] OVM self-upgradability #357
base: master
Are you sure you want to change the base?
Conversation
…er.sol Co-authored-by: smartcontracts <[email protected]>
…er.sol Co-authored-by: smartcontracts <[email protected]>
…er.sol Co-authored-by: smartcontracts <[email protected]>
…er.sol Co-authored-by: smartcontracts <[email protected]>
…er.sol Co-authored-by: smartcontracts <[email protected]>
| onlyCallableBy(address(0x4200000000000000000000000000000000000009)) | ||
| { | ||
| _checkAccountLoad(_address); | ||
| ovmStateManager.putAccountCode(_address, _code); |
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 need to be very explicit about the security considerations here. Primarily w/r/t how fraud proofs are impacted, what's safe to do, what isn't. etc.
contracts/optimistic-ethereum/libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| /** | ||
| * Performs a safe asdfasdfasdfasdf call. |
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.
Reminder to update this.
| function getAccountEthAddress(address _address) external view returns (address _ethAddress); | ||
| function getAccountStorageRoot(address _address) external view returns (bytes32 _storageRoot); | ||
| function initPendingAccount(address _address) external; | ||
| function initPendingAccount(address _address) external; // todo: deprecate/combine these two with this change? |
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.
Reminder on this todo
test/helpers/constants.ts
Outdated
| getContractDefinition('Helper_TestRunner').deployedBytecode | ||
| ).byteLength | ||
| } catch { | ||
| 1 |
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.
what does this line do?
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.
It stores a digital history of my typo :)
| import { Lib_ECDSAUtils } from "../../libraries/utils/Lib_ECDSAUtils.sol"; | ||
| import { Lib_SafeExecutionManagerWrapper } from "../../libraries/wrappers/Lib_SafeExecutionManagerWrapper.sol"; | ||
|
|
||
| contract OVM_Upgrader { |
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.
Needs comments
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.
Also although IMO the name OVM_Upgrader is fine, we should take a few mins to brainstorm names + make sure the name won't be confusing going forward. Since it's much easier to change the name now than in the future, as we're well aware...
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata