Skip to content

feat!: Convert buffer usage to Uint8Array#306

Merged
wemeetagain merged 9 commits intoChainSafe:masterfrom
acolytec3:remove-buffers
Jan 25, 2025
Merged

feat!: Convert buffer usage to Uint8Array#306
wemeetagain merged 9 commits intoChainSafe:masterfrom
acolytec3:remove-buffers

Conversation

@acolytec3
Copy link
Contributor

@acolytec3 acolytec3 commented Jan 13, 2025

Fixes #303

Problems:

  1. The enr packages references globalThis.Buffer in the base64 conversion utilities and this causes segmentation faults in the Ultralight hive tests
  2. Buffer was only used for bytes (instead of Uint8Array) because of the requirements of bcrypto which only accepted Buffer as input.
  3. The bigint-buffer package causes CI failures in Ultralight with arbitrary napi-ok === false errors that are indecipherable

Solution:

  • Remove all Buffer types and utilities and replace with equivalent Uint8Arrays
  • Switch toHex/fromHex usage to hexToBytes/bytesToHex from ethereum-cryptography
  • Switch all utf8 conversion to ethereum-cryptography utf8ToBytes/bytesToUtf8 utilities
  • Remove all references to globalThis.buffer
  • Update base64 utilities conversion to use scure/base (author same as @noble/hashes)
  • Remove bigint-buffer and replace with internal bigIntToBytes/bytesToBigInt conversion utilities

@acolytec3 acolytec3 changed the title Convert buffer usage to Uint8Array chore: Convert buffer usage to Uint8Array Jan 13, 2025
@acolytec3 acolytec3 marked this pull request as ready for review January 13, 2025 19:54
@acolytec3 acolytec3 requested a review from a team as a code owner January 13, 2025 19:54
@acolytec3
Copy link
Contributor Author

@wemeetagain Hopefully this isn't too far beyond what you had in mind with regard to #303. Removing buffers provided an opportunity to fix 2 other pain points for Ultralight. If the bigint-buffer performance drop you'll see from going over to the the bigint -> hex -> butes route is a concern, I'd be happy to look for ways to improve the performance though I think @paulm has done a pretty thorough job of squeezing all the performance he can out of that conversion process.

@acolytec3
Copy link
Contributor Author

@wemeetagain bump

@wemeetagain wemeetagain changed the title chore: Convert buffer usage to Uint8Array feat!: Convert buffer usage to Uint8Array Jan 23, 2025
}

export function numberToBuffer(value: number, length: number): Buffer {
export function numberToBuffer(value: number, length: number): Uint8Array {
Copy link
Member

Choose a reason for hiding this comment

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

Is this function still used? maybe delete it or refactor the usage of Buffer here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Not sure how I missed that but will do.

Copy link
Member

@wemeetagain wemeetagain left a comment

Choose a reason for hiding this comment

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

Generally LGTM, thanks for further simplifying the codebase.

Just had one small comment

@acolytec3
Copy link
Contributor Author

@wemeetagain should be ready for final review now.

@wemeetagain wemeetagain merged commit 1cdc424 into ChainSafe:master Jan 25, 2025
5 checks passed
@github-actions github-actions bot mentioned this pull request Jan 25, 2025
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Manually applied changes from [this PR](ChainSafe/discv5#306) to remove vulnerable dep on `bigint-buffer` to [this branch](ChainSafe/discv5@master...AztecProtocol:discv5:chore/upgrade-yarn-v2)

Co-authored-by: Santiago Palladino <santiago@aztec-labs.com>
Co-authored-by: mralj <nikola.mratinic@gmail.com>
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
Manually applied changes from [this PR](ChainSafe/discv5#306) to remove vulnerable dep on `bigint-buffer` to [this branch](ChainSafe/discv5@master...AztecProtocol:discv5:chore/upgrade-yarn-v2)
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 9, 2025
mralj pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 10, 2025
Manually applied changes from [this PR](ChainSafe/discv5#306) to remove vulnerable dep on `bigint-buffer` to [this branch](ChainSafe/discv5@master...AztecProtocol:discv5:chore/upgrade-yarn-v2)
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 10, 2025
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 14, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](https://github.com/NethermindEth/discv5/tree/mralj/chore/backport-306-v2)
Applied changes are from [this
PR](ChainSafe/discv5#306)

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this
PR](ChainSafe/discv5#306), but the changeset is
much smaller -- I have only removed the dependency on `bigint-buffer`
and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this
PR](ChainSafe/discv5#306), but the changeset is
much smaller -- I have only removed the dependency on `bigint-buffer`
and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this
PR](ChainSafe/discv5#306), but the changeset is
much smaller -- I have only removed the dependency on `bigint-buffer`
and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this
PR](ChainSafe/discv5#306), but the changeset is
much smaller -- I have only removed the dependency on `bigint-buffer`
and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
AztecBot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo [link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this PR](ChainSafe/discv5#306), but the changeset is much smaller -- I have only removed the dependency on `bigint-buffer` and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency ([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this
PR](ChainSafe/discv5#306), but the changeset is
much smaller -- I have only removed the dependency on `bigint-buffer`
and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
github-merge-queue bot pushed a commit to AztecProtocol/aztec-packages that referenced this pull request Oct 20, 2025
Removed dependency on bigint-buffer by patching discv5.
Repo
[link](ChainSafe/discv5@master...NethermindEth:discv5:mralj/chore/backport-306-v3)

The applied changes are inspired by [this
PR](ChainSafe/discv5#306), but the changeset is
much smaller -- I have only removed the dependency on `bigint-buffer`
and made it work, because previous attempts made code flaky.

This has been done to remove vulnerable transitive dependency
([link](https://github.com/AztecProtocol/aztec-packages/security/dependabot/395))
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.

Buffer usage

2 participants