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

Proof function util helpers refactor #3551

Merged
merged 12 commits into from
Jul 31, 2024

Conversation

scorbajio
Copy link
Contributor

This change is an alternative approach to the work started in #3538. It looks into refactoring the trie class to remove proof functionality from the main class and move them into standalone functions in order to improve tree shaking for the trie package and decrease bundle size.

@scorbajio
Copy link
Contributor Author

Using a minimal code path for the Trie class:

import { Trie } from '@ethereumjs/trie'

const t = new Trie()
console.log(t.root())

Results:

before: 223.6kb
after: 205.7kb
delta: -17.9kb

Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

Looks great overall. Some comments need cleanup and I think we can remove that fromProof function entirely.

packages/trie/src/proof/index.ts Outdated Show resolved Hide resolved
packages/trie/src/proof/index.ts Outdated Show resolved Hide resolved
packages/trie/src/proof/index.ts Outdated Show resolved Hide resolved
packages/trie/test/util/log.spec.ts Outdated Show resolved Hide resolved
packages/trie/test/util/log.spec.ts Outdated Show resolved Hide resolved
Copy link

codecov bot commented Jul 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.00%. Comparing base (fb50628) to head (61e98d9).
Report is 15 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (fb50628) and HEAD (61e98d9). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (fb50628) HEAD (61e98d9)
tx 1 0
Additional details and impacted files

Impacted file tree graph

Flag Coverage Δ
client 0.00% <ø> (ø)
tx ?

Flags with carried forward coverage won't be shown. Click here to find out more.

@scorbajio scorbajio requested a review from acolytec3 July 30, 2024 19:35
Copy link
Contributor

@acolytec3 acolytec3 left a comment

Choose a reason for hiding this comment

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

LGTM

@scorbajio scorbajio merged commit 61d5387 into master Jul 31, 2024
35 of 36 checks passed
@scorbajio scorbajio deleted the proof-function-util-helpers-refactor branch July 31, 2024 03:32
@holgerd77
Copy link
Member

@scorbajio Nice, looks super clean now! 😄

Results are also meaningful (so the -18KB), since this already helps the Block package upwards, and there reduction on this level definitely already counts in! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants