Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Fix estimate gas RPC failing for clique when no blocks have been created #1528

Merged

Conversation

jframe
Copy link
Contributor

@jframe jframe commented Jun 6, 2019

PR description

Fixes issue when using Clique, if the chain height is still at 0 then the estimateGas rpc fails with a internal server error.

This change special cases block 0 so we return address 0 for the proposer in this case instead of failing with an IllegalArgumentException as the genesis block does not have proposer seals.

Fixed Issue(s)

Copy link
Contributor

@ajsutton ajsutton left a comment

Choose a reason for hiding this comment

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

LGTM.

@@ -23,6 +23,7 @@
import java.util.function.Supplier;

public class CliqueBlockHashing {
private static final Address ADDRESS_ZERO = Address.extract(Hash.ZERO);
Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect we should just introduce Address.ZERO = Address.fromHexString('0x0') in the Address class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -48,6 +49,9 @@ public static Hash calculateDataHashForProposerSeal(
*/
public static Address recoverProposerAddress(
final BlockHeader header, final CliqueExtraData cliqueExtraData) {
if (header.getNumber() == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (header.getNumber() == 0) {
if (header.getNumber() == BlockHeader.GENESIS_BLOCK_NUMBER) {

Please don't ever hard code 0 as the genesis block number. One day we're going to have to support variable genesis block numbers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jframe jframe requested review from rain-on, CjHare and pinges June 6, 2019 00:54
@@ -48,6 +47,9 @@ public static Hash calculateDataHashForProposerSeal(
*/
public static Address recoverProposerAddress(
final BlockHeader header, final CliqueExtraData cliqueExtraData) {
if (header.getNumber() == BlockHeader.GENESIS_BLOCK_NUMBER) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Personally, I'd move this inside the "isPresent" check - just to minimise the conditionals executed on every check ... its probably a minor benefit at best.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@jframe jframe merged commit 1b25f6e into PegaSysEng:master Jun 6, 2019
@jframe jframe deleted the estimateGas_fails_on_clique_0_block branch June 6, 2019 02:41
iikirilov pushed a commit to Puneetha17/pantheon that referenced this pull request Jun 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants