Skip to content

trie: remove owner and binary marshaling from stacktrie#28291

Merged
holiman merged 3 commits intoethereum:masterfrom
holiman:refactor_stacktrie_pt2
Oct 11, 2023
Merged

trie: remove owner and binary marshaling from stacktrie#28291
holiman merged 3 commits intoethereum:masterfrom
holiman:refactor_stacktrie_pt2

Conversation

@holiman
Copy link
Copy Markdown
Contributor

@holiman holiman commented Oct 9, 2023

This is a (potential) follow-up to #28233. This change

  1. removes the owner-notion from a stacktrie; the owner is only ever needed for comitting to the database, but the commit-function, the writeFn is provided by the caller, so the caller can just set the owner into the writeFn instead of having it passed through the stacktrie.
  2. Removes the encoding.BinaryMarshaler/encoding.BinaryUnmarshaler interface from stacktrie. We're not using it, and I doubt anyone downstream is either.

@rjl493456442 rjl493456442 self-assigned this Oct 9, 2023
@holiman holiman force-pushed the refactor_stacktrie_pt2 branch from 9b3cb76 to d550f63 Compare October 10, 2023 06:41
Copy link
Copy Markdown
Member

@rjl493456442 rjl493456442 left a comment

Choose a reason for hiding this comment

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

Left nitpick comments

Comment thread eth/protocols/snap/sync.go Outdated
Comment thread eth/protocols/snap/sync.go Outdated
Comment thread eth/protocols/snap/sync.go Outdated
Comment thread eth/protocols/snap/sync.go Outdated
@rjl493456442
Copy link
Copy Markdown
Member

Again, we should run snap sync before merging, just ensure.

@holiman holiman force-pushed the refactor_stacktrie_pt2 branch from d550f63 to 18771a7 Compare October 10, 2023 08:45
@holiman
Copy link
Copy Markdown
Contributor Author

holiman commented Oct 10, 2023

running this on bench07 now

@holiman holiman added this to the 1.13.3 milestone Oct 10, 2023
@holiman holiman merged commit 8976a0c into ethereum:master Oct 11, 2023
@holiman holiman deleted the refactor_stacktrie_pt2 branch October 11, 2023 07:23
ghost pushed a commit to blocknative/go-ethereum that referenced this pull request Oct 12, 2023
This change
  - Removes the owner-notion from a stacktrie; the owner is only ever needed for comitting to the database, but the commit-function, the `writeFn` is provided by the caller, so the caller can just set the owner into the `writeFn` instead of having it passed through the stacktrie.
  - Removes the `encoding.BinaryMarshaler`/`encoding.BinaryUnmarshaler` interface from stacktrie. We're not using it, and it is doubtful whether anyone downstream is either.
ghost pushed a commit to blocknative/go-ethereum that referenced this pull request Oct 16, 2023
This change
  - Removes the owner-notion from a stacktrie; the owner is only ever needed for comitting to the database, but the commit-function, the `writeFn` is provided by the caller, so the caller can just set the owner into the `writeFn` instead of having it passed through the stacktrie.
  - Removes the `encoding.BinaryMarshaler`/`encoding.BinaryUnmarshaler` interface from stacktrie. We're not using it, and it is doubtful whether anyone downstream is either.
devopsbo3 pushed a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
This change
  - Removes the owner-notion from a stacktrie; the owner is only ever needed for comitting to the database, but the commit-function, the `writeFn` is provided by the caller, so the caller can just set the owner into the `writeFn` instead of having it passed through the stacktrie.
  - Removes the `encoding.BinaryMarshaler`/`encoding.BinaryUnmarshaler` interface from stacktrie. We're not using it, and it is doubtful whether anyone downstream is either.
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023
sduchesneau pushed a commit to streamingfast/go-ethereum that referenced this pull request Dec 19, 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.

2 participants