feat: BAL EIP-7928 #3070
feat: BAL EIP-7928 #3070
Conversation
CodSpeed Performance ReportMerging #3070 will degrade performances by 17.18%Comparing Summary
Benchmarks breakdown
|
rakita
force-pushed
the
rakita/bal
branch
2 times, most recently
from
8317dff to
a2d66df
Compare
* fix: bal binary search cases * nit(test): generalize BAL binary search test for any threshold * code cleanup --------- Co-authored-by: rakita <[email protected]>
|
|
||
| impl<DB: Database> Database for State<DB> { | ||
| type Error = DB::Error; | ||
| type Error = BalDatabaseError<DB::Error>; |
There was a problem hiding this comment.
hmm I think it's undesired that we break API like that
There was a problem hiding this comment.
We are going back and forth on this. It should be fine as this error is again wrapped in main EVMError as result of transact.
cc @mattsse
| // will populate account code if there was a bal change to it. If there is no change | ||
| // it will be fetched in code_by_hash. | ||
| self.bal_state | ||
| .basic(address, basic) | ||
| .map_err(Self::Error::from) |
There was a problem hiding this comment.
not sure I understand what we're doing here
does this overlay BAL on top of the state? why don't we query the BAL first then? what do errors represent?
| /// Storage id. | ||
| pub storage_id: Option<usize>, |
There was a problem hiding this comment.
can we document what this is and what it's used for?
| /// Gets storage value of account by its id. | ||
| /// | ||
| /// Default implementation is to call [`Database::storage`] method. | ||
| #[inline] | ||
| fn storage_by_account_id( | ||
| &mut self, | ||
| address: Address, | ||
| account_id: usize, | ||
| storage_key: StorageKey, | ||
| ) -> Result<StorageValue, Self::Error> { | ||
| let _ = account_id; | ||
| self.storage(address, storage_key) | ||
| } |
There was a problem hiding this comment.
also worth documenting what is account_id
Implementation of EIP-7928
This breaks the
EvmStateserde as it introduces an additional field forAccountanoriginal_info. Original info is used to calculate BAL. Discussion pending, maybe it is fine, but adding custom serde serialisation could work here.