-
Notifications
You must be signed in to change notification settings - Fork 0
feat(op-revm): add Soul Gas Token (SGT) support for OP Stack #1
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
base: op-es
Are you sure you want to change the base?
Changes from all commits
79e76f2
3a6aa08
1744019
e78df8a
c1638ca
b1a98a9
3f5854a
14c22d0
bb81552
b82be88
5d00d01
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,13 +160,17 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccount<'a, DB, ENTRY> { | |
| /// Loads the storage slot. | ||
| /// | ||
| /// If storage is cold and skip_cold_load is true, it will return [`JournalLoadError::ColdLoadSkipped`] error. | ||
| /// If `no_warm` is true, the slot is loaded without marking it warm or pushing warming | ||
| /// journal entries (used for protocol-level operations like SGT that should not | ||
| /// influence EIP-2929 gas metering). | ||
| /// | ||
| /// Does not erase the db error. | ||
| #[inline(never)] | ||
| pub fn sload_concrete_error( | ||
| &mut self, | ||
| key: StorageKey, | ||
| skip_cold_load: bool, | ||
| no_warm: bool, | ||
| ) -> Result<StateLoad<&mut EvmStorageSlot>, JournalLoadError<DB::Error>> { | ||
| let is_newly_created = self.account.is_created(); | ||
| let (slot, is_cold) = match self.account.storage.entry(key) { | ||
|
|
@@ -186,7 +190,9 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccount<'a, DB, ENTRY> { | |
| return Err(JournalLoadError::ColdLoadSkipped); | ||
| } | ||
| } | ||
| slot.mark_warm_with_transaction_id(self.transaction_id); | ||
| if !no_warm { | ||
|
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. I think the There are still two places where warming state leaks through:
Also, So the current change only avoids the explicit I think
Collaborator
Author
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. Good catch, fixed in b82be88. |
||
| slot.mark_warm_with_transaction_id(self.transaction_id); | ||
| } | ||
| (slot, is_cold) | ||
| } | ||
| Entry::Vacant(vac) => { | ||
|
|
@@ -200,19 +206,23 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccount<'a, DB, ENTRY> { | |
| if is_cold && skip_cold_load { | ||
| return Err(JournalLoadError::ColdLoadSkipped); | ||
| } | ||
|
|
||
| // if storage was cleared, we don't need to ping db. | ||
| let value = if is_newly_created { | ||
| StorageValue::ZERO | ||
| } else { | ||
| self.db.storage(self.address, key)? | ||
| }; | ||
|
|
||
| let slot = vac.insert(EvmStorageSlot::new(value, self.transaction_id)); | ||
| // When no_warm, don't set transaction_id so the slot stays | ||
| // cold to later normal accesses (is_cold_transaction_id). | ||
| let tid = if no_warm { 0 } else { self.transaction_id }; | ||
| let slot = vac.insert(EvmStorageSlot::new(value, tid)); | ||
| (slot, is_cold) | ||
| } | ||
| }; | ||
|
|
||
| if is_cold { | ||
| if is_cold && !no_warm { | ||
| // add it to journal as cold loaded. | ||
| self.journal_entries | ||
| .push(ENTRY::storage_warmed(self.address, key)); | ||
|
|
@@ -224,6 +234,7 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccount<'a, DB, ENTRY> { | |
| /// Stores the storage slot. | ||
| /// | ||
| /// If storage is cold and skip_cold_load is true, it will return [`JournalLoadError::ColdLoadSkipped`] error. | ||
| /// If `no_warm` is true, storage is accessed without affecting warm/cold status. | ||
| /// | ||
| /// Does not erase the db error. | ||
| #[inline] | ||
|
|
@@ -232,12 +243,13 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccount<'a, DB, ENTRY> { | |
| key: StorageKey, | ||
| new: StorageValue, | ||
| skip_cold_load: bool, | ||
| no_warm: bool, | ||
| ) -> Result<StateLoad<SStoreResult>, JournalLoadError<DB::Error>> { | ||
| // touch the account so changes are tracked. | ||
| self.touch(); | ||
|
|
||
| // assume that acc exists and load the slot. | ||
| let slot = self.sload_concrete_error(key, skip_cold_load)?; | ||
| let slot = self.sload_concrete_error(key, skip_cold_load, no_warm)?; | ||
|
|
||
| let ret = Ok(StateLoad::new( | ||
| SStoreResult { | ||
|
|
@@ -465,7 +477,7 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccountTr | |
| key: StorageKey, | ||
| skip_cold_load: bool, | ||
| ) -> Result<StateLoad<&mut EvmStorageSlot>, JournalLoadErasedError> { | ||
| self.sload_concrete_error(key, skip_cold_load) | ||
| self.sload_concrete_error(key, skip_cold_load, false) | ||
| .map_err(|i| i.map(ErasedError::new)) | ||
| } | ||
|
|
||
|
|
@@ -477,7 +489,7 @@ impl<'a, DB: Database, ENTRY: JournalEntryTr> JournaledAccountTr | |
| new: StorageValue, | ||
| skip_cold_load: bool, | ||
| ) -> Result<StateLoad<SStoreResult>, JournalLoadErasedError> { | ||
| self.sstore_concrete_error(key, new, skip_cold_load) | ||
| self.sstore_concrete_error(key, new, skip_cold_load, false) | ||
| .map_err(|i| i.map(ErasedError::new)) | ||
| } | ||
|
|
||
|
|
||
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.
This change adds new required methods to the
JournalTrtrait (sload_no_warm,sstore_no_warm,load_account_mut_no_warm,load_account_no_warm), but the repo still has anotherimpl JournalTrthat was not updated:examples/cheatcode_inspector/src/main.rs.As a result, this is currently source-breaking for the workspace and
cargo test --workspacefails with missing trait items forBackend.I think this needs either:
JournalTrimplementations together, orThere 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.
Good catch, fixed in b82be88.
Uh oh!
There was an error while loading. Please reload this page.
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.
@qzhodl After further thought, I think it's better to panic in the default trait implementations to ensure they're never called(committed in 5d00d01). WDYT?