-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Add initial StateTransitioner & FraudProver #156
Changes from 15 commits
148bc00
0c7a2c4
38f7842
f9c0736
b172f26
ce7221b
ea7f844
a35a34d
548a209
40ed3a3
94fc0c5
7704cf7
1a62772
bbca98f
9936120
73fae93
5c1e834
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 |
---|---|---|
|
@@ -96,6 +96,14 @@ contract ExecutionManager { | |
// TODO | ||
} | ||
|
||
/** | ||
* @notice Sets a new state manager to be associated with the execution manager. | ||
* This is used when we want to swap out a new backend to be used for a different execution. | ||
*/ | ||
function setStateManager(address _stateManagerAddress) external { | ||
stateManager = FullStateManager(_stateManagerAddress); | ||
} | ||
|
||
/** | ||
* @notice Increments the provided address's nonce. | ||
* This is only used by the sequencer to correct nonces when transactions fail. | ||
|
@@ -582,8 +590,7 @@ contract ExecutionManager { | |
restoreContractContext(oldMsgSender, oldActiveContract); | ||
return false; | ||
} | ||
// Associate the code contract with our ovm contract | ||
stateManager.associateCodeContract(_newOvmContractAddress, codeContractAddress); | ||
// TODO: Replace `getCodeContractBytecode(...) with `getOvmContractBytecode(...) | ||
bytes memory codeContractBytecode = stateManager.getCodeContractBytecode(codeContractAddress); | ||
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. Note that we need to have code hash(and probably size) authenticated in the SM, so we should be pulling this directly. |
||
// Get the code contract address to be emitted by a CreatedContract event | ||
bytes32 codeContractHash = keccak256(codeContractBytecode); | ||
|
@@ -894,6 +901,7 @@ contract ExecutionManager { | |
address _targetOvmContractAddress = address(bytes20(_targetAddressBytes)); | ||
address codeContractAddress = stateManager.getCodeContractAddress(_targetOvmContractAddress); | ||
|
||
// TODO: Replace `getCodeContractHash(...) with `getOvmContractHash(...) | ||
bytes32 hash = stateManager.getCodeContractHash(codeContractAddress); | ||
|
||
assembly { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -104,7 +104,7 @@ contract FullStateManager is StateManager { | |
* @param _ovmContractAddress The address of the OVM contract we'd like to associate with some code. | ||
* @param _codeContractAddress The address of the code contract that's been deployed. | ||
*/ | ||
function associateCodeContract(address _ovmContractAddress, address _codeContractAddress) external { | ||
function associateCodeContract(address _ovmContractAddress, address _codeContractAddress) public { | ||
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. Does this need to be authenticated? Would expect only the Fraud Verifier to be able to populate these associations. 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. Ah, whoops, I see that this isn't the PSM. Will just resurface the nit/bikeshed that |
||
ovmCodeContracts[_ovmContractAddress] = _codeContractAddress; | ||
} | ||
|
||
|
@@ -183,6 +183,10 @@ contract FullStateManager is StateManager { | |
// Contract runtime bytecode is not pure. | ||
return ZERO_ADDRESS; | ||
} | ||
|
||
// Associate the code contract with our ovm contract | ||
associateCodeContract(_newOvmContractAddress, codeContractAddress); | ||
|
||
return codeContractAddress; | ||
} | ||
} |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
pragma solidity ^0.5.0; | ||
|
||
import {StateTransitioner} from "./StateTransitioner.sol"; | ||
|
||
/** | ||
* @title FraudVerifier | ||
* @notice The contract which is able to delete invalid state roots. | ||
*/ | ||
contract FraudVerifier { | ||
mapping(uint=>StateTransitioner) stateTransitioners; | ||
|
||
function initNewStateTransitioner(uint _preStateTransitionIndex) public returns(bool) { | ||
// TODO: | ||
// Create a new state transitioner for some specific pre-state transition index (assuming one hasn't already been made). | ||
// Note that the invalid state root that we are verifying is at _preStateTransitionIndex+1. | ||
// Add it to the stateTransitioners mapping! -- stateTransitioners[_preStateTransitionIndex] = newStateTransitioner; | ||
return true; | ||
} | ||
|
||
|
||
function verifyFraud(uint _transitionIndex) public returns(bool) { | ||
// TODO: | ||
// Simply verify that the state transitioner has completed, and that the state root | ||
// at _preStateTransitionIndex+1 is not equal to the state root which was committed for that index. | ||
return true; | ||
} | ||
Comment on lines
+21
to
+26
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. Note that in the current contracts I believe that the |
||
} |
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.
Is this function meant to be atomically called before
executeTransaction
? If so, might be able to move that logic there in a future PR. If not atomic, do we need to authenticate that it's coming from the fraud prover?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.
yeah i totally agree we might as well put it into
executeTransaction(..)
itself