-
Notifications
You must be signed in to change notification settings - Fork 7
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
Scroll evm executor/v35 #1
Changes from 26 commits
2ef1c0c
098f35e
ae94add
301846b
391421f
9698f98
03990e9
66a396a
63d75b2
c98e0cb
131facf
d0c493f
66e9e83
290b5a3
c9858f7
337a2f8
591ee51
476d7ed
fddc9dd
91433b3
f6d4bd8
d06633a
c554acf
06857da
61d8ca7
b57e7d9
9ecb479
74c5a77
dae1ae4
e1e8f7a
1d44eb0
abb93af
09daf95
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,9 @@ use crate::{ | |
}; | ||
use std::vec::Vec; | ||
|
||
#[cfg(feature = "scroll")] | ||
use revm_primitives::POSEIDON_EMPTY; | ||
|
||
/// A dummy [Host] implementation. | ||
#[derive(Clone, Debug, Default, PartialEq, Eq)] | ||
pub struct DummyHost { | ||
|
@@ -64,10 +67,23 @@ impl Host for DummyHost { | |
} | ||
|
||
#[inline] | ||
#[cfg(not(feature = "scroll"))] | ||
fn code_hash(&mut self, __address: Address) -> Option<(B256, bool)> { | ||
Some((KECCAK_EMPTY, false)) | ||
} | ||
|
||
#[inline] | ||
#[cfg(feature = "scroll")] | ||
fn code_hash(&mut self, __address: Address) -> Option<(B256, bool)> { | ||
Some((POSEIDON_EMPTY, false)) | ||
} | ||
|
||
#[inline] | ||
#[cfg(feature = "scroll")] | ||
fn keccak_code_hash(&mut self, __address: Address) -> Option<(B256, bool)> { | ||
Some((KECCAK_EMPTY, false)) | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using both feature and not feature seems like an antipattern. Will be a drag in long term maintainance.
this will also remove un- needed tags on top of code that uses code_hash function also |
||
#[inline] | ||
fn sload(&mut self, __address: Address, index: U256) -> Option<(U256, bool)> { | ||
match self.storage.entry(index) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -41,6 +41,10 @@ serde = { version = "1.0", default-features = false, features = [ | |
"rc", | ||
], optional = true } | ||
|
||
# scroll | ||
halo2_proofs = { git = "https://github.com/scroll-tech/halo2.git", branch = "v1.1", optional = true } | ||
hash-circuit = { package = "poseidon-circuit", git = "https://github.com/scroll-tech/poseidon-circuit.git", branch = "scroll-dev-1201", optional = true } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. branch = "main" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hash-circuit or halo2_proofs? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hash-circuit |
||
|
||
[build-dependencies] | ||
hex = { version = "0.4", default-features = false } | ||
|
||
|
@@ -71,6 +75,11 @@ optimism = [] | |
optimism-default-handler = ["optimism"] | ||
negate-optimism-default-handler = [] | ||
|
||
scroll = ["halo2_proofs", "hash-circuit"] | ||
# Scroll default handler enabled Scroll handler register by default in EvmBuilder. | ||
scroll-default-handler = ["scroll"] | ||
negate-scroll-default-handler = [] | ||
|
||
dev = [ | ||
"memory_limit", | ||
"optional_balance_check", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hahah so we need this scroll-tech/poseidon-circuit#28, then we can only use the poseidon primitive crate, no longer needs halo2_proofs, quite confusing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re-export peer dependency is a good pattern
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i mean we should get rid of halo2_proofs in future. How to do this? Split poseidon-circuit crate into 2, one is focusing on hashing computation, not need of halo2, one is focusing on hahsing circuit, need halo2. Here we only need the former
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Re-exporting peer dependencies might seem like a shortcut to a clean and concise codebase. After all, it keeps your imports tidy and avoids users from wrestling with dependency conflicts. But this convenience comes at a hidden cost, potentially leading you down a path of dependency management nightmares.
Imagine this: Your library re-exports a peer dependency, say mathjs, at version 5.0.0. Package A, which uses your library, needs mathjs at version 4.2.1 for specific functionality. Package B, also using your library, needs the latest mathjs (version 6.0.0). This creates a conflict. Package A might break because it relies on features removed in 5.0.0, while Package B might malfunction due to incompatibilities introduced in 6.0.0.
Think of it this way: You build a house on a foundation designed for a specific weight limit. By re-exporting a peer dependency as part of your house, you're changing the foundation's specifications. If you later decide the house needs a stronger foundation, all the houses built on your design (consumers of your library) would need significant work, even if their original design was perfectly functional.
Consider this analogy: You borrow a toolbox from a friend. They've added their own custom wrench but haven't mentioned it. When you use the wrench on a project and it breaks something, troubleshooting becomes harder because you weren't aware of the extra tool and its potential impact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.