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: Replay transactions that can be finalized #9969

Merged

Conversation

roninjin10
Copy link
Contributor

@roninjin10 roninjin10 commented Mar 25, 2024

Fixes bug where finalize being called with a tx that needed to be replayed would fail.

@roninjin10
Copy link
Contributor Author

roninjin10 commented Mar 25, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @roninjin10 and the rest of your teammates on Graphite Graphite

@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from ed619f0 to 6d4d316 Compare March 25, 2024 22:01
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 20f12a6 to d877d67 Compare March 26, 2024 21:05
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 9876b3c to e31bdae Compare March 28, 2024 19:31
Copy link
Contributor

semgrep-app bot commented Mar 29, 2024

Semgrep found 1 iterate-over-empty-map finding:

Iteration over a possibly empty map Kinds. This is likely a bug or redundant code

Ignore this finding from iterate-over-empty-map.

@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch 2 times, most recently from 7be8665 to 03a779a Compare March 29, 2024 22:12
@roninjin10 roninjin10 marked this pull request as ready for review March 30, 2024 01:28
@roninjin10 roninjin10 requested review from a team as code owners March 30, 2024 01:28
Copy link
Contributor

coderabbitai bot commented Mar 30, 2024

Walkthrough

The recent updates focus on enhancing the cross-chain messaging system in the SDK, encompassing job renaming, CircleCI configuration updates, SDK logic refinements, and testing framework adjustments. Notable improvements include better handling of failed messages and support for new blockchain networks, collectively boosting the SDK's reliability and flexibility in cross-chain operations.

Changes

Files Summary
.circleci/config.yml Renamed jobs, updated commands, adjusted job configuration.
packages/sdk/src/.../cross-chain-messenger.ts Deprecated parameters, refactored logic, improved message replay mechanism.
packages/sdk/test-next/README.md Transitioned to using anvil for tests, revised testing methodology.
packages/sdk/test-next/bridgeEcoToken.spec.ts
packages/sdk/test-next/messageStatus.spec.ts
packages/sdk/test-next/proveMessage.spec.ts
Skipped specific test suites for functionalities.
packages/sdk/test-next/failedMessages.spec.ts Added functionality for replaying failed messages in the cross-chain system.
packages/sdk/test-next/testUtils/ethersProviders.ts
packages/sdk/test-next/testUtils/viemClients.ts
Updated providers, introduced new clients for different chains.
.changeset/young-gorillas-roll.md Introduced a patch addressing a bug related to replayable transactions.
bedrock-devnet/devnet/__init__.py Adjusted max_workers parameter in devnet_test function for concurrency control.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

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

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 3b24f68 to bda946a Compare March 30, 2024 01:31
@roninjin10 roninjin10 changed the base branch from develop to sdk-ci-fix March 30, 2024 01:31
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from bda946a to 27a2ddc Compare March 30, 2024 01:32
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 27a2ddc to 3203d84 Compare March 30, 2024 01:35
@roninjin10 roninjin10 removed request for a team and felipe-op March 30, 2024 01:36
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from cd0a066 to 3203d84 Compare March 30, 2024 01:37
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 3203d84 to 9c6ed27 Compare March 30, 2024 01:39
Copy link

codecov bot commented Mar 30, 2024

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes are missing coverage. Please review.

Project coverage is 42.40%. Comparing base (7fd5a19) to head (cc59585).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #9969      +/-   ##
===========================================
- Coverage    42.52%   42.40%   -0.13%     
===========================================
  Files           73       73              
  Lines         4816     4830      +14     
  Branches       759      766       +7     
===========================================
  Hits          2048     2048              
- Misses        2662     2676      +14     
  Partials       106      106              
Flag Coverage Δ
cannon-go-tests 82.29% <ø> (ø)
chain-mon-tests 27.14% <ø> (ø)
common-ts-tests 26.72% <ø> (ø)
contracts-ts-tests 12.25% <ø> (ø)
core-utils-tests 44.03% <ø> (ø)
sdk-tests 40.27% <0.00%> (-0.34%) ⬇️

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

Files Coverage Δ
packages/sdk/src/cross-chain-messenger.ts 45.15% <0.00%> (-1.03%) ⬇️

@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 06e1df2 to 671d9f9 Compare March 30, 2024 01:50
Copy link
Contributor

@smartcontracts smartcontracts left a comment

Choose a reason for hiding this comment

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

Great work, looks good (I think)

Base automatically changed from sdk-ci-fix to develop April 1, 2024 16:15
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 671d9f9 to 1139b74 Compare April 1, 2024 16:26
@roninjin10 roninjin10 added this pull request to the merge queue Apr 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to no response for status checks Apr 1, 2024
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 1139b74 to 8fa3d64 Compare April 1, 2024 18:49
@roninjin10 roninjin10 added this pull request to the merge queue Apr 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 1, 2024
Will Cory added 3 commits April 1, 2024 14:33
comments

Update packages/sdk/src/cross-chain-messenger.ts

feat: Add test

remove stale changeset

fix: remove the stale goerli tests

fix: run pnpm nx build instead of pnpm build

dpeend on pnpm monorepo instead of nx building

wrong ports

http not https

fix: sepolia chain ids

debugging why it's broke

linter: rip

rekick the tests with env variables set to sepolia op rather than mainnet op

fix: Update to a withdrawal that should actually work

fix:test
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from 8fa3d64 to 12fcf66 Compare April 1, 2024 21:38
@roninjin10 roninjin10 added this pull request to the merge queue Apr 1, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Apr 1, 2024
@roninjin10 roninjin10 requested a review from a team as a code owner April 1, 2024 22:24
@roninjin10 roninjin10 requested a review from tynes April 1, 2024 22:24
@roninjin10 roninjin10 force-pushed the 03-25-fix_Replay_transactions_that_can_be_finalized branch from cc59585 to 12fcf66 Compare April 1, 2024 22:35
@roninjin10 roninjin10 added this pull request to the merge queue Apr 1, 2024
Merged via the queue into develop with commit 372bca2 Apr 1, 2024
74 checks passed
@roninjin10 roninjin10 deleted the 03-25-fix_Replay_transactions_that_can_be_finalized branch April 1, 2024 23:04
@github-actions github-actions bot mentioned this pull request Apr 1, 2024
nitaliano pushed a commit that referenced this pull request May 20, 2024
* fix: Replay transactions that can be finalized

comments

Update packages/sdk/src/cross-chain-messenger.ts

feat: Add test

remove stale changeset

fix: remove the stale goerli tests

fix: run pnpm nx build instead of pnpm build

dpeend on pnpm monorepo instead of nx building

wrong ports

http not https

fix: sepolia chain ids

debugging why it's broke

linter: rip

rekick the tests with env variables set to sepolia op rather than mainnet op

fix: Update to a withdrawal that should actually work

fix:test

* speed up ci by installing wait-on

* clean up

* fix: Test running only 1 at a time

---------

Co-authored-by: Will Cory <willcory@Wills-MacBook-Pro.local>
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.

4 participants