Skip to content

feat(ctb): OutputBisectionGame test refactor + fixes#8346

Merged
clabby merged 9 commits intodevelopfrom
cl/ctb/output-bisection-test-refactor
Dec 7, 2023
Merged

feat(ctb): OutputBisectionGame test refactor + fixes#8346
clabby merged 9 commits intodevelopfrom
cl/ctb/output-bisection-test-refactor

Conversation

@clabby
Copy link
Contributor

@clabby clabby commented Nov 29, 2023

Overview

Implements a new set of actors for the OutputBisectionGame, which support the new semantics of the game, as well as implements some fixes / improvements to the rough draft.

Notes

  • Add a GameSolver, which is an observe-only entity that will inform actors of suggested moves when asked. This decouples the handling of the solving logic from the actors, allowing for the solving logic to be swapped out easily.
    • Doing away with the old GamePlayer idea, was too clunky and the abstractions didn't lend well towards the new semantics or refactors.
  • Fixed Position localization for the step function.
  • [ ] Add several actors derived from the GameSolver
  • [ ] Add exhaustive tests over the full game state using the actors.

Note: Merging this early to get fixes in for the challenger implementing op-e2e tests, following up with actors on a new PR.

@clabby clabby requested a review from a team as a code owner November 29, 2023 19:48
@clabby clabby requested a review from refcell November 29, 2023 19:48
@clabby clabby marked this pull request as draft November 29, 2023 19:48
@clabby
Copy link
Contributor Author

clabby commented Nov 29, 2023

Current dependencies on/for this PR:

This stack of pull requests is managed by Graphite.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 29, 2023

Walkthrough

The updates across various Solidity contracts and libraries involve renaming variables, enhancing comments with TODO notes, refining function logic, and adding new contracts and functions. The changes reflect a focus on improving the dispute resolution logic, particularly around output bisection games, with attention to handling edge cases and improving code clarity and structure.

Changes

File Path Change Summary
.../contracts-bedrock/src/dispute/OutputBisectionGame.sol Renamed l1Head to settlementHead, updated findStartingAndDisputedOutputs() logic with TODOs, added new comments, and updated the version constant.
.../contracts-bedrock/src/dispute/interfaces/IOutputBisectionGame.sol Removed l1Head() and added settlementHead() function with new comments.
.../contracts-bedrock/test/OutputBisectionGame.t.sol Updated L2_BLOCK_NUMBER, added comments, modified instantiation and function calls, and renamed gameProxy.l1Head() to gameProxy.settlementHead().
.../contracts-bedrock/test/actors/OutputBisectionActors.sol Removed abc() function, added SPDX license, imports, abstract contract GameSolver, and contract HonestGameSolver with new enums, structs, and functions.
.../contracts-bedrock/src/dispute/lib/LibPosition.sol Added import for "DisputeErrors.sol" and new function traceAncestorBounded, updated comments for getMovePosition.
.../contracts-bedrock/src/libraries/DisputeTypes.sol Updated comments from @dev to @notice, introduced LocalPreimageKey library with new constants.
.../contracts-bedrock/test/dispute/lib/LibPosition.t.sol Added internal constant SPLIT_DEPTH and updated test function to use it.

Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 5742b63 to c03c451 Compare November 29, 2023 19:51
@clabby clabby self-assigned this Nov 29, 2023
@clabby clabby added the A-pkg-contracts-bedrock Area: packages/contracts-bedrock label Nov 29, 2023
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from c03c451 to 57ee9eb Compare November 29, 2023 21:34
@clabby clabby changed the base branch from develop to cl/ctb/output-bisection-contract November 29, 2023 22:17
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 57ee9eb to 816653e Compare November 29, 2023 22:17
@clabby clabby mentioned this pull request Nov 29, 2023
7 tasks
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from eeac37f to 7fb1ce7 Compare November 30, 2023 02:17
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch 2 times, most recently from 18ae4c5 to 04352c2 Compare November 30, 2023 02:22
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from 8027f8c to 3203a93 Compare November 30, 2023 21:20
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch 5 times, most recently from 610ec16 to 768b85c Compare November 30, 2023 22:23
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from 9d11a3d to eed6a56 Compare November 30, 2023 22:24
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch 3 times, most recently from 160f094 to 475c02f Compare December 1, 2023 01:20
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from ec771af to ad048af Compare December 1, 2023 01:34
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 475c02f to 5219f71 Compare December 1, 2023 01:35
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from ad048af to 27f6d8e Compare December 1, 2023 01:45
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 5219f71 to 0c6260a Compare December 1, 2023 01:45
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 8c67edb to 3eb23f3 Compare December 1, 2023 17:13
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from 6a20e8d to 263c041 Compare December 1, 2023 20:31
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 3eb23f3 to 34f00b4 Compare December 1, 2023 20:31
@clabby clabby force-pushed the cl/ctb/output-bisection-contract branch from d37b1c3 to 833e355 Compare December 4, 2023 15:18
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 34f00b4 to 43f4ef7 Compare December 4, 2023 15:18
@codecov
Copy link

codecov bot commented Dec 4, 2023

Codecov Report

Merging #8346 (6b64499) into develop (517bf1a) will decrease coverage by 8.94%.
Report is 2 commits behind head on develop.
The diff coverage is 0.00%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8346      +/-   ##
===========================================
- Coverage    34.69%   25.75%   -8.94%     
===========================================
  Files          167      119      -48     
  Lines         7146     4873    -2273     
  Branches      1207     1058     -149     
===========================================
- Hits          2479     1255    -1224     
+ Misses        4516     3513    -1003     
+ Partials       151      105      -46     
Flag Coverage Δ
cannon-go-tests ?
chain-mon-tests 27.14% <ø> (ø)
common-ts-tests ?
contracts-bedrock-tests 20.34% <0.00%> (-0.14%) ⬇️
contracts-ts-tests 12.25% <ø> (ø)
core-utils-tests ?
sdk-next-tests 42.18% <ø> (ø)
sdk-tests 42.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
.../contracts-bedrock/src/dispute/lib/LibPosition.sol 0.00% <0.00%> (ø)
...tracts-bedrock/src/dispute/OutputBisectionGame.sol 0.00% <0.00%> (ø)

... and 49 files with indirect coverage changes

Base automatically changed from cl/ctb/output-bisection-contract to develop December 4, 2023 18:25
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch 2 times, most recently from f9655d0 to 173efbe Compare December 6, 2023 18:56
@clabby clabby marked this pull request as ready for review December 7, 2023 17:47
@clabby clabby requested a review from refcell December 7, 2023 17:47
@clabby clabby requested a review from Inphi December 7, 2023 18:48
@clabby clabby force-pushed the cl/ctb/output-bisection-test-refactor branch from 173efbe to cdca6b7 Compare December 7, 2023 20:01
Copy link
Contributor

@Inphi Inphi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. There are a lotta TODOs that I assume will be handled in another PR.

@clabby clabby added this pull request to the merge queue Dec 7, 2023
Merged via the queue into develop with commit b2a4415 Dec 7, 2023
@clabby clabby deleted the cl/ctb/output-bisection-test-refactor branch December 7, 2023 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-pkg-contracts-bedrock Area: packages/contracts-bedrock

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants