-
Notifications
You must be signed in to change notification settings - Fork 20.3k
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
Inconsistency in Transaction r,s,v between code and doc #18152
Comments
We've hit this bug a few times while troubleshooting issues in trufflesuite/ganache-core. I'm a bit troubled that it's been open for so long. If geth is going to go against Postel's Law/the robustness principle and strictly enforce the |
Please fix this and adhere to ethereum's RPC spec. It is currently impossible to test code using go-ethereum against a ganache, which does adhere to the spec. Or change the spec and make |
Just stumbled into this cluster of issues :). My 2c: geth and parity return R, S values as QUANTITY because they are treated as number by the consensus encoding of transactions. Making them DATA in RPC is weird. I understand that this causes frustration for people though. Since ganache seems to be the outlier w.r.t. to encoding them on the server side, I'll send a PR to them to correct the issue there. |
From what I understand, what keeps the Ganache team from switching to quantities isn't a missing PR. They just want the spec to be aligned with the geth&parity implementations. Then they will probably fix it immediately. So following your argument, the RPC spec needs to be fixed for |
I forgot, the commit for such a PR already exists: icherkashin/ganache-core@cb376ee |
Reproducer: https://gist.github.com/fjl/69454fe6b56d04b3c4932d8673dedc63 I've tried this against a couple clients and they all return QUANTITY encoding. I'll bring up the spec change in the next AllCoreDevs call to make sure we're not tripping up anyone else, but it should |
Let's change the spec, we just discussed it on ACD, nobody objected. |
@fjl please update the spec |
I have updated the spec. |
The Transaction struct uses
txdata
to perform unmarchalling with fields r,s,v defined as*big.Int
therefore making them all handled asquantities
(see: Hex Value encoding). In particular leading zeroes are not allowed.At the same time the API documentation (see getTransactionByHash) pretends V is a quantity and R, S are data.
txdata code:
doc:
This makes some tools (e.g. Ganache) producing incorrect values.
Here an example (see the value of
s
):The text was updated successfully, but these errors were encountered: