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

Block pipeline #3727

Draft
wants to merge 21 commits into
base: develop
Choose a base branch
from
Draft

Conversation

clemahieu
Copy link
Contributor

This is a refactor of the block processor which breaks the block processing steps in to small components focused on a specific part of processing blocks.

nano/lib/stats.hpp Outdated Show resolved Hide resolved
nano/node/blockprocessor.cpp Outdated Show resolved Hide resolved
nano/node/blockprocessor.cpp Outdated Show resolved Hide resolved
nano/node/blockprocessor.cpp Outdated Show resolved Hide resolved
nano/node/block_pipeline/stateless_signer_filter.hpp Outdated Show resolved Hide resolved
else
{
// If there is no previous block, the balance must already be 0 and therefore cannot be a send
if (block->previous ().is_zero ())
Copy link
Contributor

Choose a reason for hiding this comment

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

I do not know if this check is complete or makes sense.
At this point, the block could be even a receive, since at this point we wouldn't know that a receive doesn't have a corresponding send block.
It coudl also be a change block. I am not sure what this check gives us.

#include <nano/secure/ledger.hpp>
#include <nano/secure/store.hpp>

void nano::block_pipeline::block_position_filter::sink (std::shared_ptr<nano::block> block, std::shared_ptr<nano::block> previous)
Copy link
Contributor

Choose a reason for hiding this comment

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

This filter mentions that it checks something to do with epoch going backwards.
I do not understand what the means.
However, I see no reference to any epoch blocks, is that a sign that something is missing?
Also I see no reference to work difficulties and epoch v2 is about work difficulty.

This simply seems to check that legacy blocks never follows a state block.

break;
case nano::stat::detail::determinate_signer_filter_reject:
res = "determinate_signer_filter_reject";
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

We could add a default case with an debug_assert(0) to catch any missed cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we turned on warning to error conversion for missing case statements the CI could catch it at compile time.

nano/node/block_pipeline/determinate_signer_filter.cpp Outdated Show resolved Hide resolved
@clemahieu clemahieu force-pushed the block_pipeline branch 6 times, most recently from 47bf309 to 9c7ea8a Compare March 4, 2022 13:15
…essing steps in to small components focused on a specific part of processing blocks.
# Conflicts:
#	nano/lib/stats.cpp
#	nano/lib/stats.hpp
#	nano/node/blockprocessor.cpp
#	submodules/cpptoml
#	submodules/diskhash
#	submodules/flatbuffers
#	submodules/gtest
#	submodules/lmdb
#	submodules/miniupnp
#	submodules/phc-winner-argon2
… may miss a trigger notification while being processed by the unchecked_map thread.
…ck sequence. This test stops block processing so no blocks will be inserted which means a sequence of block would have gaps that the pipeline would reject for gaps.
# Conflicts:
#	nano/lib/stats.cpp
#	nano/lib/stats.hpp
# Conflicts:
#	nano/secure/ledger.cpp
#	nano/secure/ledger.hpp
Copy accounts locally within signature verification.
…ng it separately in epoch_restrictions_filter.
@clemahieu clemahieu added this to the V26.0 milestone Apr 20, 2023
@clemahieu clemahieu mentioned this pull request Nov 14, 2023
@qwahzi qwahzi modified the milestones: V26.0, Todo Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Research for Future
Development

Successfully merging this pull request may close these issues.

3 participants