Skip to content

Conversation

@petreze
Copy link
Contributor

@petreze petreze commented Dec 10, 2025

Description:
This PR adds support for the new Block Node response BehindPublisher.

  • It includes changes to the implementation in BlockNodeConnection#onNext() method to handle the new response
  • Added a new metric for tracking the block number of the latest BehindPublisher received
  • Adjusted the block-stream.json dashboard file accordingly
  • Unit test fixes
  • Additions and changes to the test-client in order to support testing the new response

Related issue(s):

Fixes #

Notes for reviewer:

! ---DO NOT MERGE BEFORE RELEASE 0.25.0 of the Block Node --- !

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Petar Tonev <[email protected]>
Signed-off-by: Petar Tonev <[email protected]>
@petreze petreze self-assigned this Dec 10, 2025
@petreze petreze requested review from a team as code owners December 10, 2025 15:38
@petreze petreze requested a review from rbarker-dev December 10, 2025 15:38
@lfdt-bot
Copy link

lfdt-bot commented Dec 10, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

*
* @param nodeBehind the BehindPublisher response received from the block node
*/
private void handleBlockNodeBehind(@NonNull final BehindPublisher nodeBehind) {
Copy link
Contributor

Choose a reason for hiding this comment

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

BehindPublisher is not an EndStream condition and thus the connection should not be closed and started again with the new block. Additionally, there needs to be a condition to respond with an ERROR if the block requested is above the latest block produced by the CN. Basically the handling of BehindPublisher should be the exact same as handling a ResendBlock message - just logging/metrics changed.

Copy link
Contributor Author

@petreze petreze Dec 10, 2025

Choose a reason for hiding this comment

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

The implementation is inspired from these changes in the protocol document in BN repo.
It says that the block node no longer closes the stream when behind, but
waits for the Publisher to either stream an earlier block or
send "too far behind". By sending too far behind, we are essentially closing the stream and rescheduling

Copy link
Contributor

Choose a reason for hiding this comment

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

       if (blockBufferService.getBlockState(restartBlockNumber) != null) {
            logger.info("{} Block node reported it is behind. Will restart stream at block {}.", this, restartBlockNumber);

            closeAndRestart(restartBlockNumber);
        } else { ... }

The if block above calls #closeAndRestart(long). That is the part I am talking about. The document was updated to indicate this:

      BlockNode -->> Publisher: Respond with BehindPublisher and specify the latest completed block (L)
      Publisher-->>BlockNode: Send from block L+1 or send EndStream(Too Far Behind) and retry with exponential backoff

Only if the block isn't available should the connection be closed, otherwise the connection should not be closed and we should just turn around and start sending the requested block, like we do for ResendBlock

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.

4 participants