Skip to content

Implement "pending" for qbft_getValidatorsByBlockNumber#6436

Merged
matthew1001 merged 6 commits intobesu-eth:mainfrom
matthew1001:pending-qbft-validators
Jan 30, 2024
Merged

Implement "pending" for qbft_getValidatorsByBlockNumber#6436
matthew1001 merged 6 commits intobesu-eth:mainfrom
matthew1001:pending-qbft-validators

Conversation

@matthew1001
Copy link
Copy Markdown
Contributor

@matthew1001 matthew1001 commented Jan 19, 2024

PR description

This PR adds support for "pending" as a string in qbft_getValidatorsByBlockNumber.

It returns the validators for the block that is in the process of being mined.

In certain stalled-chain scenarios where a new validator has been voted in but not enough validators are online to mine the next block, neither qbft_getPendingVotes nor qbft_getValidatorsByBlockNumber("latest") are useful since the vote is no longer pending, and the "latest" block is not the one that is failing to be mined. Similarly, qbft_getValidatorsByBlockNumber(<next-block-number>) cannot be used because the next block doesn't exist yet.

"pending" already exists as a block string, but isn't implemented by most JSON/RPC methods. This is only the 2nd method that implements it I believe, but it seems like a valid interpretation of "pending" in this case. All other JSON/RPC methods will continue to revert to "latest" if "pending" is specified.

Fixed Issue(s)

No issue raised for this.

Doc changes required

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 19, 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 changed the title Implement 'pending' for qbft_getValidatorsByBlockNumber Implement "pending" for qbft_getValidatorsByBlockNumber Jan 19, 2024
@matthew1001 matthew1001 added doc-change-required Indicates an issue or PR that requires doc to be updated enhancement New feature or request labels Jan 19, 2024
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@matthew1001 matthew1001 force-pushed the pending-qbft-validators branch from 538f2fd to 2f999fa Compare January 19, 2024 10:14
Copy link
Copy Markdown
Contributor

@jframe jframe left a comment

Choose a reason for hiding this comment

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

This use of pending makes sense to me. Just a minor code suggestion

matthew1001 and others added 3 commits January 30, 2024 09:06
…ft/jsonrpc/methods/QbftGetValidatorsByBlockNumber.java

Co-authored-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@matthew1001 matthew1001 enabled auto-merge (squash) January 30, 2024 09:09
Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
@matthew1001 matthew1001 merged commit 3646d78 into besu-eth:main Jan 30, 2024
jflo pushed a commit to jflo/besu that referenced this pull request Jan 30, 2024
* Implement 'pending' for qbft_getValidatorsByBlockNumber

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update change log

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

* Update consensus/qbft/src/main/java/org/hyperledger/besu/consensus/qbft/jsonrpc/methods/QbftGetValidatorsByBlockNumber.java

Co-authored-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>

* Refactoring

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>

---------

Signed-off-by: Matthew Whitehead <matthew1001@gmail.com>
Signed-off-by: Matt Whitehead <matthew1001@hotmail.com>
Co-authored-by: Jason Frame <jasonwframe@gmail.com>
Signed-off-by: Justin Florentine <justin+github@florentine.us>
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants