[OLD] feat(client): providers generic over oracles#295
[OLD] feat(client): providers generic over oracles#295zobront wants to merge 16 commits intoop-rs:mainfrom
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files☔ View full report in Codecov by Sentry. |
| } else if #[cfg(target_os = "zkvm")] { | ||
| #[doc = "Concrete implementation of the [BasicKernelInterface] trait for the `zkvm` target architecture."] | ||
| pub type ClientIO = crate::zkvm::io::ZkvmIO; |
There was a problem hiding this comment.
I would like to get rid of the need for any of these changes, but putting them in here for now so it compiles and works. @clabby let me know if you're ok merging like this until we figure out a solution, or if you'd prefer to wait.
There was a problem hiding this comment.
I think we definitely need this otherwise we don't have the native file descriptors (like the filesystem) so it won't compile
There was a problem hiding this comment.
Totally right. Clabby and I brainstormed a few alternatives to remove the need for it, since we don't actually use the io system so it's not doing anything.
He's thinking through it more, but my perspective is that best option would be to create a no op BKI implementation and organize the feature flags and defaults in such a way that it typically defaults to NativeIO, but that we can import with default features off to fall back to the no op.
crates/executor/src/lib.rs
Outdated
| pub fn new( | ||
| config: &'a RollupConfig, | ||
| parent_header: Sealed<Header>, | ||
| // TODO: @Clabby - would you rather refactor this to assume Provider implements F + H and just pass one argument? |
There was a problem hiding this comment.
Question for you @clabby: We can keep this as fetcher F and hinter H and just use the provider for both, or we can refactor the executor to expect the provider to handle everything. Which would you prefer?
Same question with the TrieDB, which is created in this function and also expects separate fetcher and hinter.
|
I cannot get CI to run, so going to close this PR and reopen a new one to see if it fixes... |
THIS PR WOULD NOT TRIGGER GITHUB CI. NEW PR #336 REPLACED IT.