feat(block-producer): promote redesign#541
feat(block-producer): promote redesign#541Mirko-von-Leipzig merged 16 commits intonext-block-producerfrom
Conversation
No longer required as of rust 1.81. The other instances have been removed in a separate PR onto next.
And separate runtime from configuration items by moving mempool out of the struct.
This required renaming existing BlockProver -> BlockProverKernel. Also separated runtime items from configuration by moving out mempool.
Still need to get rid of the generic.
This also removes the traits, which breaks the current BatchBuilder.
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left some comments inline. The main one is about removing the code that use to check if any of the notes became authenticated before batch construction. It seems to me that we might still need this code. But also, we can bring it back in a follow-up PR.
Once this is merged, we should try to run client integration tests against next-block-producer branch. cc @igamigo.
| batch_limit: usize, | ||
| block_limit: usize, | ||
| state_retention: usize, |
There was a problem hiding this comment.
I wonder if we should have something like MempoolConfig or MempoolOptions struct to encapsulate these. But this can also be done later on.
Mirko-von-Leipzig
added
the
no changelog
|
I managed to run the node successfully; though this is without traffic so its really just exercising empty blocks. |
bobbinth
left a comment
bobbinth
left a comment
There was a problem hiding this comment.
Looks good! Thank you! I left a couple a question and a comment for the future inline.
The only other comment is #541 (comment) - but we should probably merge this PR and then address this comment in a follow-up PR.
2 participants
This PR replaces the current FIFO block-producer with the mempool/graph based redesign.
I attempted to keep the diff manageable by not refactoring unnecessary parts i.e. I've largely only replaced and then refactored/cleaned up until clippy stopped yelling. This means that the code is not in a normalized state.
The rework attempts to avoid generics and traits where possible. The current implementation had a lot of these to allow for dependency injection and testing, however these are generally leaky abstractions and I'd prefer to avoid them where possible. In that vein, this PR removes a lot of these since they're no longer used.
txqueueandstate_viewmodules are entirely gone.I've also simply commented out the current block and batch test suites. Some of these no longer apply; some of these will need to be refactored or adjusted.
Note that this compiles and the (included) tests still pass; however this has not yet been run at all. I'll update the status here if I get it to run for a bit, however I think this PR is valuable even without it working properly. Any (larger) changes will make this PR even more unreviewable imo. And follow-up fixes/improvement PRs should be preferred once the major issues have been identified.
This PR renders #185 and #191 obsolete.
This PR also completes some tasks in #514.