Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added zero gasPrice validation for Quorum compatibility mode. #1554

Merged
merged 7 commits into from
Nov 16, 2020

Conversation

mark-terry
Copy link
Contributor

Signed-off-by: Mark Terry mark.terry@consensys.net

PR description

Updates transaction validation to enforce zero gasPrice when isQuorum genesis config is set.

Fixed Issue(s)

Fixes #1551

Changelog

@mark-terry mark-terry self-assigned this Nov 12, 2020
@mark-terry mark-terry added the TeamRevenant GH issues worked on by Revenant Team label Nov 12, 2020
@mark-terry mark-terry requested a review from macfarla November 12, 2020 06:31
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

is it worth putting a TODO to say when quorum implements support for non-zero gas we can remove this restriction?

@macfarla
Copy link
Contributor

as discussed would be good to get the msg about zero gasPrice propagated to RPC response (currently "invalid params")

@MadelineMurray
Copy link
Contributor

is it worth putting a TODO to say when quorum implements support for non-zero gas we can remove this restriction?

Do we have a policy against putting TODO's into the codebase? I'm neutral but if so, we could flag to Nicolas Maurice to ensure we capture this in that work. Let me know if you want to go down that route instead of the TODO.

Signed-off-by: Mark Terry <mark.terry@consensys.net>
Signed-off-by: Mark Terry <mark.terry@consensys.net>
Signed-off-by: Mark Terry <mark.terry@consensys.net>
Signed-off-by: Mark Terry <mark.terry@consensys.net>
Signed-off-by: Mark Terry <mark.terry@consensys.net>
Signed-off-by: Mark Terry <mark.terry@consensys.net>
@mark-terry mark-terry requested a review from macfarla November 15, 2020 13:51
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

overall LGTM - still some instances of Quorum that should be goQuorum

@@ -30,7 +30,8 @@ public static ProtocolSchedule create(
config,
builder -> builder.difficultyCalculator(FixedDifficultyCalculators.calculator(config)),
privacyParameters,
isMetadataEnabled)
isMetadataEnabled,
config.isQuorum())
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be isGoQuorum()

Signed-off-by: Mark Terry <mark.terry@consensys.net>
@mark-terry mark-terry merged commit e783dd7 into hyperledger:master Nov 16, 2020
@mark-terry mark-terry deleted the 1551 branch November 16, 2020 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TeamRevenant GH issues worked on by Revenant Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Quorum-compatible gas price rule
3 participants