-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: blockchain provider tree state integration #9518
feat: blockchain provider tree state integration #9518
Conversation
2d3b219
to
91ec045
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this makes sense, I have mostly doc nits!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, this is a very good start,
we're still missing some things but we can iterate on it.
I think from tree perspective it would be great if this state is managed separately as CanonicalState { Rwlocks }
so that the tree updates only the canonical blocks in there
/// State composed of a block hash, and the receipts, state and transactions root | ||
/// after executing it. | ||
#[derive(Debug, PartialEq, Eq)] | ||
pub struct State { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we also need the SealedBlock
+ the actual state so we can perform eth_call, trace etc. on this but this is a good start
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you mean, including a field with the SealedBlock
? the rest of fields could be obtained from it or am I missing something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, what we need to access is:
block,receipts+state at this block
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, will propose it in a followup
Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>
Co-authored-by: Dan Cline <6798349+Rjected@users.noreply.github.com>
9a445d0
to
78edbaf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think for provider access we should put all of this a separate container type that can be easily shared:
struct InMemoryState {
blocks: HashMap<Hash, Arc<State>>
numbers: HashMap<u64, Hash>,
pending: Option<State>,
}
we need to share access to this with rpc etc. I think we should consider maintaining this as a separate instance in the tree that only tracks the canonical chain. because otherwise we'd need locks around all of these:
blocks_by_hash: HashMap<B256, ExecutedBlock>,
/// Executed blocks grouped by their respective block number. blocks_by_number: BTreeMap<BlockNumber, Vec<ExecutedBlock>>,
but maybe that's fine? at least for now
/// State composed of a block hash, and the receipts, state and transactions root | ||
/// after executing it. | ||
#[derive(Debug, PartialEq, Eq)] | ||
pub struct State { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, what we need to access is:
block,receipts+state at this block
makes sense, will add all this in a followup |
Towards #8747
Scaffolds
InMemoryState
trait and implements it forEngineApiTreeHandlerImpl
.TreeState
is updated to includepending
andcurrent_head
data.For now in
BlockchainProvider
there are only references to the field that will provide this state.