Bugfix: Added _attackbranch logic to the _verifyExecBisectionRoot fun… #33
Conversation
| uint8 vmStatus = uint8(_rootClaim.raw()[0]); | ||
|
|
||
| if ((0 != _attackBranch) || (disputedPos.depth() / N_BITS) % 2 == (SPLIT_DEPTH / N_BITS) % 2) { | ||
| if ((MAX_ATTACK_BRANCH != _attackBranch) || (disputedPos.depth() / N_BITS) % 2 == (SPLIT_DEPTH / N_BITS) % 2) { |
There was a problem hiding this comment.
I think if disputed output and creator of the execution trace subgame disagrees, it might be ((disputedPos.depth() / N_BITS) % N_BITS == (SPLIT_DEPTH / N_BITS) % N_BITS) instead?
There was a problem hiding this comment.
% 2 is because we have only two parties - challenger and defender that play the game. The condition here is basically to check the submitter of the disputedLeafPos is not the same as the submitter of the output of the block transition (at SPLIT_DEPTH + 1).
|
|
||
| /// @dev Tests that making a claim at the execution trace bisection root level with a valid status | ||
| /// byte succeeds. | ||
| function test_move_correctStatusExecRoot_succeeds() public { |
There was a problem hiding this comment.
As suggested in last week's meeting, better to do regression test. e.g, write tests that can reproduce the failing cases before the fix and pass after the fix
qizhou
left a comment
qizhou
left a comment
There was a problem hiding this comment.
Can we confirm the new tests fail the previous code?
| uint8 vmStatus = uint8(_rootClaim.raw()[0]); | ||
|
|
||
| if ((0 != _attackBranch) || (disputedPos.depth() / N_BITS) % 2 == (SPLIT_DEPTH / N_BITS) % 2) { | ||
| if ((MAX_ATTACK_BRANCH != _attackBranch) || (disputedPos.depth() / N_BITS) % 2 == (SPLIT_DEPTH / N_BITS) % 2) { |
There was a problem hiding this comment.
% 2 is because we have only two parties - challenger and defender that play the game. The condition here is basically to check the submitter of the disputedLeafPos is not the same as the submitter of the output of the block transition (at SPLIT_DEPTH + 1).
|
fix by #35 |
3 participants
forge test --mc FaultDisputeGameN_Test --mt test_move_incorrectStatusExecRoot_reverts
forge test --mc FaultDisputeGameN_Test --mt test_move_correctStatusExecRoot_succeeds