Skip to content

Separate branch and block processor#8980

Merged
asdacap merged 5 commits into
masterfrom
feature/separate-branch-and-block-processor
Jul 29, 2025
Merged

Separate branch and block processor#8980
asdacap merged 5 commits into
masterfrom
feature/separate-branch-and-block-processor

Conversation

@asdacap

@asdacap asdacap commented Jul 13, 2025

Copy link
Copy Markdown
Contributor
  • Require Refactor/Move block processor in test to DI. #8959
  • Split the branch logic out of IBlockProcessor into IBranchProcessor.
  • This is so that the state handling (in BranchProcessor) is outside, which resolve some double world scoping.
  • This PR only split the class without doing further modification.

Types of changes

What types of changes does your code introduce?

  • Refactoring

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

  • Mainnet can sync normally

Comment thread src/Nethermind/Nethermind.AuRa.Test/AuraBlockProcessorTests.cs
Comment thread src/Nethermind/Nethermind.Blockchain.Test/BlockProcessorTests.cs
WaitForCacheClear();
IReleaseSpec spec = specProvider.GetSpec(suggestedBlock.Header);
(CancellationTokenSource? prewarmCancellation, Task? preWarmTask)
= PreWarmTransactions(suggestedBlock, baseBlock, spec);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pre-warming is per-block it is weird it ended up in Branch part

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea, its probably better made as wrapper to IBlockProcessor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should we do it in this PR? Or have next PR ready before merging this one? Without it, it is hard for me to see it as an upgrade.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Probably next, next PR. Although I won't do anything with prewarmer for now.

@asdacap asdacap force-pushed the test/move-main-processing-to-di branch from 6214f02 to a3828ba Compare July 14, 2025 02:59
@asdacap asdacap force-pushed the feature/separate-branch-and-block-processor branch from 13eab8b to 603fb6d Compare July 14, 2025 03:00
@asdacap asdacap mentioned this pull request Jul 15, 2025
6 tasks

@LukaszRozmej LukaszRozmej left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

waiting for that next PR

@asdacap asdacap force-pushed the test/move-main-processing-to-di branch from a3828ba to 50eb2bc Compare July 21, 2025 13:44
@asdacap asdacap force-pushed the feature/separate-branch-and-block-processor branch from 603fb6d to e8b5c4e Compare July 21, 2025 13:44
@asdacap asdacap force-pushed the test/move-main-processing-to-di branch from 50eb2bc to 823b270 Compare July 23, 2025 12:26
@asdacap asdacap force-pushed the feature/separate-branch-and-block-processor branch from e8b5c4e to c2e847b Compare July 23, 2025 12:26
}
}

// TODO: move to block processing pipeline

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this TODO to move it back 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yea, I just copy it. Not sure what does it suppose to be.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove it

@asdacap asdacap force-pushed the test/move-main-processing-to-di branch from 823b270 to d4b992c Compare July 26, 2025 01:04
Base automatically changed from test/move-main-processing-to-di to master July 26, 2025 06:35
@benaadams

Copy link
Copy Markdown
Member

Conflicts

@asdacap asdacap force-pushed the feature/separate-branch-and-block-processor branch from c2e847b to 9295fce Compare July 27, 2025 01:30

@emlautarom1 emlautarom1 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. There are probably some unused imports that we could remove here and there though.

Comment thread src/Nethermind/Nethermind.Consensus/Processing/IBlockProcessor.cs Outdated
if (_logger.IsTrace) _logger.Trace($"Restored the branch checkpoint - {branchingPointHeader?.ToString(BlockHeader.Format.Short)} | {_stateProvider.StateRoot}");
}

// TODO: block processor pipeline

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.

What about this TODO?

asdacap and others added 3 commits July 29, 2025 14:48
Co-authored-by: Lautaro Emanuel <31224949+emlautarom1@users.noreply.github.com>
…k-processor' into feature/separate-branch-and-block-processor
@asdacap asdacap merged commit c1a6553 into master Jul 29, 2025
77 checks passed
@asdacap asdacap deleted the feature/separate-branch-and-block-processor branch July 29, 2025 07:18
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.

5 participants