Add timestamp to meta block #3738
Conversation
mangas
force-pushed
the
filipe/block-timestamp
branch
7 times, most recently
from
c90a78d to
3c0f2fb
Compare
mangas
force-pushed
the
filipe/block-timestamp
branch
from
3c0f2fb to
167861a
Compare
mangas
force-pushed
the
filipe/block-timestamp
branch
from
167861a to
96a3955
Compare
|
Has this been tested on a firehose block stream? |
|
@leoyvens on a firehose stream is the thing I was testing locally but the blocks look the same on the database. The firehose ingestor I didn't test because as far as I can tell it is not wired (for eth) but the types are the same so should be fine as well |
mangas
force-pushed
the
filipe/block-timestamp
branch
from
8297527 to
48c0a31
Compare
mangas
force-pushed
the
filipe/block-timestamp
branch
from
48c0a31 to
b8b71c7
Compare
mangas
force-pushed
the
filipe/block-timestamp
branch
from
b8b71c7 to
088f567
Compare
mangas
force-pushed
the
filipe/block-timestamp
branch
2 times, most recently
from
e3c50b0 to
b4c40b7
Compare
| fn block_number( | ||
| &self, | ||
| hash: &BlockHash, | ||
| ) -> Result<Option<(String, BlockNumber, Option<String>)>, StoreError>; |
There was a problem hiding this comment.
This tuple is getting too complex, seems worth introducing a struct. Also there might be a better name than block_number since this also returns other info.
There was a problem hiding this comment.
I'm not sure this refactoring should go on the same PR though. I'm happy to make the changes and raise a new PR for it
| fn block_number_with_timestamp( | ||
| &self, | ||
| block_hash: &BlockHash, | ||
| ) -> Result<Option<(BlockNumber, Option<String>)>, StoreError>; |
There was a problem hiding this comment.
Since we have a struct BlockPtrTs (not a fan of the name but ok), we could also return it here.
There was a problem hiding this comment.
it's an package internal name, I went with I thought was an accurate description 😛 happy to rename if you have suggestions
| fn block_number( | ||
| &self, | ||
| hash: &BlockHash, | ||
| ) -> Result<Option<(String, BlockNumber, Option<String>)>, StoreError> { | ||
| let conn = self.get_conn()?; |
There was a problem hiding this comment.
This would ideally be asyncified to use with_conn.
There was a problem hiding this comment.
this one is also used in a few places so I'd rather take that as a separate PR
mangas
force-pushed
the
filipe/block-timestamp
branch
from
b4c40b7 to
07fb4fc
Compare
* Add timestamp to meta block * expose timestamp on meta block
Fixes: #3060
This change should add a timestamp for EVM chains, should work well for both RPC and firehose ingestors.
The fact the query depends heavily on the format of the block means it will not work for any block format that doesn't add the expected timestamp field inside a top level block field.
To solve this further we could ensure that all block formats have the necessary timestamp or alternative we could extract/duplicate that field outside the block and run some migrations. The downside of the migration approach is that it needs to be communicated more widely. This migration would also be potentially long due to the size of the block cache tables.