-
Notifications
You must be signed in to change notification settings - Fork 118
[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
@@ -322,6 +322,24 @@ library Lib_SafeExecutionManagerWrapper { | |||
); | |||
} | |||
|
|||
/** | |||
* 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 getAccount(address _address) external view returns (Lib_OVMCodec.Account memory _account); | ||
function hasAccount(address _address) external view returns (bool _exists); | ||
function hasEmptyAccount(address _address) external view returns (bool _exists); | ||
function setAccountNonce(address _address, uint256 _nonce) external; | ||
function getAccountNonce(address _address) external view returns (uint256 _nonce); | ||
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