Skip to content
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

Emit l2BlockNumber in the Resolved event #226

Merged
merged 4 commits into from
Aug 12, 2024
Merged

Conversation

boyuan-chen
Copy link

@boyuan-chen boyuan-chen commented Aug 8, 2024

📋 Add associated issues, tickets, docs URL here.

Overview

Describe what your Pull Request is about in a few sentences.

To reduce the number of RPC calls needed to retrieve the l2BlockNumber submitted with the state root in the game, I have added the l2BlockNumber and rootClaim to the event.

This allows us to retrieve all necessary information for the withdrawal from our subgraph rather than making RPC calls to get the l2BlockNumber.

Here is how the current implementation works: https://github.com/bobanetwork/graphql-utils/pull/9/files Basically, we need to perform a for loop to find the game where its l2BlockNumber is larger than the target value, and the previous game’s l2BlockNumber is smaller than the target value.

Changes

Describe your changes and implementation choices. More details make PRs easier to review.

  • Add new fields
  • Add tests

Testing

Describe how to test your new feature/bug fix and if possible, a step by step guide on how to demo this.

The tests are added

@codecov-commenter
Copy link

codecov-commenter commented Aug 8, 2024

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 41.31%. Comparing base (bb82a9b) to head (22ba25c).
Report is 3 commits behind head on develop.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop     #226   +/-   ##
========================================
  Coverage    41.31%   41.31%           
========================================
  Files           74       74           
  Lines         5001     5001           
  Branches       785      785           
========================================
  Hits          2066     2066           
  Misses        2828     2828           
  Partials       107      107           
Flag Coverage Δ
cannon-go-tests 81.92% <ø> (ø)
chain-mon-tests 27.14% <ø> (ø)
common-ts-tests 26.72% <ø> (ø)
contracts-ts-tests 12.25% <ø> (ø)
core-utils-tests 44.03% <ø> (ø)
sdk-tests 37.59% <ø> (ø)

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

@boyuan-chen
Copy link
Author

The new FaultDisputeGame is deployed to 0x5D20776Fb8D91181873F210bFb89eAA694217c0b and is set as the implementation contract for game type 0 in our DisputeGameFactoryProxy.

@boyuan-chen
Copy link
Author

I will try to push this to the upstream.

@boyuan-chen
Copy link
Author

I have followed their contribution rule and created a Github issue for the discussion. ethereum-optimism#11409

Copy link

@souhaillM souhaillM left a comment

Choose a reason for hiding this comment

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

LGTM

@boyuan-chen boyuan-chen merged commit e40a8e7 into develop Aug 12, 2024
71 checks passed
@boyuan-chen boyuan-chen deleted the emit-l2blocknumber branch August 12, 2024 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants