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.

Sign up for GitHub

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

Jump to bottom

Disable txpool if SequencerUrl is set #7513

Merged
merged 5 commits into from
Oct 11, 2024
Merged

Disable txpool if SequencerUrl is set #7513

merged 5 commits into from
Oct 11, 2024

Conversation

benaadams
Copy link
Member

Changes

  • Disable txpool if SequencerUrl is set or it does a bunch of busy work on NewHead/HeadChange, AddPeer

Types of changes

What types of changes does your code introduce?

  • Optimization

Testing

Requires testing

  • No

@LukaszRozmej LukaszRozmej mentioned this pull request Oct 4, 2024
Merged
16 tasks
@benaadams
Copy link
Member Author

Looks good

jmederosalvarado
jmederosalvarado reviewed Oct 10, 2024
View reviewed changes
Copy link
Member

@jmederosalvarado jmederosalvarado left a comment

Choose a reason for hiding this comment

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

Overall good. I left a question. Let's make sure we run a node and send it a tx before merging.

src/Nethermind/Nethermind.Optimism/Rpc/RegisterOptimismRpcModules.cs Show resolved Hide resolved
@LukaszRozmej
Copy link
Member

Overall good. I left a question. Let's make sure we run a node and send it a tx before merging.

I think the tx will be ignored as the tx pool won't do anything with it

@benaadams
Copy link
Member Author

Overall good. I left a question. Let's make sure we run a node and send it a tx before merging.

I think the tx will be ignored as the tx pool won't do anything with it

Should forward it to the sequencerUrl; which is why no point in having the TxPool doing work on every new head

@emlautarom1
Copy link
Contributor

Tested in OP-Sepolia through eth_sendRawTransaction and I'm getting responses from the sequencer as expected.

@LukaszRozmej LukaszRozmej merged commit 43b9af9 into master Oct 11, 2024
73 checks passed
@LukaszRozmej LukaszRozmej deleted the disable-txpool branch October 11, 2024 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmederosalvarado jmederosalvarado jmederosalvarado left review comments

@LukaszRozmej LukaszRozmej LukaszRozmej approved these changes

@emlautarom1 emlautarom1 emlautarom1 approved these changes

@deffrian deffrian Awaiting requested review from deffrian

@flcl42 flcl42 Awaiting requested review from flcl42

Assignees
No one assigned
Labels
optimism
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benaadams @LukaszRozmej @emlautarom1 @jmederosalvarado