Skip to content
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

Transactions trie support #1232

Merged
merged 10 commits into from
Sep 27, 2023
Merged

Conversation

4l0n50
Copy link
Contributor

@4l0n50 4l0n50 commented Sep 13, 2023

This PR updates the transaction trie with the processed transactions.

The transaction trie update is done as follows:

  • After reading and decoding each transaction from the prover inputs we copy the txn rlp and its size to the trie data segment. The pointer to this data is then used as payload for inserting a node with key RLP(txn_counter) in the transaction trie.

  • For hashing the transaction trie we implemented the function encode_txn. This function simply copies the rlp encoded transaction to the address received as input

@@ -119,7 +127,7 @@ fn test_receipt_encoding() -> Result<()> {
// Get the expected RLP encoding.
let expected_rlp = rlp::encode(&rlp::encode(&receipt_1));

let initial_stack = vec![retdest, 0.into(), 0.into()];
let initial_stack: Vec<U256> = vec![retdest, 0.into(), 0.into()];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those changes seem unnecessary, but they silenced the clippy warning of needless vec (which is actually needed because of the Interpreter::new_with_kernel expected parameter types).

@Nashtare Nashtare changed the title Transactions Transactions trie support Sep 14, 2023
@pgebheim pgebheim requested a review from wborgeaud September 18, 2023 16:06
Copy link
Contributor

@wborgeaud wborgeaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

evm/src/cpu/kernel/asm/main.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/main.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/main.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/asm/transactions/router.asm Outdated Show resolved Hide resolved
evm/src/cpu/kernel/tests/mpt/load.rs Outdated Show resolved Hide resolved
evm/src/generation/mpt.rs Outdated Show resolved Hide resolved
@Nashtare
Copy link
Collaborator

@wborgeaud Sorry for the force-push, the history got completely messed up during a rebase, and induced an error in one of the tests which was easier to just revert to the state of your review (commit fa63f90), and then do a clean merge followed by applying your comments.
Let me know if it still looks good for you!

@wborgeaud
Copy link
Contributor

No worries, still looks good 👍

@Nashtare Nashtare merged commit f49fbc8 into 0xPolygonZero:main Sep 27, 2023
2 checks passed
@Nashtare Nashtare deleted the transactions branch October 31, 2023 21:27
Nashtare added a commit to topos-protocol/plonky2 that referenced this pull request Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants