6004 replace buffer with uint8array

[luu-alex]

Description

Supersecedes #6004 and fixes the broken testcases
Please include a summary of the changes and be sure to follow our Contribution Guidelines.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist for 1.x:

• I have selected the correct base branch.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• Any dependent changes have been merged and published in downstream modules.
• I ran npm run dtslint with success and extended the tests and types if necessary.
• I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
• I ran npm run build with success.
• I have tested the built dist/web3.min.js in a browser.
• I have tested my code on the live network.
• I have checked the Deploy Preview and it looks correct.
• I have updated the CHANGELOG.md file in the root folder.

Checklist for 4.x:

• I have selected the correct 4.x base branch.
• Within the description, the feature or issue is discussed in detail or an issue is linked.
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas.
• I have added any required tests for the changes I made
• I have made corresponding changes to the documentation
• Any dependent changes have been merged and published in downstream modules.
• I ran yarn successfully
• I ran yarn lint successfully
• I ran yarn build:web successfully
• I ran yarn test:unit successfully
• I ran yarn test:integration successfully
• I ran compile:contracts successfully
• I have tested my code.
• I have updated the corresponding CHANGELOG.md file in the packages I have edited.

[github-actions]

Bundle Stats

Hey there, this message comes from a github action that helps you and reviewers to understand how these changes affect the size of this project's bundle.

As this PR is updated, I'll keep you updated on how the bundle size is impacted.

Total

Asset	Old size	New size	Diff	Diff %
Total	721 KB	695 KB	-25.5 KB	-3.54%

View detailed bundle breakdown

Added

No assets were added

Removed

No assets were removed

Bigger

No assets were bigger

Smaller

No assets were smaller

Unchanged

Asset	Old size	New size	Diff	Diff %
web3.min.js	704 KB	679 KB	-25.5 KB	-3.62%
../lib/commonjs/index.d.ts	8.43 KB	8.43 KB	0	0.00%
../lib/commonjs/accounts.d.ts	3.66 KB	3.67 KB	8 bytes	0.21%
../lib/commonjs/types.d.ts	2.4 KB	2.37 KB	-27 bytes	-1.10%
../lib/commonjs/abi.d.ts	1000 bytes	1000 bytes	0	0.00%
../lib/commonjs/web3.d.ts	808 bytes	808 bytes	0	0.00%
../lib/commonjs/eth.exports.d.ts	280 bytes	280 bytes	0	0.00%
../lib/commonjs/providers.exports.d.ts	148 bytes	148 bytes	0	0.00%
../lib/commonjs/version.d.ts	60 bytes	60 bytes	0	0.00%

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Pages

Latest commit:	0fd7780
Status:	✅  Deploy successful!
Preview URL:	https://661157e4.web3-js-docs.pages.dev
Branch Preview URL:	https://6004-alex-1.web3-js-docs.pages.dev

View logs

[codecov]

Codecov Report

Merging #6033 (0fd7780) into 4.x (7c13a90) will increase coverage by 0.11%.
The diff coverage is 96.46%.

Additional details and impacted files

@@            Coverage Diff             @@
##              4.x    #6033      +/-   ##
==========================================
+ Coverage   85.85%   85.97%   +0.11%     
==========================================
  Files         156      157       +1     
  Lines        7425     7474      +49     
  Branches     2018     2022       +4     
==========================================
+ Hits         6375     6426      +51     
+ Misses       1050     1048       -2     

Flag	Coverage Δ
UnitTests	85.97% <96.46%> (+0.11%)	⬆️

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

Components	Coverage Δ
web3	∅ <ø> (∅)
web3-core	∅ <ø> (∅)
web3-errors	∅ <ø> (∅)
web3-eth	∅ <ø> (∅)
web3-eth-abi	∅ <ø> (∅)
web3-eth-accounts	∅ <ø> (∅)
web3-eth-contract	∅ <ø> (∅)
web3-eth-ens	∅ <ø> (∅)
web3-eth-iban	∅ <ø> (∅)
web3-eth-personal	∅ <ø> (∅)
web3-net	∅ <ø> (∅)
web3-providers-http	∅ <ø> (∅)
web3-providers-ipc	∅ <ø> (∅)
web3-providers-ws	∅ <ø> (∅)
web3-rpc-methods	∅ <ø> (∅)
web3-utils	∅ <ø> (∅)
web3-validator	∅ <ø> (∅)

[spacesailor24]

This looks like a legitimate error

[spacesailor24]

I imagine sign is covered by unit tests elsewhere, but we should probably create an issue to create unit tests for these methods so completeness

[luu-alex]

@spacesailor24 This error has been present in other jobs like integration (14, ganache, ws) on earlier commits but would pass after running a couple of times. I tried producing this issue locally but End-to-End ganache:ws (14, firefox) pass.

Edit: after running the failed test a third time it works, i think there might be a chance of a 65 length string will be produced somehow and causes the tests to fail, in particular it happens to the getblock function

[spacesailor24]

@spacesailor24 This error has been present in other jobs like integration (14, ganache, ws) on earlier commits but would pass after running a couple of times. I tried producing this issue locally but End-to-End ganache:ws (14, firefox) pass.

Edit: after running the failed test a third time it works, i think there might be a chance of a 65 length string will be produced somehow and causes the tests to fail, in particular it happens to the getblock function

@luu-alex Oh no worries then, I thought it was only the two Build / Integration IPC (14/16, geth, ipc) (pull_request) tests that were failing - thank you for checking though

[luu-alex]

@spacesailor24 Created an issue for sign unit tests cod cov #6055

[spacesailor24]

This looks like a legitimate error

After some investigation, it looks this test fails when Ganache is returning a transaction with a signature that has an r value of < 33 bytes (in this case the r value is 65 characters long), which is apparently allowed to happen. I tested a transaction with the shorter r value using ethers and it seems like ethers pads the value with 0 at the beginning:

0x3ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928    // What 4.x returns
0x3ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928   // What 1.x returns
0x03ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928 // What ethers@v5 returns

[mpetrunic]

This looks like a legitimate error

After some investigation, it looks this test fails when Ganache is returning a transaction with a signature that has an r value of < 33 bytes (in this case the r value is 65 characters long), which is apparently allowed to happen. I tested a transaction with the shorter r value using ethers and it seems like ethers pads the value with 0 at the beginning:

0x3ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928    // What 4.x returns
0x3ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928   // What 1.x returns
0x03ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928 // What ethers@v5 returns

bytes array cannot have hex string with odd amount of characters (each byte is represented by 2 characters). It could be that it worked before because Buffer when given odd length hex string just drops last character internally.

Pretty sure the correct behavior is to pad with zero as ethers does.

[spacesailor24]

This looks like a legitimate error

After some investigation, it looks this test fails when Ganache is returning a transaction with a signature that has an r value of < 33 bytes (in this case the r value is 65 characters long), which is apparently allowed to happen. I tested a transaction with the shorter r value using ethers and it seems like ethers pads the value with 0 at the beginning:

0x3ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928    // What 4.x returns
0x3ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928   // What 1.x returns
0x03ddadb60ab671088edd732b21ea6de0d9c073c06ca799538c2d2d153f403928 // What ethers@v5 returns

bytes array cannot have hex string with odd amount of characters (each byte is represented by 2 characters). It could be that it worked before because Buffer when given odd length hex string just drops last character internally.

Pretty sure the correct behavior is to pad with zero as ethers does.

@mpetrunic Interestingly, we only found this issue because Ganache returns the unpadded value. Don't believe Geth does this though as the test only fails for Ganache

[jdevcs]

thanks @mpetrunic for pinning it down, and @spacesailor24 for your findings.
There must be an even number of hex digits to converting to bytes, else discarding a digit is not valid.

Based on team discussion we will open issue in ganache to return valid hex value ( padded ) for getblock with txs like geth does.

[luu-alex]

@jdevcs was taking a look at our failing testcases and geth fails sometimes meaning its not a ganache issue. Looking around stackexchange it seems that r value can be less than 32 bytes. I believe this change is needed to be on our side and to add padding to equal 32 bytes, ill create an issue .