events: Fetch metadata at arbitrary blocks #727
Conversation
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
lexnv
force-pushed
the
lexnv/events_md
branch
from
a6be41f
to
381409b
Compare
|
One reason I've been pretty hesitant about introducing this sort of change (and instead just warning users) is that it's quite a lot of overhead to download a big metadata for every single block requested in this way. It's also v hard to cache (and I wouldn't want to cache a ton of metadata blobs per block or something). Given the new RPC API that you're working on too, I wonder whether this sort of change would be compatible or not; I can see us eventually going the way of removing the ability to query previous block numbers at all, and providing either "archive" methods to "get anything you want" or "chainHead" things to follow and be able to work with anything at the head of the chain. |
subxt/src/events/events_client.rs
Outdated
| @@ -73,7 +85,7 @@ where | |||
| .map(|e| e.0) | |||
| .unwrap_or_else(Vec::new); | |||
|
|
|||
| Ok(Events::new(client.metadata(), block_hash, event_bytes)) | |||
| Ok(Events::new(metadata, block_hash, event_bytes)) | |||
There was a problem hiding this comment.
Maybe the solution really is that we allow the user to manually do the above and obtain metadata for the older block if they are in doubt, and ensure that they can construct this Events object.
It might be nicer then to move the event_bytes stuff into Events::new and make that async or something, so that the interface is simpler (they provide metadata and block hash and get events back for it).
There was a problem hiding this comment.
I recall that comparing metadata is super expensive as well, maybe there is some simple way to find which runtime version a block belongs to without using some block explorer API. So how would I know if a block is "old" i.e. need to call metadata? I suppose one could do some simple calculation based on that 14 400 blocks per day is produced * 30 or something.
My point is that comparing metadata is superslow as well IIRC then fetching metadata could make sense to just be naive and always fetch the metadata regardless..... so folks doesn't end up with decoding errors.
However, I agree it's probably better to have different APIs or take Option<Metadata> as input in fn at i.e, if None just use the latest.
There was a problem hiding this comment.
Yep, we should try to minimize the overhead of the metadata fetching. To achieve this, I added a new Events::new_from_client API, the documentation should steer away the users from using this low-level API, but at the same time should provide the level of flexibility a minority of users are after.
Let me know what you think of it! Thanks!
This reverts commit 381409b.
Signed-off-by: Alexandru Vasile <alexandru.vasile@parity.io>
Co-authored-by: James Wilson <james@jsdw.me>
The events are using the client metadata for dynamic decoding.
However, the users are allowed to inspect the events at an arbitrary block.
If the arbitrary block is a block where a runtime upgrade happened that
changed the metadata content, the dynamic decoding of the events
would fail.
To mitigate this scenario, expose the ability to query the metadata at
an arbitrary block, and fetch the metadata of a block when fetching the
block events.
This ensures that dynamic decoding of events will succeed if an arbitrary
block hash is provided.
The client is encouraged to keep the metadata in sync via subscriptions
to the runtime upgrade.
Closes: #725