core, eth, ethapi, les, light, rpc: Support Block Number or Hash on state-related RPCs.

[ryanschneider]

For awhile now I've been thinking it'd be nice if one could specify a block hash instead of number for the default block param. This PR is an attempt at supporting that in geth.

And it turned out that just a week or so before @charles-cooper created an EIP for more or less the same thing:

https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1898.md

As mentioned in #19394 there's a couple scenarios where I could see this being useful:

As a DApp developer, I might have logic like:

result := eth_call({....}, "latest")
// do some potentially slow processing w/ result
// send another RPC a couple seconds later
balance := eth_getBalance(0xsomeAdress, "latest")

The issue above is that "latest" could refer to totally different blocks. So next one tries:

latest := eth_getBlockByNumber("latest", false)
result := eth_call({....}, latest.Number)
// do some potentially slow processing w/ result
// send another RPC a couple seconds later
balance := eth_getBalance(0xsomeAdress, latest.Number)

But even still, because of reorgs, block number 0x7654321 might refer to two different blocks depending on when it is used. So my PR allows:

latest := eth_getBlockByNumber("latest", false)
result := eth_call({...}, latest.Hash)
// do some potentially slow processing w/ result
// send another RPC a couple seconds later
balance := eth_getBalance(0xsomeAdress, latest.Hash)

So now you are sure you are referring to the same block in both calls.

This also becomes very useful for Exchanges or Infura-a-likes, basically anyone w/ enough RPC load or need for HA that they need to run more than one node, since the chances of 0x7654321 pointing to two different blocks on two different nodes is even higher. And if one of the nodes hasn't seen that block hash yet, the caller gets an error rather than a 0-values result. So, this actually skirts the breaking behavior of #18346 by continuing to behave like 1.8.x for block numbers, but returns explicit errors for block hashes.

Anyways, I'd love to get @karalabe and @holiman 's feedback on this, and see if a) there's any interest in accepting such a feature and b) if so, would you support me proposing it as an EIP?

In addition, to be compliant w/ EIP-1898 and the discussion in #19394 this PR accepts the hash as either a string or a {blockHash:,canonical:} object.

[holiman]

There's a PR to that effect. Don't have the pr number right now (on mobile), but it's fairly easy to do, but we need to specify semantics -- see my comment on that ticket

[ryanschneider]

Interesting, if you get a chance please send the PR number. I just did a quick search but didn’t see anything obvious (on mobile too atm).

…

On Fri, Apr 12, 2019 at 2:04 PM Martin Holst Swende < ***@***.***> wrote: There's a PR to that effect. Don't have the pr number right now (on mobile), but it's fairly easy to do, but we need to specify semantics -- see my comment on that ticket — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#19459 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADREIbTibpitshXQhyHNlMZudYEMgqJks5vgPTOgaJpZM4cs5Iz> .

[holiman]

Ah,see #19394

[ryanschneider]

Thanks! FWIW this PR implements "Return the data if we have the state, but return error if we don't".
I'll comment on that issue.

[ryanschneider]

@holiman @charles-cooper I've force pushed a new version that supports all the variants discussed in #19394:

• block number as string
• block hash as string
• block number as JSON object
• block hash as JSON object, w/ or w/o canonical boolean

I actually didn't see an existing way to determine if a given hash is canonical or not so had to expose a GetCanonicalHash method on the BlockChain interface.

[charles-cooper]

@ryanschneider cool! I think that covers all the use cases discussed :). By the way, hope you can accept my apologies if I came off as a little harsh the other day in #19394 (comment). I do see the issue in the geth codebase with backwards compatibility now. I think what I will do is update EIP1898 to support the 'canonical' field as in this PR, and then assume that the behavior for parsing block hash/number as string is geth-specific behavior.

[ryanschneider]

By the way, hope you can accept my apologies if I came off as a little harsh the other day in #19394 (comment).

Not at all! I agree that the EIP should focus on clarity, so I think accepting hash as a string should be client-specific.

I think the main thing now is to get some feedback on my implementation, primarily:

• The JSON parsing
• The canonical checks

I'm going to remove the draft tag now as I think it's ready for preliminary review.

[ryanschneider]

Root comment updated and draft flag removed.

[ryanschneider]

BTW looks like the AppVeyor build failed due to unrelated swarm tests timing out, any way to get that to re-run?

[holiman]

Don't worry about Appveyor, that's very flaky

[charles-cooper]

@ryanschneider @holiman I updated the EIP draft with the results of our discussion. The only thing I feel is still materially differing from the spec is to use requireCanonical instead of canonical. I think requireCanonical is clearer since "canonical": false makes it seem like the client specifically wants a non-canonical block instead of just relaxing the input validation allow a non-canonical block.

[ryanschneider]

Thanks @charles-cooper I force pushed a new version that uses requireCanonical, agree that canonical: false sounds strange.

[ryanschneider]

Actually just realized this PR is still for 1.8.x, rebasing my code against master now but running into what I think are some legit bugs w/ the graphQL resolvers that I think my code will actually allow us to fix, so digging into that first.

[ryanschneider]

Closing in favor of #19491 since this is against 1.8.x branch.