Skip to content

bump to kona 1.0.0#79

Merged
bxue-l2 merged 4 commits intomasterfrom
bump-to-kona-1.0
May 5, 2025
Merged

bump to kona 1.0.0#79
bxue-l2 merged 4 commits intomasterfrom
bump-to-kona-1.0

Conversation

@bxue-l2
Copy link
Copy Markdown
Collaborator

@bxue-l2 bxue-l2 commented May 5, 2025

This Pr bump kona deps to 1.0.0

Kona adds EvmFactory and op-revm to abstract execution and precompile, see op-rs/kona#1543

@bxue-l2 bxue-l2 requested a review from samlaf May 5, 2025 00:24
@bxue-l2 bxue-l2 marked this pull request as ready for review May 5, 2025 00:24
Copy link
Copy Markdown
Contributor

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

Approved. kurtosis CI is failing because of ethpandaops/optimism-package#243. Will fix in another PR

Comment on lines +137 to +147
// uncomment if running preloaded eigenda client
//let client_task = task::spawn(
// hokulea_witgen_client::witgen_client::run_preloaded_eigenda_client(
// OracleReader::new(preimage.client.clone()),
// HintWriter::new(hint.client.clone()),
// FpvmOpEvmFactory::new(
// HintWriter::new(hint.client),
// OracleReader::new(preimage.client),
// ),
// ),
//);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

when do you uncomment this? Could we put this behind an if statement, or a compile flag instead?

Copy link
Copy Markdown
Collaborator Author

@bxue-l2 bxue-l2 May 5, 2025

Choose a reason for hiding this comment

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

I am thinking we should have a binary or test to just run preloaded eigenda client. Not sure how to do it with a test

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

#81

P: PreimageOracleClient + Send + Sync + Debug + Clone,
H: HintWriterClient + Send + Sync + Debug + Clone,
Evm: EvmFactory<Spec = OpSpecId> + Send + Sync + Debug + Clone + 'static,
<Evm as EvmFactory>::Tx: FromTxWithEncoded<OpTxEnvelope> + FromRecoveredTx<OpTxEnvelope>,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this associated type bound that we always add really needed? We don't even seem to be calling evmFactory ourselves. Guessing its used somewhere in some internal kona library, but I don't get it (not super familiar with how these bounds are used)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I just make it consistent to kona library. Will think more about it,
Maybe relate to this? op-rs/kona#1400

Copy link
Copy Markdown
Contributor

@samlaf samlaf left a comment

Choose a reason for hiding this comment

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

LGTM. Prob good to rebase on top of my kurtosis fix PR to make CI pass, but not strictly necessary.

@bxue-l2 bxue-l2 force-pushed the bump-to-kona-1.0 branch from 89d5233 to 4a41751 Compare May 5, 2025 05:46
@bxue-l2 bxue-l2 merged commit b10e35c into master May 5, 2025
8 checks passed
@bxue-l2 bxue-l2 deleted the bump-to-kona-1.0 branch May 5, 2025 06:00
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.

2 participants