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

Use the latest espresso dev node #303

Merged
merged 2 commits into from
Nov 12, 2024
Merged

Use the latest espresso dev node #303

merged 2 commits into from
Nov 12, 2024

Conversation

ImJeremyHe
Copy link
Member

@ImJeremyHe ImJeremyHe commented Nov 12, 2024

Use the latest espresso-dev-node in Espresso tests.
This is quite necessary since we are still testing our code with an outdated version

This PR:

  • Unpin the espresso-dev-node version
  • Fix an issue that causes the reorg in validation. This was because the goroutine in batch poster didn't check the pending positions and the submitted position before it added one to the queue. In the case where the batch poster processes its task twice while the goroutine in tx-streamer haven't submitted the responding message, duplicated messages would be submitted to hotshot. I do think it is better write the relevent code in batch poster, but it requires a bit refactor and seems not efficient
  • Wait for the L1 node a bit in tests. This seems a potential issue in espresso dev node. If we don't do this, espresso-dev-node will miss some leaves, eventually leading to the failure of fetching the merkle proof in batch poster.

Key places to review:

arbnode/transaction_streamer.go

@sveitser
Copy link
Collaborator

I do think it is better to write the relevent code in batch poster, but it requires a bit refactor and seems not efficient

Can you leave this as a TODO comment in the code so we don't forget about it?

@sveitser sveitser self-requested a review November 12, 2024 12:37
Copy link
Collaborator

@sveitser sveitser left a comment

Choose a reason for hiding this comment

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

LGTM

- Fix an issue that may cause the validation reorg
- Waiting for L1 to be ready can avoid some potential bugs in dev node
@ImJeremyHe ImJeremyHe merged commit bb4641d into integration Nov 12, 2024
11 checks passed
@ImJeremyHe ImJeremyHe deleted the jh/dev-node branch November 12, 2024 14:57
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.

2 participants