-
Notifications
You must be signed in to change notification settings - Fork 897
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
update json-rpc responses for 1559 #2222
update json-rpc responses for 1559 #2222
Conversation
@@ -198,6 +206,11 @@ public String getGasLimit() { | |||
return gasLimit; | |||
} | |||
|
|||
@JsonGetter(value = "gasTarget") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to add tests for impacted RPC methods in the case of an eip1559 transaction ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking the same thing actually. This PR is probably best to wait until next week to merge, so we can implement whatever the consensus is from the london readiness call where json-rpc format will be discussed (hopefully finalized)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added serialization tests for the added fields, and removed gasTarget in favor of gasLimit. should be gtg
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just an error message to update
throw new InvalidJsonRpcParameters("gasPrice cannot be used with baseFee or feeCap"); | ||
&& (callParams.getMaxFeePerGas().isPresent() | ||
|| callParams.getMaxPriorityFeePerGas().isPresent())) { | ||
throw new InvalidJsonRpcParameters("gasPrice cannot be used with baseFee or maxFeePerGas"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's an old issue but we need to replace baseFee in the message (maxPriorityFeePerGas ?)
…ck config fixes #2192 Signed-off-by: garyschulte <garyschulte@gmail.com>
…regression Signed-off-by: garyschulte <garyschulte@gmail.com>
… field names Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: Gary Schulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
…less Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte <garyschulte@gmail.com>
* address assumption that if 1559 is enabled there is a london fork block config fixes hyperledger#2192 Signed-off-by: garyschulte <garyschulte@gmail.com> * omit null chainId from transaction json serialization, and fix merge regression Signed-off-by: garyschulte <garyschulte@gmail.com> * update json-rpc responses for 1559, use current feeCap and gasPremium field names Signed-off-by: garyschulte <garyschulte@gmail.com> * add pr number to changelog Signed-off-by: garyschulte <garyschulte@gmail.com> * fix transaction decoder Signed-off-by: Gary Schulte <garyschulte@gmail.com> * rebase and fix merge conflicts Signed-off-by: garyschulte <garyschulte@gmail.com> * remove null chainId from json-rpc test fixtures Signed-off-by: garyschulte <garyschulte@gmail.com> * the great 1559 rename Signed-off-by: garyschulte <garyschulte@gmail.com> * re-remove gasTarget Signed-off-by: garyschulte <garyschulte@gmail.com> * add serialization tests for 1559 transaction fields Signed-off-by: garyschulte <garyschulte@gmail.com> * add json property ordering annotations to Transaction*Result and spotless Signed-off-by: garyschulte <garyschulte@gmail.com> * rebase on master, update error message feedback from Karim Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: garyschulte garyschulte@gmail.com
PR description
Fixed Issue(s)
fixes #2215
fixes #2287
Changelog