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

chore: refactor fetcher to extend kona's fetcher instead of copying all of its logic #1

Merged
merged 9 commits into from
Dec 17, 2024
8 changes: 7 additions & 1 deletion bin/client/justfile
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,16 @@ run-client-native-against-devnet verbosity='' block_number='' rollup_config_path

if [ -z "{{block_number}}" ]; then
BLOCK_NUMBER=$(cast block finalized --json --rpc-url $L2_RPC | jq -r .number | cast 2d)
if [ $BLOCK_NUMBER -eq 0 ]; then
echo "No finalized blocks found on L2 chain. If devnet was just started, wait a bit and try again..."
echo "You can run the following command to check the latest finalized block."
echo "cast block finalized --json --rpc-url $L2_RPC | jq -r .number | cast 2d"
exit 1
fi
else
BLOCK_NUMBER={{block_number}}
fi

set -x
just run-client-native $BLOCK_NUMBER \
$L1_RPC $L1_BEACON_RPC $L2_RPC $ROLLUP_NODE_RPC \
$ROLLUP_CONFIG_PATH {{verbosity}}
Expand Down
70 changes: 40 additions & 30 deletions bin/host/src/extended_fetcher/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,13 @@ use std::sync::Arc;
use tokio::sync::RwLock;
use tracing::{error, info, trace, warn};

/// The [Fetcher] struct is responsible for fetching preimages from a remote source.
/// The [FetcherWithEigenDASupport] struct wraps and extends kona's [Fetcher] struct with the ability
/// to fetch preimages from EigenDA.
/// TODO: Kona is planning to change the fetcher interface to allow registering extra hints
/// without needing a new type. We will probably want to switch when possible.
/// See https://github.com/anton-rs/kona/issues/369
#[derive(Debug)]
pub struct ExtendedFetcher<KV>
pub struct FetcherWithEigenDASupport<KV>
where
KV: KeyValueStore + ?Sized,
{
Expand All @@ -25,10 +29,10 @@ where
/// The eigenda provider
eigenda_blob_provider: OnlineEigenDABlobProvider,
/// The last hint that was received. [None] if no hint has been received yet.
last_hint: Option<String>,
last_eigenda_hint: Option<String>,
}

impl<KV> ExtendedFetcher<KV>
impl<KV> FetcherWithEigenDASupport<KV>
where
KV: KeyValueStore + ?Sized,
{
Expand All @@ -42,7 +46,7 @@ where
fetcher,
kv_store,
eigenda_blob_provider,
last_hint: None,
last_eigenda_hint: None,
}
}

Expand All @@ -65,33 +69,41 @@ where
fetcher,
kv_store,
eigenda_blob_provider,
last_hint: None,
last_eigenda_hint: None,
}
}

/// 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());
pub fn hint(&mut self, hint: &str) -> Result<()> {
trace!(target: "fetcher_with_eigenda_support", "Received hint: {hint}");
let (hint_type, _) = ExtendedHint::parse(hint)?.split();
// We route the hint to the right fetcher based on the hint type.
match hint_type {
ExtendedHintType::EigenDACommitment => {
self.last_eigenda_hint = Some(hint.to_string());
}
_ => {
self.fetcher.hint(hint);
// get_preimage will fetch from the underlying fetcher when last_eigenda_hint = None
self.last_eigenda_hint = None;
Copy link
Collaborator

Choose a reason for hiding this comment

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

just to confirm last_hint is a private field, so it is safe to set last_eigenda_hint to be None

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

correct

}
}
Ok(())
}

/// Get the preimage for the given key.
/// TODO: there's probably a much nicer way to write this function.
/// Fetcher was not written with extensibility in mind, so we have to do this,
/// but there might be ways to change the Fetcher's API to make this easier (eg by making prefetch public).
/// Fetch the preimage for the given key. The requested is routed to the appropriate fetcher
/// based on the last hint that was received (see hint() above).
/// ExtendedFetcher -> get_preimage_altda -> prefetch that only understands altda hints
/// \-> Fetcher -> get_preimage -> prefetch that understands all other hints
/// Both get_preimages just for loop until the preimage is found. Their hint coverage is mutually exclusive,
/// so only one of them will ever return, hence the tokio::select!.
pub async fn get_preimage(&self, key: B256) -> Result<Vec<u8>> {
tokio::select! {
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,
Copy link
Collaborator

Choose a reason for hiding this comment

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

make sense

Copy link
Collaborator

Choose a reason for hiding this comment

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

name change get_preimage_eigend?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

None => self.fetcher.get_preimage(key).await,
}
}

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>> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

change the name into get_preimage_eigenda?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

trace!(target: "fetcher_with_eigenda_support", "Pre-image requested. Key: {key}");

// Acquire a read lock on the key-value store.
let kv_lock = self.kv_store.read().await;
Expand All @@ -101,12 +113,10 @@ where
drop(kv_lock);

// Use a loop to keep retrying the prefetch as long as the key is not found
while preimage.is_none() && self.last_hint.is_some() {
let hint = self.last_hint.as_ref().expect("Cannot be None");

while preimage.is_none() {
if let Err(e) = self.prefetch(hint).await {
error!(target: "fetcher", "Failed to prefetch hint: {e}");
warn!(target: "fetcher", "Retrying hint fetch: {hint}");
error!(target: "fetcher_with_eigenda_support", "Failed to prefetch hint: {e}");
warn!(target: "fetcher_with_eigenda_support", "Retrying hint fetch: {hint}");
continue;
}

Expand All @@ -119,22 +129,22 @@ where

/// Fetch the preimage for the given hint and insert it into the key-value store.
async fn prefetch(&self, hint: &str) -> Result<()> {
trace!(target: "fetcher", "prefetch: {hint}");
trace!(target: "fetcher_with_eigenda_support", "prefetch: {hint}");
let hint = ExtendedHint::parse(hint)?;
let (hint_type, hint_data) = hint.split();
trace!(target: "fetcher", "Fetching hint: {hint_type} {hint_data}");
trace!(target: "fetcher_with_eigenda_support", "Fetching hint: {hint_type} {hint_data}");

if hint_type == ExtendedHintType::AltDACommitment {
if hint_type == ExtendedHintType::EigenDACommitment {
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we add else part that panic! when accidentally get other hint

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let cert = hint_data;
info!(target: "fetcher", "Fetching AltDACommitment cert: {:?}", cert);
info!(target: "fetcher_with_eigenda_support", "Fetching AltDACommitment cert: {:?}", cert);
// Fetch the blob sidecar from the blob provider.
let eigenda_blob = self
.eigenda_blob_provider
.fetch_eigenda_blob(&cert)
.await
.map_err(|e| anyhow!("Failed to fetch eigenda blob: {e}"))?;

info!(target: "fetcher", "eigenda_blob len {}", eigenda_blob.len());
info!(target: "fetcher_with_eigenda_support", "eigenda_blob len {}", eigenda_blob.len());
// Acquire a lock on the key-value store and set the preimages.
let mut kv_write_lock = self.kv_store.write().await;

Expand Down
6 changes: 3 additions & 3 deletions bin/host/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use kona_host::kv;

use crate::eigenda_blobs::OnlineEigenDABlobProvider;
use anyhow::{anyhow, Result};
use extended_fetcher::ExtendedFetcher;
use extended_fetcher::FetcherWithEigenDASupport;
use kona_preimage::{
BidirectionalChannel, HintReader, HintWriter, NativeChannel, OracleReader, OracleServer,
};
Expand Down Expand Up @@ -46,7 +46,7 @@ pub async fn start_server_and_native_client(cfg: HostCli) -> Result<i32> {
.await
.map_err(|e| anyhow!("Failed to load eigenda blob provider configuration: {e}"))?;

Some(Arc::new(RwLock::new(ExtendedFetcher::new_from_parts(
Some(Arc::new(RwLock::new(FetcherWithEigenDASupport::new_from_parts(
kv_store.clone(),
l1_provider,
blob_provider,
Expand Down Expand Up @@ -84,7 +84,7 @@ pub async fn start_server_and_native_client(cfg: HostCli) -> Result<i32> {

pub async fn start_native_preimage_server<KV>(
kv_store: Arc<RwLock<KV>>,
fetcher: Option<Arc<RwLock<ExtendedFetcher<KV>>>>,
fetcher: Option<Arc<RwLock<FetcherWithEigenDASupport<KV>>>>,
hint_chan: NativeChannel,
preimage_chan: NativeChannel,
) -> Result<()>
Expand Down
25 changes: 8 additions & 17 deletions bin/host/src/preimage.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
//! Contains the implementations of the [HintRouter] and [PreimageFetcher] traits.]

use crate::{extended_fetcher::ExtendedFetcher, kv::KeyValueStore};
use crate::{extended_fetcher::FetcherWithEigenDASupport, kv::KeyValueStore};
use async_trait::async_trait;
use kona_preimage::{
errors::{PreimageOracleError, PreimageOracleResult},
Expand All @@ -15,7 +15,7 @@ pub struct OnlinePreimageFetcher<KV>
where
KV: KeyValueStore + ?Sized,
{
inner: Arc<RwLock<ExtendedFetcher<KV>>>,
inner: Arc<RwLock<FetcherWithEigenDASupport<KV>>>,
}

#[async_trait]
Expand All @@ -37,7 +37,7 @@ where
KV: KeyValueStore + ?Sized,
{
/// Create a new [OnlinePreimageFetcher] from the given [ExtendedFetcher].
pub const fn new(fetcher: Arc<RwLock<ExtendedFetcher<KV>>>) -> Self {
pub const fn new(fetcher: Arc<RwLock<FetcherWithEigenDASupport<KV>>>) -> Self {
Self { inner: fetcher }
}
}
Expand Down Expand Up @@ -80,7 +80,7 @@ pub struct OnlineHintRouter<KV>
where
KV: KeyValueStore + ?Sized,
{
inner: Arc<RwLock<ExtendedFetcher<KV>>>,
inner: Arc<RwLock<FetcherWithEigenDASupport<KV>>>,
}

#[async_trait]
Expand All @@ -90,7 +90,9 @@ where
{
async fn route_hint(&self, hint: String) -> PreimageOracleResult<()> {
let mut fetcher = self.inner.write().await;
fetcher.hint(&hint);
fetcher
.hint(&hint)
.map_err(|e| PreimageOracleError::Other(e.to_string()))?;
Ok(())
}
}
Expand All @@ -100,18 +102,7 @@ where
KV: KeyValueStore + ?Sized,
{
/// Create a new [OnlineHintRouter] from the given [ExtendedFetcher].
pub const fn new(fetcher: Arc<RwLock<ExtendedFetcher<KV>>>) -> Self {
pub const fn new(fetcher: Arc<RwLock<FetcherWithEigenDASupport<KV>>>) -> Self {
Self { inner: fetcher }
}
}

/// An [OfflineHintRouter] is a [HintRouter] that does nothing.
#[derive(Debug)]
pub struct OfflineHintRouter;

#[async_trait]
impl HintRouter for OfflineHintRouter {
async fn route_hint(&self, _hint: String) -> PreimageOracleResult<()> {
Ok(())
}
}
15 changes: 7 additions & 8 deletions bin/host/src/server.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
//! This module contains the [PreimageServer] struct and its implementation.

use crate::{
extended_fetcher::ExtendedFetcher,
extended_fetcher::FetcherWithEigenDASupport,
kv::KeyValueStore,
preimage::{
OfflineHintRouter, OfflinePreimageFetcher, OnlineHintRouter, OnlinePreimageFetcher,
},
preimage::{OfflinePreimageFetcher, OnlineHintRouter, OnlinePreimageFetcher},
};
use anyhow::{anyhow, Result};
use kona_host::preimage::OfflineHintRouter;
use kona_preimage::{
errors::PreimageOracleError, HintReaderServer, HintRouter, PreimageFetcher,
PreimageOracleServer,
Expand All @@ -33,7 +32,7 @@ where
kv_store: Arc<RwLock<KV>>,
/// The fetcher for fetching preimages from a remote source. If [None], the server will only
/// serve preimages that are already in the key-value store.
fetcher: Option<Arc<RwLock<ExtendedFetcher<KV>>>>,
fetcher: Option<Arc<RwLock<FetcherWithEigenDASupport<KV>>>>,
}

impl<P, H, KV> PreimageServer<P, H, KV>
Expand All @@ -49,7 +48,7 @@ where
oracle_server: P,
hint_reader: H,
kv_store: Arc<RwLock<KV>>,
fetcher: Option<Arc<RwLock<ExtendedFetcher<KV>>>>,
fetcher: Option<Arc<RwLock<FetcherWithEigenDASupport<KV>>>>,
) -> Self {
Self {
oracle_server,
Expand Down Expand Up @@ -80,7 +79,7 @@ where
/// client.
async fn start_oracle_server(
kv_store: Arc<RwLock<KV>>,
fetcher: Option<Arc<RwLock<ExtendedFetcher<KV>>>>,
fetcher: Option<Arc<RwLock<FetcherWithEigenDASupport<KV>>>>,
oracle_server: P,
) -> Result<()> {
#[inline(always)]
Expand Down Expand Up @@ -121,7 +120,7 @@ where
/// handler.
async fn start_hint_router(
hint_reader: H,
fetcher: Option<Arc<RwLock<ExtendedFetcher<KV>>>>,
fetcher: Option<Arc<RwLock<FetcherWithEigenDASupport<KV>>>>,
) -> Result<()> {
#[inline(always)]
async fn do_loop<R, H>(router: &R, server: &H) -> Result<()>
Expand Down
6 changes: 3 additions & 3 deletions crates/proof/src/eigenda_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl<T: CommsClient + Sync + Send> EigenDABlobProvider for OracleEigenDAProvider

async fn get_blob(&mut self, cert: &Bytes) -> Result<Bytes, Self::Error> {
self.oracle
.write(&ExtendedHintType::AltDACommitment.encode_with(&[cert]))
.write(&ExtendedHintType::EigenDACommitment.encode_with(&[cert]))
.await
.map_err(OracleProviderError::Preimage)?;
let data = self
Expand All @@ -45,14 +45,14 @@ impl<T: CommsClient + Sync + Send> EigenDABlobProvider for OracleEigenDAProvider

async fn get_element(&mut self, cert: &Bytes, element: &Bytes) -> Result<Bytes, Self::Error> {
self.oracle
.write(&ExtendedHintType::AltDACommitment.encode_with(&[cert]))
.write(&ExtendedHintType::EigenDACommitment.encode_with(&[cert]))
.await
.map_err(OracleProviderError::Preimage)?;

let cert_point_key = Bytes::copy_from_slice(&[cert.to_vec(), element.to_vec()].concat());

self.oracle
.write(&ExtendedHintType::AltDACommitment.encode_with(&[&cert_point_key]))
.write(&ExtendedHintType::EigenDACommitment.encode_with(&[&cert_point_key]))
.await
.map_err(OracleProviderError::Preimage)?;
let data = self
Expand Down
6 changes: 3 additions & 3 deletions crates/proof/src/hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ impl ExtendedHint {
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
pub enum ExtendedHintType {
Original(HintType),
AltDACommitment,
EigenDACommitment,
}

impl ExtendedHintType {
Expand All @@ -68,7 +68,7 @@ impl TryFrom<&str> for ExtendedHintType {

fn try_from(value: &str) -> Result<Self, Self::Error> {
match value {
"altda-commitment" => Ok(Self::AltDACommitment),
"eigenda-commitment" => Ok(Self::EigenDACommitment),
_ => Ok(Self::Original(HintType::try_from(value)?)),
}
}
Expand All @@ -77,7 +77,7 @@ impl TryFrom<&str> for ExtendedHintType {
impl From<ExtendedHintType> for &str {
fn from(value: ExtendedHintType) -> Self {
match value {
ExtendedHintType::AltDACommitment => "altda-commitment",
ExtendedHintType::EigenDACommitment => "eigenda-commitment",
ExtendedHintType::Original(hint_type) => hint_type.into(),
}
}
Expand Down
Loading