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

fix: fix CI after 4844 merge #198

Merged
merged 13 commits into from
May 20, 2024

Conversation

welkin22
Copy link
Contributor

Description

After merging the code from upstream v1.7.2, our CI pipeline failed to pass. This is due to differences in the code on both sides. I modified the test code to make the CI pipeline pass again.
In addition, there are three points worth noting:

  1. Due to the differences in code for blob support between the BSC and ETH networks, we do not need a new beaconClient. The previous code only returned nil, but could not pass lint. I directly deleted the relevant code to pass lint.
  2. The upstream v1.7.2 code relies on geth version v1.101308.3, while our op-geth repository initially merged code from version v1.101308.2. Although this difference does not affect the core code logic, it will cause the opbnb test code to fail to compile. I temporarily modified the relevant test code to make it compile, and will eliminate the version difference through fix: cherry-picked from upstream code op-geth#109. Once that PR is merged, I will fix the relevant test code for this PR.
  3. The e2e tests related to failure proof are very difficult to fix. We need to thoroughly study the code for failure proof before fixing these e2e test cases. For now, I can only skip them.

Rationale

Modify the code to make the CI pipeline pass.

Example

none

Changes

Notable changes:

  • Deleted beaconClient related code
  • Modify the code to make it pass lint, UT, and e2e cases.

@github-actions github-actions bot requested review from bnoieh and krish-nr May 13, 2024 13:31
@owen-reorg owen-reorg merged commit c7157c0 into bnb-chain:develop May 20, 2024
11 checks passed
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.

3 participants