Skip to content
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

op-program: Populate preimage store #5452

Merged
merged 3 commits into from
Apr 17, 2023
Merged

Conversation

ajsutton
Copy link
Contributor

@ajsutton ajsutton commented Apr 14, 2023

Description

Implements a pre-fetcher that takes in the hints from HintReader.NextHint and request for preimages from OracleServer.NextPreimageRequest and then populates the KV store by fetching required data.

Modifies the op-program main to use this fetcher and an in-memory KV store as the backing for oracle requests. This makes the fetching mode run using the same key/value requests and access as non-fetching mode would just using hints to fetch the required data on demand.

Additional context

Builds on #5463 and #5465

Metadata

@changeset-bot
Copy link

changeset-bot bot commented Apr 14, 2023

⚠️ No Changeset found

Latest commit: 373c5aa

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@netlify
Copy link

netlify bot commented Apr 14, 2023

Deploy Preview for opstack-docs canceled.

Name Link
🔨 Latest commit 373c5aa
🔍 Latest deploy log https://app.netlify.com/sites/opstack-docs/deploys/643dacc78aa5fe0008c58a21

@ajsutton ajsutton changed the base branch from develop to op-program-pre-image-comms April 14, 2023 05:42
@ajsutton ajsutton force-pushed the aj/fpp-store-fetched-data branch from d383e68 to d99b6f6 Compare April 14, 2023 05:45
Base automatically changed from op-program-pre-image-comms to develop April 14, 2023 14:28
@ajsutton ajsutton changed the base branch from develop to aj/block-info-rlp April 17, 2023 00:38
@ajsutton ajsutton changed the base branch from aj/block-info-rlp to aj/fpp-test-stubs April 17, 2023 00:46
@ajsutton ajsutton force-pushed the aj/fpp-store-fetched-data branch from 09a410e to 9e3a7f1 Compare April 17, 2023 00:53
@ajsutton ajsutton marked this pull request as ready for review April 17, 2023 01:08
@ajsutton ajsutton requested a review from a team as a code owner April 17, 2023 01:08
@ajsutton ajsutton requested review from tynes and removed request for a team April 17, 2023 01:08
@ajsutton ajsutton requested review from Inphi and protolambda April 17, 2023 03:45
Copy link
Contributor

@protolambda protolambda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR looks good, but I think we want to use the L1/L2 interfaces with error handling in the prefetcher, since it's running in the host.

op-program/host/prefetcher/prefetcher.go Outdated Show resolved Hide resolved
@protolambda
Copy link
Contributor

Alternatively we could also add error-returns to the "fetching oracle" code, if there's functionality there that we want to share rather than plugging directly into a L1/L2 client from sources package. And then define wrappers in the client package that map an error-returning oracle to one that escalates them to critical errors.

Base automatically changed from aj/fpp-test-stubs to develop April 17, 2023 09:13
@protolambda
Copy link
Contributor

As per Slack discussion: updated the prefetcher to take an interface matching the L1/L2 rpc bindings (and added debug bindings for the MPT-node / code fetching). This way the prefetcher can handle errors and bubble them up to the get-preimage caller (who can then shut down the program from the host side etc.).

protolambda
protolambda previously approved these changes Apr 17, 2023
…e oracle

op-program now uses an in-memory key-value store as the backing for all data with the same key-based retrieval and deserialization as in non-fetching mode.
@ajsutton ajsutton force-pushed the aj/fpp-store-fetched-data branch from 09a1965 to 4739b0f Compare April 17, 2023 20:09
@tynes tynes removed their request for review April 17, 2023 20:13
@protolambda protolambda dismissed their stale review April 17, 2023 20:14

adding changes

@OptimismBot OptimismBot merged commit 62b9850 into develop Apr 17, 2023
@OptimismBot OptimismBot deleted the aj/fpp-store-fetched-data branch April 17, 2023 21:31
@mergify
Copy link
Contributor

mergify bot commented Apr 17, 2023

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Apr 17, 2023

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify mergify bot removed the on-merge-train label Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants