feat: support in memory lookups in BlockchainProvider2 #9999
Conversation
fgimenez
requested review from
rakita,
joshieDo,
shekhirin,
mattsse and
rkrasiuk
as code owners
| let block = executed_block.block(); | ||
| let receipts = block_state.executed_block_receipts(); | ||
|
|
||
| // assuming 1:1 correspondence between transactions and receipts |
There was a problem hiding this comment.
is this always true, do we have one receipt per tx?
There was a problem hiding this comment.
yeah if you look at the tables they're both keyed by number and the only junction table is the one that maps txhash to number
| let pending = self.inner.in_memory_state.pending.read().clone(); | ||
| let head = self.inner.in_memory_state.head_state(); | ||
|
|
||
| let blocks = self.inner.in_memory_state.blocks.read().clone(); |
There was a problem hiding this comment.
this should be okay because we only expect a few blocks and all of them are just arcs
There was a problem hiding this comment.
yep, added a comment to make it more clear
| if let Some(block_state) = self.canonical_in_memory_state.state_by_hash(*block_hash) { | ||
| return Ok(Some(block_state.block().block().header.header().clone())); |
There was a problem hiding this comment.
doing mem lookups first is the right order because we only remove from mem after we flushed
| for num in start..=end { | ||
| if let Some(block_state) = self.canonical_in_memory_state.state_by_number(num) { | ||
| headers.push(block_state.block().block().header.header().clone()); |
There was a problem hiding this comment.
this is fine for now, but I suppose there's a low chance of a race condition because there can be an update in between these calls, we can track this in new issues though.
There was a problem hiding this comment.
ok, added TODO to keep it in mind
| let start = match range.start_bound() { | ||
| Bound::Included(&n) => n, | ||
| Bound::Excluded(&n) => n + 1, | ||
| Bound::Unbounded => 0, | ||
| }; | ||
| let end = match range.end_bound() { | ||
| Bound::Included(&n) => n, | ||
| Bound::Excluded(&n) => n - 1, | ||
| Bound::Unbounded => self.canonical_in_memory_state.get_canonical_block_number(), | ||
| }; |
There was a problem hiding this comment.
this pattern repeats a few times, should we extract this into a function instead?
something that accepts a closure for the end unbounded case? or just fetches the canonical block number
| for (block_number, block_state) in | ||
| self.canonical_in_memory_state.canonical_chain().enumerate() | ||
| { |
There was a problem hiding this comment.
I'd actually like to move these helper functions into the canonical memory type directly
There was a problem hiding this comment.
ok, I've moved the tx-related functions to CanonicalInMemoryState, should we move anything else?
| // TODO: there might be an update between loop iterations, we | ||
| // need to handle that situation. |
There was a problem hiding this comment.
it might be that we need to move this feature into the canonicalmemory type directly.
will track in a new issue
Closes #9924
I've left all the methods involving
TxNumberuntouched, not sure how to manage them.