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(mining): Advertise mined blocks #9176

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

oxarbitrage
Copy link
Contributor

Motivation

When blocks are mined and we are not close to the tip the mined block do not get advertised to the network.

Close #8909

Solution

Introduce a channel with a sender part used by the rpc method submit block to add a signal when a block is submitted and accepted. The receiver part of the channel is used by the block gossip task to advertise the block.

Tests

  • A submitblock_channel was added to check the gossip task can receive mined blocks.
  • A check was added to existing test nu6_funding_streams_and_coinbase_balance to make sure the sender part of the channel is working.

PR Author's Checklist

  • The PR name will make sense to users.
  • The PR provides a CHANGELOG summary.
  • The solution is tested.
  • The documentation is up to date.
  • The PR has a priority label.

PR Reviewer's Checklist

  • The PR Author's checklist is complete.
  • The PR resolves the issue.

@oxarbitrage oxarbitrage added A-network Area: Network protocol updates or fixes A-rpc Area: Remote Procedure Call interfaces A-concurrency Area: Async code, needs extra work to make it work properly. P-Medium ⚡ labels Jan 29, 2025
@oxarbitrage oxarbitrage requested review from a team as code owners January 29, 2025 00:08
@oxarbitrage oxarbitrage requested review from upbqdn and removed request for a team January 29, 2025 00:08
@arya2 arya2 self-requested a review January 30, 2025 01:45
Copy link
Contributor

@arya2 arya2 left a comment

Choose a reason for hiding this comment

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

This looks great, thank you for these changes. It's more complicated than what I had in mind, but it makes it easy to now handle mined blocks differently and broadcast them to all ready peers instead of a third of them.

I opened a suggestion PR, #9183, for your review. Ideally this PR could advertise mined blocks immediately and return early from check_synced_to_tip() on test networks, but I see no blockers to merging this as-is.

use crate::components::sync::PEER_GOSSIP_DELAY;

// Custom in-memory writer to capture logs
struct TestWriter(Arc<Mutex<Vec<u8>>>);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you leave a note about the conditional compilation issue or how this is used?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-concurrency Area: Async code, needs extra work to make it work properly. A-network Area: Network protocol updates or fixes A-rpc Area: Remote Procedure Call interfaces P-Medium ⚡
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Zebra does not advertise mined blocks in some cases
2 participants