Skip to content

QBFT + shanghai support#6353

Merged
matthew1001 merged 22 commits intobesu-eth:mainfrom
matthew1001:qbft-shanghai
Feb 2, 2024
Merged

QBFT + shanghai support#6353
matthew1001 merged 22 commits intobesu-eth:mainfrom
matthew1001:qbft-shanghai

Conversation

@matthew1001
Copy link
Copy Markdown
Contributor

@matthew1001 matthew1001 commented Jan 4, 2024

PR description

Implement shanghai support for QBFT and IBFT chains.

Main areas of change:

  • Previously the code made the assumption that BFT block creators and validators could be created ahead of time, on creation of a new QbftRound/IbftRound, since the next block number was known ahead of time. With number-based milestones such as london this is fine, but with timestamp-based milestones the type of EVM to use for a new round needs to be based on the time that the block is being created. As such, in many places a protocolSchedule is now passed down the stack instead of a blockValidator or a blockImporter to allow these to be created using the protocolSchedule when the timestamp is known.
  • The BftProtocolSchedule was only designed to allow a protocol spec to be requested by block number, so a new method getByBlockNumberAndTimestamp that takes both the block number and timestamp has replaced getByBlockNumber, and determines the fork spec based on whichever milestone is most recent.

I don't think any docs changes are required since we don't currently make any mention of shanghai in the docs today.

Fixed Issue(s)

Fixes #5446

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 4, 2024

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

@matthew1001 matthew1001 force-pushed the qbft-shanghai branch 11 times, most recently from 7bb70c0 to c1f726f Compare January 8, 2024 17:55
@matthew1001 matthew1001 force-pushed the qbft-shanghai branch 5 times, most recently from 0ba4bda to 3e4e682 Compare January 11, 2024 11:35
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@matthew1001 matthew1001 force-pushed the qbft-shanghai branch 5 times, most recently from f73e9e0 to 8195395 Compare January 16, 2024 11:06
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
…age validator creation

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@matthew1001 matthew1001 marked this pull request as ready for review January 17, 2024 17:43
@matthew1001 matthew1001 changed the title WIP: QBFT + shanghai support QBFT + shanghai support Jan 17, 2024
Signed-off-by: Matt Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
* @return the protocol spec for that block number and timestamp
*/
public ProtocolSpec getByBlockNumber(final long number) {
public ProtocolSpec getByBlockNumberAndTimestamp(final long number, final long timestamp) {
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.

I think this getting a protocol spec by either the block number or timestamp. Better name would be getByBlockNumberOrTimestamp

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.

Done - updated to getByBlockNumberOrTimestamp

LOG.trace("Importing block with extraData={}", extraData);
final BlockImporter blockImporter =
((BftProtocolSchedule) protocolSchedule)
.getByBlockNumberAndTimestamp(
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.

I think this could just use protocolSchedule.getByBlockHeader(blockToImport.getHeader()) instead. It will look up protocol schedules by both block timestamps and block numbers.

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.

Fixed


final BlockValidator blockValidator =
((BftProtocolSchedule) protocolSchedule)
.getByBlockNumberAndTimestamp(
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.

Similar to the IbftRound I think you could use the existing protocolSchedule.getByBlockHeader method here instead

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.

Fixed

}
LOG.trace("Importing proposed block with extraData={}", extraData);
final BlockImporter blockImporter =
((BftProtocolSchedule) protocolSchedule)
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.

Same in the IbftRound think we get away with using the protocolSchedule.getByBlockHeader method here

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.

Fixed

matthew1001 and others added 5 commits January 30, 2024 13:44
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew.whitehead@kaleido.io>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@matthew1001
Copy link
Copy Markdown
Contributor Author

Thanks for the latest comments @jframe. I think the latest commits resolve them - let me know what you think.

Signed-off-by: Matt Whitehead <matthew.whitehead@kaleido.io>
matthew1001 and others added 2 commits February 1, 2024 10:42
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Comment on lines +99 to +101
checkState(
protocolSchedule instanceof BftProtocolSchedule,
"Wrong class type for protocol schedule, requires BftProtocolSchedule");
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.

nit: don't need this check anymore

matthew1001 and others added 2 commits February 2, 2024 08:51
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
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.

Shanghai support for QBFT/IBFT

2 participants