refactor: Remove the unnecessary chainId parameter

[in74mz]

Description

Fixes #5949

The toChecksumAddress() function had an unnecessary chainId parameter.
In fact, the chainId parameter is not used within the toChecksumAddress() function, so I've removed it because it might raise questions about why it exists when you want to use that function.
(web3.js/packages/web3-utils/types/index.d.ts)

[luu-alex]

Thank you for this, it looks like this was added mistakenly.

[coveralls]

Pull Request Test Coverage Report for Build 4560520082

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 440 unchanged lines in 7 files lost coverage.
• Overall coverage decreased (-2.4%) to 72.59%

---

Files with Coverage Reduction	New Missed Lines	%
packages/web3-core-requestmanager/src/jsonrpc.js	2	82.76%
packages/web3-core-helpers/src/formatters.js	25	80.58%
packages/web3-core-helpers/src/errors.js	33	4.17%
packages/web3-utils/src/soliditySha3.js	55	5.13%
packages/web3-utils/src/index.js	58	36.89%
packages/web3-utils/src/utils.js	98	12.87%
packages/web3-eth-accounts/src/index.js	169	23.26%

Totals
Change from base Build 4538520910:	-2.4%
Covered Lines:	3466
Relevant Lines:	4493

---

💛 - Coveralls

[luu-alex]

This looks like it'll be a breaking change but its also a redundant parameter that effects the d.ts file.

[nikoulai]

This looks like it'll be a breaking change but its also a redundant parameter that effects the d.ts file.

Yep, breaking change. But, in any case, we want to merge it, lint actions should be passing.

[in74mz]

This looks like it'll be a breaking change but its also a redundant parameter that effects the d.ts file.

Yep, breaking change. But, in any case, we want to merge it, lint actions should be passing.

Is there anything else I need to work on?

[luu-alex]

@in74mz nope, i think you are all good to go. I'll debug this when I have the time. Can you make an issue for this PR and make link it to this PR?

[in74mz]

@in74mz nope, i think you are all good to go. I'll debug this when I have the time. Can you make an issue for this PR and make link it to this PR?

I have just created this issue and linked it with this PR.

[spacesailor24]

Because chainId isn't actually an expected parameter of toChecksumAddress I think this is fine to be merged. chainId also never seems to be passed to toChecksumAddress

[luu-alex]

@spacesailor24 Agreed. Will merge after solving test issue, theres a lint failure in the tests
Even though the test is removed it still is run for an unknown reason.

#4841 (comment) I believe it is the same issue this PR faces

The tests are pulling expected types being compared to from what is published on NPM, possibly due to its attempt at linearizing a circular dependency, instead of pulling the type definitions from the current package.

I think we should merge this and the other pr I linked. it'll break lint tests until next release, so we should merge them before a cut-off. thoughts?

[spacesailor24]

@luu-alex Yes seems to be an erroneously reported failing test. I'm less inclined to merge #4841 though as it has a higher likelihood of breaking typescript user's code for no real benefit. I understand #4841 is a matter of correctness, but no other users seems to have reported the issue and it isn't an issue in 4.x

[Muhammad-Altabba]

So, I think this MR should be closed as it is fixed in 4.x version, and because changing it at version 1.x could trouble some users if they updated their npm package after this change came while they expect non-breaking changes to their TypeScript code.

(By the way, there is an EIP (https://eips.ethereum.org/EIPS/eip-1191) for using the chain Id when calculating the checksum. This is the reason for this parameter addition. Even though, it was not implemented in web3.js later.)