forked from ethstorage/optimism
-
Notifications
You must be signed in to change notification settings - Fork 1
Develop 4ary e2e #7
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
Open
JustXxx
wants to merge
11
commits into
develop
Choose a base branch
from
develop_4ary_e2e
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
| // Fetch the grandparent clock, if it exists. | ||
| // The grandparent clock should always exist unless the parent is the root claim. | ||
| Clock grandparentClock; |
Check warning
Code scanning / Slither
Uninitialized local variables
FaultDisputeGame.moveV2(uint256,Claim,uint256).grandparentClock (src/dispute/FaultDisputeGame.sol#930) is a local variable never initialized
Comment on lines
+810
to
+877
| function stepV2( | ||
| uint256 _claimIndex, | ||
| uint64 _attackBranch, | ||
| bytes calldata _stateData, | ||
| bytes calldata _proof | ||
| ) | ||
| public | ||
| virtual | ||
| { | ||
| // INVARIANT: Steps cannot be made unless the game is currently in progress. | ||
| if (status != GameStatus.IN_PROGRESS) revert GameNotInProgress(); | ||
|
|
||
| // Get the parent. If it does not exist, the call will revert with OOB. | ||
| ClaimData storage parent = claimData[_claimIndex]; | ||
|
|
||
| // Pull the parent position out of storage. | ||
| Position parentPos = parent.position; | ||
| // Determine the position of the step. | ||
| Position stepPos = parentPos.moveN(nBits, _attackBranch); | ||
|
|
||
| // INVARIANT: A step cannot be made unless the move position is 1 below the `MAX_GAME_DEPTH` | ||
| if (stepPos.depth() != MAX_GAME_DEPTH + nBits) revert InvalidParent(); | ||
|
|
||
| // Determine the expected pre & post states of the step. | ||
| Claim preStateClaim; | ||
| Position preStatePosition; | ||
| Claim postStateClaim; | ||
| Position postStatePosition; | ||
| //ClaimData storage postState; | ||
|
|
||
| // TODO: deal with SPLIT_DEPTH | ||
| (preStatePosition, preStateClaim) = findPreStateClaim(1 << nBits, stepPos, _claimIndex); | ||
| //(preStatePosition, preStateClaim) = findPreStateClaim(1 << nBits, parentPos, _claimIndex); | ||
| (postStatePosition, postStateClaim) = findPostStateClaim(1 << nBits, stepPos, _claimIndex); | ||
| //(postStatePosition, postStateClaim) = findPostStateClaim(1 << nBits, parentPos, _claimIndex); | ||
|
|
||
| // INVARIANT: The prestate is always invalid if the passed `_stateData` is not the | ||
| // preimage of the prestate claim hash. | ||
| // We ignore the highest order byte of the digest because it is used to | ||
| // indicate the VM Status and is added after the digest is computed. | ||
| if (keccak256(_stateData) << 8 != preStateClaim.raw() << 8) revert InvalidPrestate(); | ||
|
|
||
| // Compute the local preimage context for the step. | ||
| Hash uuid = _findLocalContext(_claimIndex); | ||
|
|
||
| // INVARIANT: If a step is an attack, the poststate is valid if the step produces | ||
| // the same poststate hash as the parent claim's value. | ||
| // If a step is a defense: | ||
| // 1. If the parent claim and the found post state agree with each other | ||
| // (depth diff % 2 == 0), the step is valid if it produces the same | ||
| // state hash as the post state's claim. | ||
| // 2. If the parent claim and the found post state disagree with each other | ||
| // (depth diff % 2 != 0), the parent cannot be countered unless the step | ||
| // produces the same state hash as `postState.claim`. | ||
| // SAFETY: While the `attack` path does not need an extra check for the post | ||
| // state's depth in relation to the parent, we don't need another | ||
| // branch because (n - n) % 2 == 0. | ||
| bool validStep = VM.step(_stateData, _proof, uuid.raw()) == postStateClaim.raw(); | ||
| bool parentPostAgree = (parentPos.depth() - postStatePosition.depth()) % 2 == 0; | ||
| if (parentPostAgree == validStep) revert ValidStep(); | ||
|
|
||
| // INVARIANT: A step cannot be made against a claim for a second time. | ||
| if (parent.counteredBy != address(0)) revert DuplicateStep(); | ||
|
|
||
| // Set the parent claim as countered. We do not need to append a new claim to the game; | ||
| // instead, we can just set the existing parent as countered. | ||
| parent.counteredBy = msg.sender; | ||
| } |
Check warning
Code scanning / Slither
Reentrancy vulnerabilities
Reentrancy in FaultDisputeGame.stepV2(uint256,uint64,bytes,bytes) (src/dispute/FaultDisputeGame.sol#810-877):
External calls:
- validStep = VM.step(_stateData,_proof,uuid.raw()) == postStateClaim.raw() (src/dispute/FaultDisputeGame.sol#867)
State variables written after the call(s):
- parent.counteredBy = msg.sender (src/dispute/FaultDisputeGame.sol#876)
FaultDisputeGame.claimData (src/dispute/FaultDisputeGame.sol#77) can be used in cross function reentrancies:
- FaultDisputeGame._findStartingAndDisputedOutputs(uint256) (src/dispute/FaultDisputeGame.sol#688-714)
- FaultDisputeGame._findTraceAncestor(Position,uint256,bool) (src/dispute/FaultDisputeGame.sol#661-679)
- FaultDisputeGame.claimData (src/dispute/FaultDisputeGame.sol#77)
- FaultDisputeGame.claimDataLen() (src/dispute/FaultDisputeGame.sol#472-474)
- FaultDisputeGame.findPostStateClaim(uint256,Position,uint256) (src/dispute/FaultDisputeGame.sol#793-808)
- FaultDisputeGame.findPreStateClaim(uint256,Position,uint256) (src/dispute/FaultDisputeGame.sol#770-791)
- FaultDisputeGame.initialize() (src/dispute/FaultDisputeGame.sol#415-469)
- FaultDisputeGame.moveV2(uint256,Claim,uint64) (src/dispute/FaultDisputeGame.sol#879-985)
- FaultDisputeGame.resolve() (src/dispute/FaultDisputeGame.sol#302-314)
- FaultDisputeGame.resolveClaim(uint256) (src/dispute/FaultDisputeGame.sol#317-389)
- FaultDisputeGame.step(uint256,bool,bytes,bytes) (src/dispute/FaultDisputeGame.sol#149-227)
- FaultDisputeGame.stepV2(uint256,uint64,bytes,bytes) (src/dispute/FaultDisputeGame.sol#810-877)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.