Skip to content

fix(pmt): dedupe hashtree hasher dependency - #9349

Closed
lodekeeper wants to merge 1 commit into
ChainSafe:unstablefrom
lodekeeper:fix/pmt-dedup-hashtree-hasher
Closed

fix(pmt): dedupe hashtree hasher dependency#9349
lodekeeper wants to merge 1 commit into
ChainSafe:unstablefrom
lodekeeper:fix/pmt-dedup-hashtree-hasher

Conversation

@lodekeeper

Copy link
Copy Markdown
Contributor

Summary

  • bump Lodestar packages that still pin @chainsafe/persistent-merkle-tree to ^1.2.1
  • align them on ^1.2.5 so they dedupe with @chainsafe/ssz@1.4.0
  • refresh the lockfile so the workspace resolves a single PMT version

Why

setHasher(hashtreeHasher) only helps when callers and SSZ share the same PMT instance. The split between direct ^1.2.1 deps and SSZ's transitive 1.2.5 kept two copies in the graph, so some paths still used the slow noble hasher.

After this change, pnpm why @chainsafe/persistent-merkle-tree shows the workspace resolving PMT 1.2.5 consistently through the direct deps and through SSZ.

Testing

  • pnpm lint

@lodekeeper
lodekeeper requested a review from a team as a code owner May 9, 2026 15:42

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the @chainsafe/persistent-merkle-tree dependency from version 1.2.1 to 1.2.5 across several workspace packages, including api, beacon-node, cli, light-client, prover, and state-transition. Feedback was provided regarding a resulting version split for the @chainsafe/as-sha256 dependency; it is recommended to align all packages to version 1.2.4 to maintain dependency deduplication and avoid loading redundant WASM blobs.

Comment thread pnpm-lock.yaml
'@chainsafe/persistent-merkle-tree@1.2.5':
dependencies:
'@chainsafe/as-sha256': 1.2.0
'@chainsafe/as-sha256': 1.2.4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The update to @chainsafe/persistent-merkle-tree@1.2.5 has introduced a version split for its dependency @chainsafe/as-sha256 (now using 1.2.4), while other packages in the workspace (and @chainsafe/ssz@1.4.0) are still locked to 1.2.0 in the lockfile.

To maintain the goal of deduplication and ensure a consistent hashing stack, consider aligning all direct dependencies on @chainsafe/as-sha256 to ^1.2.4 across the workspace packages. This will prevent having multiple versions of the same library (and potentially multiple WASM blobs) in the dependency graph.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. The new @chainsafe/as-sha256@1.2.4 was pulled transitively by persistent-merkle-tree@1.2.5, while @chainsafe/ssz@1.4.0 and our direct workspace deps still resolve 1.2.0. That split is orthogonal to the goal of this PR (deduping PMT so setHasher(hashtreeHasher) applies to all SSZ consumers) — addressing it would also require an aligned bump in @chainsafe/ssz itself, since its 1.4.0 release pins as-sha256 ^1.2.0. I'd prefer to keep this PR focused on the PMT dedup and handle the as-sha256 alignment in a follow-up that coordinates the SSZ side.

@nflaig nflaig left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@lodekeeper what's the reason for opening this PR?

@lodekeeper

Copy link
Copy Markdown
Contributor Author

Triggered by your ping in topic #10031 that the local patch had been sitting too long.

Substantive reason: workspace deps declare @chainsafe/persistent-merkle-tree: ^1.2.1, while @chainsafe/ssz@1.4.0 transitively pulls 1.2.5. The lockfile resolved both, so two PMT module instances were loaded at runtime. setHasher(hashtreeHasher) in packages/cli/src/applyPreset.ts is module-level state on the PMT instance it imports — so it only swaps the hasher on one of the two PMT copies, and SSZ types whose internal trees come from the other instance keep falling back to the noble hasher.

Bumping all direct deps to ^1.2.5 collapses the graph to a single PMT, after which pnpm why @chainsafe/persistent-merkle-tree shows one resolution. Happy to add a benchmark if you'd like hard numbers before merging — or to drop the PR if you'd rather close out the broader hasher issue differently.

@nflaig

nflaig commented May 11, 2026

Copy link
Copy Markdown
Member

superseded by #9352

@nflaig nflaig closed this May 11, 2026
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