Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Warn when reading date from non-existing ancient blocks. #7411

Closed
tomusdrw opened this issue Dec 29, 2017 · 5 comments
Closed

Warn when reading date from non-existing ancient blocks. #7411

tomusdrw opened this issue Dec 29, 2017 · 5 comments
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon.

Comments

@tomusdrw
Copy link
Collaborator

Currently after warp sync not all blocks might be present, although you can see the latest state in the UI.

This might be especiall frustrating for developers, since RPC is just simply not returning data for such blocks.

Example:
Fetching all the logs for particular address can render an empty list even though there are clearly blocks with logs out there.

Proposal:

  1. Display a warning if such query is detected (blockNumber < latestBlock && block not found)
  2. Return a "ancient block missing" RPC error.
@tomusdrw tomusdrw added F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon. labels Dec 29, 2017
@5chdn 5chdn modified the milestones: 1.9, 1.10 Jan 2, 2018
@5chdn 5chdn modified the milestones: 1.10, 1.11 Jan 23, 2018
@5chdn 5chdn modified the milestones: 1.11, 1.12 Mar 1, 2018
@5chdn 5chdn modified the milestones: 1.12, 1.13 Apr 24, 2018
@5chdn 5chdn modified the milestones: 2.1, 2.2 Jul 17, 2018
@epheph
Copy link

epheph commented Sep 4, 2018

This issue is really critical for Augur, which sync's the blockchain via a series of getLogs, making the assumption that blocks prior to getBlock("latest").number are available. To illustrate the issue outlined above, here is an inspection of CryptoKitties Birth events from a recently warp-sync'd parity node (warp-barrier: 0x5f8521):

$ curl -s -X POST -H "Content-Type: application/json" --data '{"jsonrpc":"2.0","method":"eth_getLogs","params":[ {
"fromBlock":"0x500000",
"toBlock":  "0x5f853e",
"address":"0x06012c8cf97bead5deae237070f9587f8e7a266d","topics":["0x0a5311bd2a6608f08a180df2ee7c5946819a649b204b554bb8e39825b2c50ad5"]} ],"id": 74}' localhost:8545 | jq .result[].blockNumber | uniq -c
      1 "0x5f8523"
      2 "0x5f8527"
      2 "0x5f8528"
     13 "0x5f852b"
      2 "0x5f852d"
      3 "0x5f852e"
      7 "0x5f852f"
      2 "0x5f8530"
      3 "0x5f8532"
      2 "0x5f8533"
     18 "0x5f8535"
      1 "0x5f8536"
      2 "0x5f8537"
      6 "0x5f8539"
     14 "0x5f853a"
     16 "0x5f853c"
      1 "0x5f853d"
      1 "0x5f853e"

There were a great number of events between 0x500000 and 0x5f8523, but they were simply omitted from this getLogs call, with no indication that anything went wrong inside Parity. Running this query again later, after ancient blocks downloaded, would return different results.

We will add a section to our README advising users not to use Parity with warp-sync (or to use warp-sync only with a specific -warp-barrier prior to any blocks that contain relevant logs), but changing this behavior in Parity seems like the right thing to do. Ideally, an error would be returned if Parity could not satisfy a request, instead of a silent or partial failure.

@udoprog
Copy link
Contributor

udoprog commented Sep 12, 2018

I'll echo my comment on a duplicate issue (#9542) I opened here instead:

After I've set up a new mode with warp and I try to query for historical logs, I get incomplete results. IIUC this is expected since parity by default prunes historical data. However, RPC clients can easily make the assumption that eth_getLogs Returns an array of all logs matching a given filter object. With emphasis on all. An application written with this assumption is very likely to be broken.

We should consider returning an error instead if logs for a queried-for block is not available.

Relates to #9541

@epheph
Copy link

epheph commented Sep 12, 2018

@udoprog The issue isn't that it "prunes" this data in warp mode, the issue is that it has not yet back-filled the data (even though much newer blocks DO have this data). This issue is specific to warp-mode. If you wait long enough, those blocks and logs will become available to your warp-sync'd parity node.

Your conclusion is correct, though: "We should consider returning an error instead if logs for a queried-for block is not available."

@5chdn 5chdn added this to the 2.2 milestone Sep 27, 2018
@5chdn 5chdn modified the milestones: 2.2, 2.3 Oct 29, 2018
@5chdn 5chdn modified the milestones: 2.3, 2.4 Jan 10, 2019
@5chdn 5chdn removed this from the 2.4 milestone Feb 21, 2019
@5chdn 5chdn added this to the 2.5 milestone Feb 21, 2019
@soc1c soc1c modified the milestones: 2.5, 2.6 Apr 2, 2019
@ordian ordian modified the milestones: 2.6, 2.7 Jul 12, 2019
@tomusdrw
Copy link
Collaborator Author

tomusdrw commented Oct 1, 2019

@seunlanlege wasn't that done already?

@seunlanlege
Copy link
Member

yup in #9475

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
F3-annoyance 💩 The client behaves within expectations, however this “expected behaviour” itself is at issue. M6-rpcapi 📣 RPC API. P5-sometimesoon 🌲 Issue is worth doing soon.
Projects
None yet
Development

No branches or pull requests

7 participants