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

core/state/snapshot, core/types, eth: move account definition to type #27323

Merged
merged 2 commits into from
Jun 6, 2023
Merged

core/state/snapshot, core/types, eth: move account definition to type #27323

merged 2 commits into from
Jun 6, 2023

Conversation

rjl493456442
Copy link
Member

@rjl493456442 rjl493456442 commented May 23, 2023 •
edited
Loading

This PR moves all "slim account" notion to types package.

It's a prerequsite of tracking state diffs in path-based storage scheme. Originally
"slim-rlp" format is only used in snapshot to minimize the storage size of accounts,
but will be used in state diffs in path-based as well. types is the better location
to put these common utilities.

holiman
holiman approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

core/types/state_account.go
Comment on lines +60 to +63
data, err := rlp.EncodeToBytes(slim)
if err != nil {
panic(err)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This panic here can probably be omitted, -- we can ignore the err, right?

Copy link
Member Author

@rjl493456442 rjl493456442 Jun 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We can omit the error and commit "rlp encode can never be failed".

Note, in original implementation, we also panic in case error occurs. I am fine with both approaches.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also fine with either (hence I approved it), but since rlp encoding cannot fail, I think we should omit the panic

Copy link
Member Author

Choose a reason for hiding this comment

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

I found another case that RLP encode can fail.

var acct types.StateAccount
rlp.EncodeBytes(acct)

In this case, pointer of types.StateAccount is required as the parameter. I just want to say that rlp may or may not will fail, but panic is better than ignore error silently.

@karalabe karalabe added this to the 1.12.1 milestone Jun 6, 2023
karalabe
karalabe approved these changes Jun 6, 2023
View reviewed changes
Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

SGTM

@karalabe karalabe merged commit 0e5d2c7 into ethereum:master Jun 6, 2023
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@rjl493456442 @devopsbo3
core/state/snapshot, core/types, eth: move account definition to type (…
1b82886 
…ethereum#27323)

* core/state/snapshot, core/types, eth: move account definition to type

* core, eth: revert snapshot Account API change
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "core/state/snapshot, core/types, eth: move account definition…
b200541 
… to type (ethereum#27323)"

This reverts commit 1b82886.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
@devopsbo3
Revert "core/state/snapshot, core/types, eth: move account definition…
b6b6d79 
… to type (ethereum#27323)"

This reverts commit 1b82886.
Francesco4203 added a commit to Francesco4203/ronin that referenced this pull request Oct 8, 2024
@Francesco4203
core/state: move account definition to core/types
c5e5497 
Reference: ethereum/go-ethereum#27323
@Francesco4203 Francesco4203 mentioned this pull request Oct 8, 2024
Merged
Francesco4203 added a commit to axieinfinity/ronin that referenced this pull request Oct 10, 2024
@Francesco4203 @huyngopt1994
trie: path based scheme implementing (#598)
85dc7e2 
* core/state: move account definition to core/types

Reference: ethereum/go-ethereum#27323

* trie: add path base utils

* triedb: implement history and adding some test utils

* trie/triedb/pathdb: implement difflayer and disklayer

* Fix some issues related to history, and add logic checking maxbyte when is zero for retrieving ancient ranges with maxbyte is zero

* trie/triedb/pathdb: implement database.go

* freezer: Add unit test and docs for support freezer reading with no limit size

* trie/triedb/pathdb: add database and difflayer tests

* triedb/pathdb: implement journal and add more comments

---------

Co-authored-by: Huy Ngo <huyngopt1994@gmail.com>
huyngopt1994 added a commit to axieinfinity/ronin that referenced this pull request Oct 10, 2024
@Francesco4203 @huyngopt1994
trie: path based scheme implementing (#598)
2cbb47a 
* core/state: move account definition to core/types

Reference: ethereum/go-ethereum#27323

* trie: add path base utils

* triedb: implement history and adding some test utils

* trie/triedb/pathdb: implement difflayer and disklayer

* Fix some issues related to history, and add logic checking maxbyte when is zero for retrieving ancient ranges with maxbyte is zero

* trie/triedb/pathdb: implement database.go

* freezer: Add unit test and docs for support freezer reading with no limit size

* trie/triedb/pathdb: add database and difflayer tests

* triedb/pathdb: implement journal and add more comments

---------

Co-authored-by: Huy Ngo <huyngopt1994@gmail.com>
huyngopt1994 added a commit to axieinfinity/ronin that referenced this pull request Oct 10, 2024
@huyngopt1994 @Francesco4203
final implementing path base (#591)
9ae5a2b 
* all: clean up overall structure, preparing for path-based (#594)

* trie/triedb/pathdb: init pathdb components

* core, trie: track state change with address instead of hash

Reference: ethereum/go-ethereum@817553c

* trie: refactor

* rawdb: implement freezer resettable & state freezer  (#596)

* rawdb: implement freezer resettable

* rawdb: implement state freezer

* rawdb: update description

* trie: path based scheme implementing (#598)

* core/state: move account definition to core/types

Reference: ethereum/go-ethereum#27323

* trie: add path base utils

* triedb: implement history and adding some test utils

* trie/triedb/pathdb: implement difflayer and disklayer

* Fix some issues related to history, and add logic checking maxbyte when is zero for retrieving ancient ranges with maxbyte is zero

* trie/triedb/pathdb: implement database.go

* freezer: Add unit test and docs for support freezer reading with no limit size

* trie/triedb/pathdb: add database and difflayer tests

* triedb/pathdb: implement journal and add more comments

---------

Co-authored-by: Huy Ngo <huyngopt1994@gmail.com>

---------

Co-authored-by: Francesco4203 <100074926+Francesco4203@users.noreply.github.com>
Francesco4203 added a commit to axieinfinity/ronin that referenced this pull request Oct 17, 2024
@huyngopt1994 @Francesco4203
final implementing path base (#591)
3cc69e7 
* all: clean up overall structure, preparing for path-based (#594)

* trie/triedb/pathdb: init pathdb components

* core, trie: track state change with address instead of hash

Reference: ethereum/go-ethereum@817553c

* trie: refactor

* rawdb: implement freezer resettable & state freezer  (#596)

* rawdb: implement freezer resettable

* rawdb: implement state freezer

* rawdb: update description

* trie: path based scheme implementing (#598)

* core/state: move account definition to core/types

Reference: ethereum/go-ethereum#27323

* trie: add path base utils

* triedb: implement history and adding some test utils

* trie/triedb/pathdb: implement difflayer and disklayer

* Fix some issues related to history, and add logic checking maxbyte when is zero for retrieving ancient ranges with maxbyte is zero

* trie/triedb/pathdb: implement database.go

* freezer: Add unit test and docs for support freezer reading with no limit size

* trie/triedb/pathdb: add database and difflayer tests

* triedb/pathdb: implement journal and add more comments

---------

Co-authored-by: Huy Ngo <huyngopt1994@gmail.com>

---------

Co-authored-by: Francesco4203 <100074926+Francesco4203@users.noreply.github.com>
huyngopt1994 added a commit to axieinfinity/ronin that referenced this pull request Oct 25, 2024
@huyngopt1994 @Francesco4203
final implementing path base (#591)
f5ad220 
* all: clean up overall structure, preparing for path-based (#594)

* trie/triedb/pathdb: init pathdb components

* core, trie: track state change with address instead of hash

Reference: ethereum/go-ethereum@817553c

* trie: refactor

* rawdb: implement freezer resettable & state freezer  (#596)

* rawdb: implement freezer resettable

* rawdb: implement state freezer

* rawdb: update description

* trie: path based scheme implementing (#598)

* core/state: move account definition to core/types

Reference: ethereum/go-ethereum#27323

* trie: add path base utils

* triedb: implement history and adding some test utils

* trie/triedb/pathdb: implement difflayer and disklayer

* Fix some issues related to history, and add logic checking maxbyte when is zero for retrieving ancient ranges with maxbyte is zero

* trie/triedb/pathdb: implement database.go

* freezer: Add unit test and docs for support freezer reading with no limit size

* trie/triedb/pathdb: add database and difflayer tests

* triedb/pathdb: implement journal and add more comments

---------

Co-authored-by: Huy Ngo <huyngopt1994@gmail.com>

---------

Co-authored-by: Francesco4203 <100074926+Francesco4203@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@holiman holiman holiman approved these changes

@karalabe karalabe karalabe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.12.1
Development

Successfully merging this pull request may close these issues.
3 participants
@rjl493456442 @karalabe @holiman