feat: Restrict Database::Error. JournaledAccountTr#3199
Conversation
CodSpeed Performance ReportMerging #3199 will not alter performanceComparing Summary
|
klkvr
left a comment
klkvr
left a comment
There was a problem hiding this comment.
lgtm, though would be cool to avoid manual drops
| acc.bump_nonce(); | ||
| acc.incr_balance(U256::from(mint.unwrap_or_default())); | ||
|
|
||
| drop(acc); // Drop acc to avoid borrow checker issues. |
There was a problem hiding this comment.
hmm which change triggered this to cause issues with lifetimes?
There was a problem hiding this comment.
It is with the introduction of GAT in JournalTr::JournaledAccount, the problem here is that for a generic JournaledAccount compiler does not know if manual drop is implemented, so it can't free the acc before the end of the function (NLL Non-Lexical Lifetimes) as objects are freed/dropped in the end.
It sucks a little, but this is an idiomatic solution for this problem.
| /// Increments the balance of the account. | ||
| /// | ||
| /// Touches the account in all cases. | ||
| fn incr_balance(&mut self, balance: U256) -> bool; | ||
|
|
||
| /// Decrements the balance of the account. | ||
| /// | ||
| /// Touches the account in all cases. | ||
| fn decr_balance(&mut self, balance: U256) -> bool; |
There was a problem hiding this comment.
what's the bool that this returns?
* feat: Restrict Database::Error. JournaledAccountTr * nits * nits std
3 participants
Few changes in here, Database::Error now is restricted to
Sync+Send+core::error::Error, it is a breaking change, but usage of this type should not be very disruptive.Additionally,
JournaledAccountTris added that wraps JournaledAccount and removesJournalEntryTrfromJournalTrAdds
ErasedErrorto JournaledAccount functions, which allows erasing Database::Error from it and simplifying the usage in alloy/evm. Have tried propagating ErasedError even more from Journal, but this change will be a lot bigger, so i skipped it.