-
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: preload cachedreads with tip state #5804
Conversation
…CachedReads preload
… noop function on_new_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.
great start so far!
we likely need more changes in the BasicPayloadGenerator
which now needs a CachedReads and needs to use that to the instantiate a job if the parent_hash matches
…und Events with Unpin
…y to resolve compiler errors
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.
functionality wise this is exactly what we want!
this just needs a few more changes
sorry about the conflicts -.- |
5f7c206
to
2adb0c8
Compare
crates/payload/basic/src/lib.rs
Outdated
fn on_new_state(&mut self, new_state: CanonStateNotification) { | ||
if let Some(committed) = new_state.committed() { | ||
let mut cached = CachedReads::default(); | ||
|
||
// extract the state from the notification and put it into the cache | ||
let new_state = committed.state(); | ||
for (addr, acc) in new_state.accounts_iter() { | ||
if let Some(acc) = acc { | ||
let storage = if let Some(acc) = new_state.state().account(&addr) { | ||
acc.storage.iter().map(|(key, slot)| (*key, slot.present_value)).collect() | ||
} else { | ||
Default::default() | ||
}; | ||
cached.insert_account(addr, acc.clone(), storage); | ||
} | ||
} | ||
|
||
self.pre_cached = Some(PrecachedState { block: committed.tip().hash, cached }); | ||
} | ||
} |
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.
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.
In general this is okay, but would add helper function to BundleStateWithReceipts
to make it more obvious, maybe something like fn bundle_account_iter()
.
To point out, the bundle state contains only written accounts, not the reads, but it is still better than nothing.
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.
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.
lgtm
d99e662
to
7d6eeea
Compare
draft PR for cache reads preload