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.

Sign up for GitHub

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

Jump to bottom

Not to wrap values into RLP when storing the valueNode in the trie #126

Closed
AlexeyAkhunov opened this issue Oct 19, 2019 · 1 comment · Fixed by #170
Closed

Not to wrap values into RLP when storing the valueNode in the trie #126

AlexeyAkhunov opened this issue Oct 19, 2019 · 1 comment · Fixed by #170
Assignees
@mandrigin

Comments

@AlexeyAkhunov
Copy link
Contributor

AlexeyAkhunov commented Oct 19, 2019 •
edited
Loading

Before we performed state trie unification and therefore created a separate leaf node type accountNode, simple value nodes and account nodes needed different amount of RLP "wrapping" (The quirk of yellow paper). Now that that their are separate, we should remove RLP wrapping from valueNode leaves. This will simplify code in some parts, and also reduce number of allocation due to wrapping/up-wrapping.
Issue #118 was a result of extra wrapping applied erroneously
@AlexeyAkhunov
Copy link
Contributor Author

Additionally, HashBuilder (in trie/structural_2.go) should be changed to assume that value tape provides "raw" (without any RLP encoding) values, and additional (2nd) wrapping to occur here:
https://github.com/ledgerwatch/turbo-geth/blob/c9e1cb8b7728d7d302cf192512fa6614e2584cbe/trie/structural_2.go#L306

by using generateByteArrayLenDouble and then subsequently generating two RLP length prefixes instead of one (that is quite weird design in Ethereum, but we've got to live with it for now).

@mandrigin mandrigin self-assigned this Nov 7, 2019
@mandrigin mandrigin mentioned this issue Nov 13, 2019
Merged
battlmonstr pushed a commit that referenced this issue Sep 14, 2023
@AskAlexSharov
Pool intrinsic gas check (#126)
70c39cd
cffls pushed a commit to cffls/erigon that referenced this issue Feb 14, 2024
@V-Staykov
throw error instead of just warning (erigontech#126)
12b1cdc
jyellick pushed a commit to bobanetwork/op-erigon that referenced this issue Feb 26, 2024
@ImTei
Merge pull request erigontech#126 from testinprod-io/ecotone-extend-e…
2ab467e 
…ngine-api

[Ecotone] Extend engine api
wtdcode pushed a commit to wtdcode/erigon that referenced this issue May 27, 2024
@blxdyx
london: Support london upgrade erigontech#126
2a3298c 
node-real#126
taratorio pushed a commit that referenced this issue Jul 23, 2024
@AskAlexSharov
save (#126)
3cef6c8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mandrigin mandrigin

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't store RLPs in valueNodes (except tests). erigontech/erigon
2 participants
@mandrigin @AlexeyAkhunov