Fix: BSC - get transaction with gasLimit over 53 bits

[alstjd0921]

Description

Fix outputTransactionFormatter to format gas limit hex string to decimal string.

There are a lot of transactions in the BSC network that have set gas limit by 9,223,372,036,854,775,807 and I think it is necessary to switch to string format for look up these transactions.

The links below are examples of such transactions.
0xb48fbbab4feee817b18efdbde134dba83ba47ea479ab85a2b3ff068bf7c8f5d8
0x02a3e52e32d1433f2d356ddf1408c3d9fb916c0a244f18e8a90ba742f45ae32a
0x65f46d95e70b1906694dbaaccaad12c2ffa9ae5c2a6513719837f50c9f97940a

Fixes #3936
Fixes #3912
Fixes #3745

Type of change

• Breaking change (fix or feature that would cause existing functionality to not work as expected)

[spacesailor24]

Hi there, thank you for submitting this PR!
Our team has discussed fixing this in a subsequent major release, but we greatly appreciate the work that you did!

Additionally, if you wouldn't mind updating the CHANGELOG.md since we can't do it for you

Thank you again for your contributions!

[spacesailor24]

@frankiebee @koraykoska moving the discussion to here:

We previously discussed returning a hex string, but this PR uses an included method to format to a number string. Do we prefer a hex string over using existing util functions?

[alstjd0921]

Hi there, thank you for submitting this PR!
Our team has discussed fixing this in a subsequent major release, but we greatly appreciate the work that you did!

Additionally, if you wouldn't mind updating the CHANGELOG.md since we can't do it for you

Thank you again for your contributions!

@spacesailor24 CHANGELOG.md updated!

[spacesailor24]

Thanks for updating it so quickly!
Unfortunately we'd need to include it in a separate major version, but I'm not sure if this will be 3.x or something else. So, you can hold off on updating CHANGELOG.md again till we can clarify what version this will be included in

[alstjd0921]

@spacesailor24 I see. However, many users are complaining about this issue. I hope this changes included in the release version ASAP.

[aslvrstn]

I think there are a number of other formatters that need this change as well. Notably outputBlockFormatter at https://github.com/ChainSafe/web3.js/blob/1.x/packages/web3-core-helpers/src/formatters.js#L302 and outputTransactionReceiptFormatter at https://github.com/ChainSafe/web3.js/blob/1.x/packages/web3-core-helpers/src/formatters.js#L274

As-is, I don't think this fixes #3912 for instance.

Thanks for the change! This has been driving me crazy.

[spacesailor24]

@aslvrstn oof, thank you for bringing this to our attention, should've looked around for more instances of this bug
I think we should bundle these all in the same major release, but that shouldn't slow things down - probably

[alstjd0921]

@aslvrstn Thanks for review.
In my analysis, the abnormally high gasLimit value is specified only in the transaction, and the gasLimit aggregated from the block does not add up this value. Its weird but, this change can fix #3912 i think.
Also, transactions with these abnormal gasLimit values were all zero-fee transactions. outputTransactionReceiptFormatter will not cause an error because the transactionReceipt does not contain abnormal gasLimit value.

[aslvrstn]

The gas limit aggregated in the block might not be added up, but when called with returnTransactionObjects=true it still seemed broken to me .. do the transactions returned in the getBlock call use a different formatter?

Anyway, I only noticed because I actually tried installing from your branch and re-running the failed call, and it still seemed to fail, so unless I botched the install, I think at least one other location needs to be updated.

Thanks for turning that patch around so quickly, btw! I would have been going insane otherwise.

[alstjd0921]

@aslvrstn https://github.com/ChainSafe/web3.js/blob/1.x/packages/web3-core-helpers/src/formatters.js#L314
Transactions returned in the getBlock call use a same formatter

Following captures are the result of call 'web3.eth.getBlock (5591908, true)' using modified formatter. (BSC Network)
Please make sure you have built the right branch. I'm glad it helped!

[tima-t]

Is it fine, If I create a fork of web3 with this change and deploy new npm package called something like web3-bsc.
Does the license allow it?

I just need this modification for one of our projects that uses BSC.

Another option is if you are planning to merge this PR in the next few days.

Thanks!

[spacesailor24]

Commenting here as well, so this isn't lost:

Waiting on @alstjd0921 to merge this, then we can merge this PR

[coveralls]

Pull Request Test Coverage Report for Build 731388338

• 5 of 6 (83.33%) changed or added relevant lines in 1 file are covered.
• 247 unchanged lines in 8 files lost coverage.
• Overall coverage increased (+2.5%) to 75.95%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
packages/web3-core-helpers/src/formatters.js	5	6	83.33%

Files with Coverage Reduction	New Missed Lines	%
packages/web3-core-requestmanager/src/jsonrpc.js	1	70.0%
packages/web3-core-helpers/src/formatters.js	10	78.88%
packages/web3-core-helpers/lib/formatters.js	19	81.52%
packages/web3-utils/src/utils.js	27	10.74%
packages/web3-core-helpers/src/errors.js	29	1.56%
packages/web3-utils/src/soliditySha3.js	34	3.92%
packages/web3-utils/src/index.js	42	32.12%
packages/web3-eth-accounts/src/index.js	85	33.06%

Totals
Change from base Build 706495893:	2.5%
Covered Lines:	3108
Relevant Lines:	3877

---

💛 - Coveralls

[aslvrstn]

Along @tima-t 's request: how are people installing this as a temporary dependency and having the fix come in? I've been trying variations from https://stackoverflow.com/questions/17509669/how-to-install-an-npm-package-from-github-directly and yet it still seems to be pulling in old web3-core-helpers dependencies or something, because despite the code I see being updated, the code the browser gets is clearly the old hexToNumber version.

I know this is largely unrelated, but it'd really help to have a clear short-term workaround, including pushing an early 3.x release, or allowing a web3-bsc module to be published.

[spacesailor24]

#3976 was merged so the 3.0.0 RC can be released

[phpnikolov]

Easy temporary fix:

1. Open file .\node_modules\number-to-bn\node_modules\bn.js\lib\bn.js
2. Go to line 506 assert(false, 'Number can only safely store up to 53 bits');
3. Replace it with ret = Number.MAX_SAFE_INTEGER;

[krunkosaurus]

@phpnikolov Wow thanks. This worked perfectly!

[hoangvnextdev]

@spacesailor24 can you create PR to fix web3 1.3.x too ? I can't modify node_modules to fix this .

[spacesailor24]

@hoangvnextdev Unfortunately we cannot apply this fix to the 1.x branch, because this is considering a breaking change (since we're returning a number string instead of a number now). To apply these changes right now, you must specifically use the
3.0.0-rc.4 version as the 3.x branch will contain these fixes

[s29papi]

@spacesailor24 Help please, I have made this change to my node module but I am still getting this error. I think my direct changes in the module didn't reflect, please help @phpnikolov just to add the error is emitted from web3-core-helpers errors.js ( I also made the changes here)

[hoangvnextdev]

@nots29vv Hi i think when you reinstall node_modules your modify is change back to origin ? Because i use @phpnikolov solution is worked for me. But I change to using ethersjs more stable than web3

[s29papi]

@hoangvnextdev that is right. moving to ethersjs would not be possible for me, as i am trying to implement gasless transactions using @opengsn and web3 is a dependency for the package. I really appreciate.

[itxtoledo]

using 3.0.0-rc.4 i can use this in BSC but is pretty cool to have this fix in main version

[francesco-clementi-92]

Hi guys, any news on the fix? Which version should I use?

[crellstae]

Hi guys, any news on the fix? Which version should I use?

I had to create a prebuild script for replace some strings, includes the path because it fails after build an run an electron app :/

[tiendq]

Hi guys, any news on the fix? Which version should I use?

Is it reported in number-to-bn or bn.js repos? why don't they fix it?

[stychu]

Easy temporary fix:

1. Open file .\node_modules\number-to-bn\node_modules\bn.js\lib\bn.js
2. Go to line 506 assert(false, 'Number can only safely store up to 53 bits');
3. Replace it with ret = Number.MAX_SAFE_INTEGER;

As long as this solution works this is insane that this error still occurs. How this is not fixed yet?

[Mehranh]

Pleaaaaase fix this error

[goodstemy]

Have the same error execute getTransaction(...tx) on BSC network. Anyone have an idea when it will be fixed?

[goodstemy]

Also for people who want to find a solution rn: #3936 (comment)

Its solve my problem by extending new method.

[htkcodes]

• 1 plus for this fix BSC - certain tx values break web3 #3936

[lucaspiressimao]

Same here but

Also for people who want to find a solution rn: #3936 (comment)

Its solve my problem by extending new method.

this solve for now

[spartanz51]

Have the same error execute getTransaction(...tx) on BSC network. Anyone have an idea when it will be fixed?

It will not be fixed, see #3789 (comment)