Skip to content

Client: Pooled Txs Broadcasting Fixes - #2935

Merged
jochem-brouwer merged 21 commits into
masterfrom
pooled-txs-fixes
Aug 6, 2023
Merged

Client: Pooled Txs Broadcasting Fixes#2935
jochem-brouwer merged 21 commits into
masterfrom
pooled-txs-fixes

Conversation

@acolytec3

@acolytec3 acolytec3 commented Aug 3, 2023

Copy link
Copy Markdown
Contributor

Addresses a failing test noted in #2931 where we should be broadcasting the contents of our transaction pool to newly connected peers. Still WIP

This PR is a continuation of #2931, to test hive and other notes: instructions are there.

This also fixes a performance issue in the sendNewTransactionHashesfunction in txPool where it was awaiting a devp2p message that never gets a response and was causing some devp2p message handlers to time out silently.

@codecov

codecov Bot commented Aug 3, 2023

Copy link
Copy Markdown

Codecov Report

Merging #2935 (9e327c4) into master (845a26b) will decrease coverage by 0.06%.
The diff coverage is 61.70%.

Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
block 88.65% <ø> (-0.01%) ⬇️
blockchain 92.58% <ø> (ø)
client 87.58% <60.86%> (-0.10%) ⬇️
common 98.64% <100.00%> (ø)
ethash ∅ <ø> (∅)
evm 69.95% <ø> (ø)
rlp ?
statemanager 85.16% <ø> (ø)
trie 89.66% <ø> (-0.33%) ⬇️
tx 95.96% <ø> (ø)
util 86.77% <ø> (ø)
vm 79.20% <ø> (ø)

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

@holgerd77

Copy link
Copy Markdown
Member

Actually that’s not only “a failing test” but real misbehavior in our client. 😋

@jochem-brouwer

jochem-brouwer commented Aug 3, 2023

Copy link
Copy Markdown
Member

For completeness for anyone reading this PR: this is relevant and super nice to read https://github.com/ethereum/devp2p/blob/master/caps/eth.md#transaction-exchange

@jochem-brouwer jochem-brouwer mentioned this pull request Aug 4, 2023
3 tasks
@jochem-brouwer jochem-brouwer changed the title Broadcast pooled tx hashes to new peers Fix engine-blobs hive tests Aug 4, 2023
Comment thread packages/client/src/service/service.ts Outdated
@acolytec3
acolytec3 marked this pull request as ready for review August 5, 2023 18:49

@g11tech g11tech left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lgtm 🥳

@jochem-brouwer
jochem-brouwer merged commit 71619d8 into master Aug 6, 2023
@holgerd77
holgerd77 deleted the pooled-txs-fixes branch August 7, 2023 09:48
@holgerd77 holgerd77 changed the title Fix engine-blobs hive tests Client: Pooled Txs Broadcasting Fixes Aug 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants