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

Fix JSON encoding of Log & Receipt #73

Merged
merged 2 commits into from
Apr 12, 2018

Conversation

protolambda
Copy link
Contributor

Fix JSON encoding of Log & Receipt; respect data/quantity format types (no leading zeroes for quantities)

As you may already know, the RPC format defines DATA and QUANTITY hex formats:

  • DATA has leading zeroes, padding length to a multiple of 2
  • QUANTITY may not have any leading zeroes

The log/receipt JSON encodings in the Ganache RPC turned out to be invalid sometimes, i.e. leading zeroes in quantities. Go-ethereum failed to decode the eth_getLogs RPC responses, as it strictly enforces the formatting as defined in the spec.

You can find a clear spec of the DATA/QUANTITY properties in the ethereum wiki:

Receipt: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_gettransactionreceipt
Logs: https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_getfilterchanges

I changed the toJSON methods of Log and Receipt to call the to.rpcQuantityHexString and to.rpcDataHexString, to force proper formatting (these methods where already made available for some other RPC formatting).
Note that the previous implementation, to.hex, didn't pad with zeroes, but also didn't enforce the spec. (E.g. call hex("0x0123") for a quantity and you get "0x0123").

@protolambda
Copy link
Contributor Author

protolambda commented Feb 24, 2018

Turns out that there was one test assertion that was made incorrectly. (Travis CI found a tx hash with leading zeroes by chance, which should have been accepted).

Transaction hash output of eth_sendTransaction can have leading zeroes, as it is a DATA type (See https://github.com/ethereum/wiki/wiki/JSON-RPC#eth_sendtransaction)

@protolambda
Copy link
Contributor Author

Changed my old formatting fix to resolve a merge conflict with another similar fix that was pushed to master. Can someone take a look at this PR? It's open for quite a while now.

@benjamincburns
Copy link
Contributor

@protolambda thanks for this contribution, and sorry that it took me so long to circle around to it!

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.

2 participants