bug fix "The value HexBytes() is 31 bytes, but should be 32"

[namuyan]

I get transaction from Nekonium, Ether fork coin, I find error.

ValueError: The value HexBytes('0xdf44753580e848a8e2db1b8bb5feacd2e07b72f10fb2064f7a0aff70510872') is 31 bytes, but should be 32

gnekonium output the data, I find "s" is 62 letters

{ blockHash: "0xd1717626d448a942b4c831f9bd669b9a78b04d6c5387195f193fb3c88bc364d0", blockNumber: 594388, from: "0xf177a0bfc6bb19cc468d555c6a517471e7d59586", gas: 121001, gasPrice: 4000000000, hash: "0xd82f3f61280b52b8168e5b8bc133a0bbe76b80e3c8f39c850094fd0667d8460e", input: "0x", nonce: 12, r: "0x704fa964b5db28cda3dae9f2d1cad32595e0b208a3e589c4ab3ee6223a23d851", s: "0xdf44753580e848a8e2db1b8bb5feacd2e07b72f10fb2064f7a0aff70510872", to: "0x14713a2f4f1234b7bad2ba12ede7ee842e928666", transactionIndex: 0, v: "0x26", value: 100000000000000000 }

So I escape the error by appending zero to head.

[carver]

Thanks for the report and hunting down the issue!

This particular solution is problematic, because it defeats the error checking that we want in place. The question is: are we expecting the wrong kind of value, or does the client that you're using return an incorrect value? (what client are you using?)

Unfortunately, the values r, s, v are not defined in the rpc spec. So we can't precisely answer the question without both: determining the "naturally correct" answer and investigating what current clients do.

Based on how web3.js was handling them, it looks like s and r are expected to be 32-byte data (already 0-padded). On the other hand, ethereum-keys seems to think of s and r as integers, which implies that it should not be 0-padded. I'm looking further into what parity and geth do now...

[carver]

ethereum-keys, geth, and parity seem to agree that s and r should be treated as integers rather than binary data. Since the direct implementation and major clients agree, I'm satisfied that it should be treated as an integer instead of bytes.

Could you remove the change to line 119, and instead change the to_hexbytes call on r and s to include variable_length=True?

https://github.com/namuyan/web3.py/blob/57b96ea8f4a148bd10e56736f49e37e4235888b0/web3/middleware/pythonic.py#L146..L148

[carver]

I also created a suggestion for web3.js to follow the same pattern: web3/web3.js#1170

[namuyan]

Thank you for support.
I use https://github.com/nekonium/go-nekonium .

You can get transaction dict by

curl -X POST --data '{"jsonrpc":"2.0","method":"eth_getTransactionByHash","params":["0xd82f3f61280b52b8168e5b8bc133a0bbe76b80e3c8f39c850094fd0667d8460e"],"id":1}' http://nukowallet.com:8293

result

{
   "blockHash": "0xd1717626d448a942b4c831f9bd669b9a78b04d6c5387195f193fb3c88bc364d0",
   "blockNumber": "0x911d4",
   "from": "0xf177a0bfc6bb19cc468d555c6a517471e7d59586",
   "gas": "0x1d8a9",
   "gasPrice": "0xee6b2800",
   "hash": "0xd82f3f61280b52b8168e5b8bc133a0bbe76b80e3c8f39c850094fd0667d8460e",
   "input": "0x",
   "nonce": "0xc",
   "to": "0x14713a2f4f1234b7bad2ba12ede7ee842e928666",
   "transactionIndex": "0x0",
   "value": "0x16345785d8a0000",
   "v": "0x26",
   "r": "0x704fa964b5db28cda3dae9f2d1cad32595e0b208a3e589c4ab3ee6223a23d851",
   "s": "0xdf44753580e848a8e2db1b8bb5feacd2e07b72f10fb2064f7a0aff70510872"
}

I don't use values r, s. I will edit code.

[carver]

Welcome to the repository, and thanks for your contribution @namuyan !

[carver]

Regarding your question on the linked issue:

https://github.com/pipermerriam/web3.py/blob/master/web3/account.py#L181..L182
Do we need to change this lines to?
'r': HexBytes...

HexBytes is already variable length (different from to_hexbytes in the middleware that this PR already handles). It's a little weird that odd-length hex values will end up with a leading zero, but I don't think it will get in anyone's way. So I think we can leave it as is.