Fix check for consensus algorithm to address broken RAFT timestamp in console #1616
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Use loaded rpc modules to deduce which consensus algorithm is running. * Previous use of nodeInfo isn't valid anymore as those fields were removed in Geth 1.10 and the protocol manager that ensured the field is set doesn't exist anymore
|
baptiste-b-pegasys
approved these changes
Mar 2, 2023
|
Shouldn't we be looking to bring back the changes in https://github.com/ConsenSys/quorum/blob/v21.4.2/eth/handler.go#L1005-L1035 With the way you've implemented I think you can have the wrong consensus reported if you haven't enabled the rpc module. |
|
@antonydenyer Agree, but in such cases, the console is no more broken than what it is currently.. I'm not against bringing back the original changes, I made the assumption that nodes not running with rpc modules enabled is likely is a remote possibility given most IBFT/RAFT info & administrative APIs require access to the rpc methods. Will take a stab at what you've suggested next week. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Invalid Datefor RAFT blocksThe updated code in Geth 1.10 removed
Consensusfield from Node information and the protocol manager that ensured the field is set doesn't exist anymore. So the output ofadmin_nodeInfono longer contains information about the consensus algorithm. Due to this, the adjustment to RAFT block timestamps is not applied.In this PR, I propose using the output of
rpc_modulesAPI call to determine which consensus algorithm is in force. I made sure I am consistent with the previous method used to determine the consensus algorithm.Code references:
Previous code that used to set & expose the consensus algo: https://github.com/ConsenSys/quorum/blob/v21.4.2/eth/handler.go#L1005-L1035
Refactored code in Geth 1.10 train that no longer exposes consensus algo: https://github.com/ConsenSys/quorum/blob/master/eth/protocols/eth/handler.go#L134-L154