Skip to content

Fix Block Tag Formatter #1550

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix Block Tag Formatter #1550

wants to merge 1 commit into from

Conversation

willrnch
Copy link

@willrnch willrnch commented May 10, 2021 •
edited
Loading

eth_call (eth_getBalance, etc...) accepts a blockTag as its second parameter.
The block tag can be the string latest or pending, a block number or a block hash.
If it’s a block number, it must be a non zero padded hex string.
Otherwise, geth returns this error:

0x0829ee2 is padded. Should be 0x829ee2.

$ curl -X "POST" "https://geth-rinkeby/" \
     -H 'Content-Type: application/json' \
     -d $'{
  "id": 1,
  "method": "eth_getBalance",
  "jsonrpc": "2.0",
  "params": [
    "0x02680640DEbf91dFF2cF51131545FCBbbEbCA0b5",
    "0x0829ee2"
  ]
}'

{"jsonrpc":"2.0","id":1,"error":{"code":-32602,"message":"invalid argument 1: hex number with leading zero digits"}}

If it’s a block hash, if must be a 32 bytes hex string (zero padded if needed). If not padded, geth will return the following error:

$ curl -X "POST" "https://geth-rinkeby/" \
     -H 'Content-Type: application/json; charset=utf-8' \
     -d $'{
  "id": 1,
  "method": "eth_getBalance",
  "jsonrpc": "2.0",
  "params": [
    "0x02680640DEbf91dFF2cF51131545FCBbbEbCA0b5",
    "0x281610a62b7bc11190c76e2774375feb7205baba91a3a4ff8a6a65139208cab"
  ]
}'

{
  "jsonrpc": "2.0",
  "id": 1,
  "error": {
    "code": -32602,
    "message": "invalid argument 1: hex number \u003e 64 bits"
  }
}

0x281610a62b7bc11190c76e2774375feb7205baba91a3a4ff8a6a65139208cab was trimmed. Should be 0x0281610a62b7bc11190c76e2774375feb7205baba91a3a4ff8a6a65139208cab.

When passing a block hash as the block tag (to eth_call or eth_getBalance), the zeros in the hex value are trimmed (and produce the previous error).
This pull request fixes this issue by considering that if the block tag is an hex string, it will be considered as the block hash and won't be trimmed. If it's a number, it will be converted to a trimmed hex string.

Reproduce

const provider = new JsonRpcProvider(
  'https://geth-rinkeby/'
);
const balance = await provider.getBalance(
  '0x02680640DEbf91dFF2cF51131545FCBbbEbCA0b5',
  '0x0281610a62b7bc11190c76e2774375feb7205baba91a3a4ff8a6a65139208cab'
);

@willrnch
Copy link
Author

Can you please have a look @ricmoo?

@ricmoo
Copy link
Member

ricmoo commented May 16, 2021

Good catch! I’ll add that to the v5.2 changes which should be coming out this week. :)

@ricmoo ricmoo added minor-bump Planned for the next minor version bump. on-deck This Enhancement or Bug is currently being worked on. labels May 16, 2021
@ricmoo
Copy link
Member

ricmoo commented May 16, 2021

The fix looks good, but I don’t think block tag is allowed to be a block hash. It must be a tag or quantity. So, using hexValue is the right thing to do in the case of a hex string.

@ricmoo
Copy link
Member

ricmoo commented May 16, 2021

Actually, I think the original code is correct. Hex values like 0x042 should be trimmed to 0x42.

At best, I could for now imply in the docs that block hashes aren’t supported, but first I can test out all the existing backends and if they seem to support it, I can add an additional || isHexString(blockTag, 32) to the verbatim condition.

@ricmoo ricmoo added the investigate Under investigation and may be a bug. label May 16, 2021
@LogvinovLeon
Copy link

Please notice, that there is one more way to specify block tag: https://eips.ethereum.org/EIPS/eip-1898
It's currently supported by Geth and Infura

@ricmoo
Copy link
Member

ricmoo commented Oct 4, 2021

I don't think I will attempt to support requesting by blockhash as a valid BlockTag in v5. Documentation I am finding from the backends docs that they don't seem to indicate explicit support. The EIP is also listed as stagnant.

I'll create an "Idea" discussion to discuss it for v6.

@ricmoo
Copy link
Member

ricmoo commented Oct 4, 2021

See #2135.

I'm going to close this PR for now though. Any assistance exploring whether Alchemy, Pocket, etc. support it though is welcome. :)

@ricmoo ricmoo closed this Oct 4, 2021
@ricmoo ricmoo added fixed/complete This Bug is fixed or Enhancement is complete and published. and removed on-deck This Enhancement or Bug is currently being worked on. fixed/complete This Bug is fixed or Enhancement is complete and published. labels Oct 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
investigate Under investigation and may be a bug. minor-bump Planned for the next minor version bump.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@willrnch @ricmoo @LogvinovLeon