-
Notifications
You must be signed in to change notification settings - Fork 28
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
Linked lists for the state trie #402
Conversation
…45336afd not found in linked list
…ert_storage_change
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 have a general comment which comes up in several places of the codebase: the LL nodes for the accounts are indexed with their state key, which is the hash of the address. It gets pretty confusing since it's called addr
in lots of cases. I would change them all for something like addr_key
.
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/linked_list.asm
Outdated
Show resolved
Hide resolved
|
||
/// Inserts the account addr and payload pointer into the linked list if it is not already present. | ||
/// Returns `payload_ptr` if the account was inserted, `original_ptr` if it was already present. | ||
global insert_account_to_linked_list: |
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.
From what I'm seeing, we make use of payload_ptr
in only one context: to return 0
if the account is not found. We might as well hardcode 0
as the value if the account wasn't already present.
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 removed insert_account_to_linked_list
, along with some macros, and made other insertion methods not return the payload pointer.
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/linked_list.asm
Outdated
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/initial_tries.asm
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/initial_tries.asm
Outdated
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/initial_tries.asm
Outdated
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/initial_tries.asm
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/initial_tries.asm
Outdated
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/constants/global_metadata.rs
Outdated
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/linked_list.asm
Outdated
Show resolved
Hide resolved
PROVER_INPUT(trie_ptr::state) | ||
|
||
%mstore_global_metadata(@GLOBAL_METADATA_STATE_TRIE_ROOT) | ||
%set_initial_tries |
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.
Maybe we can make use of this call to validate the initial value of @GLOBAL_METADATA_ACCOUNTS_LINKED_LIST_LEN
, if we store it somewhere?
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.
Or maybe it would be easier to check it in store_initial_accounts
(resp. `store_initial_slots) since we're iterating over the accounts (resp. slots) there?
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 removed the PROVER_INPUT calls and computed the lengths in store_initial_accounts
and store_initial_slots
directly
evm_arithmetization/src/cpu/kernel/asm/mpt/linked_list/linked_list.asm
Outdated
Show resolved
Hide resolved
evm_arithmetization/src/cpu/kernel/asm/journal/account_destroyed.asm
Outdated
Show resolved
Hide resolved
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.
Nothing substantial from my side. Feel free to ignore any of my comments.
I notice the amount of tests done for this one and learnt something about how to pull the strings on the interpreter from them.
Thank you for your hard work!
This PR introduces the use of linked lists to store the state trie's accounts and storage slots. This is done to reduce the memory impact of the
TrieData
segment.The PR also hashes both the initial and final tries at the end. With this, the impact on
Memory
,MemBefore
andMemAfter
are reduced for most segments, since the hashing would only appear on the final segments of a proof.The changes aim at making possible the batching of a high number of transactions within a block.
closes #172