Skip to content

fix: correctly set the recovered mempool parts#2163

Merged
rach-id merged 4 commits intomainfrom
rachid/wait-before-responding-to-haves
Jul 9, 2025
Merged

fix: correctly set the recovered mempool parts#2163
rach-id merged 4 commits intomainfrom
rachid/wait-before-responding-to-haves

Conversation

@rach-id
Copy link
Member

@rach-id rach-id commented Jul 9, 2025

Improves on: #2161

@rach-id rach-id self-assigned this Jul 9, 2025
@rach-id rach-id added this to core/app Jul 9, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in core/app Jul 9, 2025
}
// check if we have any transactions that are in the compact block
go blockProp.recoverPartsFromMempool(cb)
blockProp.recoverPartsFromMempool(cb)
Copy link
Member Author

Choose a reason for hiding this comment

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

this is beneficial because we don't want to act on haves before we know what we have locally and what we should request

Copy link
Member

Choose a reason for hiding this comment

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

iirc didn't we add the coroutine part as mempools we're blocking or slow?

could that happen now? If the mempool was very congested, would that slow consensus?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the goroutine was added as a premature optimization. Even if the mempool is very congested, retrieving transactions from the mempool should be fast enough and if we have any local parts, it's better to retrieve them rather than request them. If this process is slow, I guess the right is to speed it up instead of making it run in parallel.

Copy link
Member Author

Choose a reason for hiding this comment

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

also, if the mempool is very congested, we just won't find any transactions in it that are good for us, and this will exist fast.

@rach-id rach-id merged commit 2607b6a into main Jul 9, 2025
20 checks passed
@github-project-automation github-project-automation bot moved this from Needs Triage to Done in core/app Jul 9, 2025
@rach-id rach-id deleted the rachid/wait-before-responding-to-haves branch July 9, 2025 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants