Skip to content

Qbft spike#67

Closed
jframe wants to merge 31 commits intoConsensys:mainfrom
jframe:qbft_spikev2
Closed

Qbft spike#67
jframe wants to merge 31 commits intoConsensys:mainfrom
jframe:qbft_spikev2

Conversation

@jframe
Copy link
Contributor

@jframe jframe commented Mar 25, 2025

Spike of integrating Qbft into Maru

DO NOT MERGE

kvStoreAccessor.get(Schema.BlockRootByBlockNumber, blockNumber).getOrNull()?.let { blockRoot ->
return getSealedBeaconBlock(blockRoot)
}
return null
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think you need a separate return statement. I believe you should be able to pop it before kvStoreAccessor.get(...

private val proposerSelector =
object : ProposerSelector {
override fun getProposerForBlock(header: BeaconBlockHeader): SafeFuture<Validator> =
override fun selectProposerForRound(header: BeaconBlockHeader): SafeFuture<Validator> =
Copy link
Collaborator

Choose a reason for hiding this comment

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

FYI, I changed it to accept ConsensusRoundIdentifier instead in another PR. here is the relevant discussion.

I see you've added the required getSealedBeaconBlock(blockNumber: ULong) to the BeaconChain, so we're aligned here

Copy link
Collaborator

Choose a reason for hiding this comment

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

Now I see that you've added another ProposerSelector accepting ConsensusRoundIdentifier, so this one can be removed now

apply plugin: 'java-library'

repositories {
mavenLocal()
Copy link
Collaborator

@Filter94 Filter94 Mar 26, 2025

Choose a reason for hiding this comment

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

Should this go into maru/buildSrc/src/main/groovy/maru.kotlin-common-minimal-conventions.gradle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was planning to remove this before any code is merged in. Just a temporary until we have a Besu release out with the changes I need.

messageFactory,
)

val transitionLogger = QbftValidatorModeTransitionLoggerAdapter()
Copy link
Collaborator

Choose a reason for hiding this comment

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

What kind of transition is it? From a non-validator to a validator and vise versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a transition to a new block. I don't think we need this interface; we should be able to remove it and just add a subscriber to the new block event in Besu. If I can do that, I'll remove this as I don't like this QbftValidatorModeTransitionLogger.

@jframe jframe closed this Apr 22, 2025
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.

2 participants

Comments