chore: refactor fetcher to extend kona's fetcher instead of copying all of its logic #1
Conversation
bin/host/src/extended_fetcher/mod.rs
Outdated
| /// Set the last hint to be received. | ||
| pub fn hint(&mut self, hint: &str) { | ||
| trace!(target: "fetcher", "Received hint: {hint}"); | ||
| self.last_hint = Some(hint.to_string()); |
There was a problem hiding this comment.
since we use self.fetcher.get_preimage(key), 10 lines below, we should pass the last_hint into the inner fetcher.
There was a problem hiding this comment.
oh snap that's a good point... kona's fetcher's hint is no longer updated now right?
Hmm not sure how to best structure this then, we can talk tomorrow.
There was a problem hiding this comment.
Changed the logic quite a bit, see: a7723ee
|
Looking at alloy vs op-alloy, it looks that when they add a new type inside enum, the whole enum is replicated |
samlaf
force-pushed
the
chore--extend-konas-fetcher-instead-of-forking
branch
from
848e10e to
31e02bf
Compare
- Logic was previously routing to both inner fetcher and eigenda fetcher, which was a bug - Renamed ExtendedFetcher -> FetcherWithEigenDASupport - Also tested against devnet successfully
bin/host/src/extended_fetcher/mod.rs
Outdated
| result = self.get_preimage_altda(key) => result, | ||
| result = self.fetcher.get_preimage(key) => result, | ||
| match self.last_eigenda_hint.as_ref() { | ||
| Some(hint) => self.get_preimage_altda(key, hint).await, |
There was a problem hiding this comment.
name change get_preimage_eigend?
bin/host/src/extended_fetcher/mod.rs
Outdated
| } | ||
| } | ||
|
|
||
| async fn get_preimage_altda(&self, key: B256) -> Result<Vec<u8>> { | ||
| trace!(target: "extended fetcher", "Pre-image requested. Key: {key}"); | ||
| async fn get_preimage_altda(&self, key: B256, hint: &str) -> Result<Vec<u8>> { |
There was a problem hiding this comment.
change the name into get_preimage_eigenda?
bin/host/src/extended_fetcher/mod.rs
Outdated
|
|
||
| if hint_type == ExtendedHintType::AltDACommitment { | ||
| if hint_type == ExtendedHintType::EigenDACommitment { |
There was a problem hiding this comment.
should we add else part that panic! when accidentally get other hint
bin/host/src/extended_fetcher/mod.rs
Outdated
| _ => { | ||
| self.fetcher.hint(hint); | ||
| // get_preimage will fetch from the underlying fetcher when last_eigenda_hint = None | ||
| self.last_eigenda_hint = None; |
There was a problem hiding this comment.
just to confirm last_hint is a private field, so it is safe to set last_eigenda_hint to be None
Currently our fetcher is copy pasted from kona's fetcher with an added 20ish lines to support eigenda hints.
This PR refactors hokulea's fetcher into an
ExtendedFetcherstruct which wraps kona's fetcher instead, which simplifies the code and makes it easy to inherit any code modifications to kona's logic by simply bumping the client's version.There has to be a better way to architect this wrapping... see TODO comments in the code.
Tests
I tried running this against the devnet the same way we did in our kona-fork repo (I also added steps in README in this PR), but it doesn't work (even the current main branch). How did you run this @bxue-l2 ?