Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Responses get mixed up due to conflicting payload IDs (v 4.x) #5373

Closed
1 task done
Muhammad-Altabba opened this issue Aug 23, 2022 · 4 comments · Fixed by #5628
Closed
1 task done

Responses get mixed up due to conflicting payload IDs (v 4.x) #5373

Muhammad-Altabba opened this issue Aug 23, 2022 · 4 comments · Fixed by #5628
Assignees
@Muhammad-Altabba
Labels
4.x 4.0 related Bug Addressing a bug

Comments

@Muhammad-Altabba
Copy link
Contributor

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Discuss the issue #5327 at v4.x

Expected Behavior

To have a unique number for the Json RPC message id like in #5371.
However, version 1.x it was better to not change the type of jsonrpc id from being a number to being a string or a UUID
But, for v4.x, I suggest one of the following instead of an incremental value:

  • A UUID with will ensure being unique.
  • Or using a pattern like [random-word]-[incremental-counter] if it is preferable to have an incremental part for easy tracking.

Steps to Reproduce

As in #5327

Web3.js Version

4.x

Environment

No response

Anything Else?

No response
@Muhammad-Altabba Muhammad-Altabba added Bug Addressing a bug 4.x 4.0 related labels Aug 23, 2022
@Muhammad-Altabba Muhammad-Altabba mentioned this issue Aug 23, 2022
Merged
31 tasks
@ravenflores
Copy link

same problem I got one do u have solutions ?

erver\node_modules\web3-providers-ws\lib\index.js:260
id = payload[0].id;

@Muhammad-Altabba
Copy link
Contributor Author

Muhammad-Altabba commented Aug 25, 2022 •
edited
Loading

Thanks @ravenflores ,
The changes are done now for 1.x and it will be available in the next release. And this issue has been addressed here and is in discussion for version 4.x.

@jdevcs
Copy link
Contributor

jdevcs commented Aug 25, 2022

Using uuid will be good fix for this problem, optionally we can also allow user to control assigning id functionality as additional feature in 4.x as well.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.

@github-actions github-actions bot added the Stale Has not received enough activity label Oct 25, 2022
@mconnelly8 mconnelly8 removed the Stale Has not received enough activity label Nov 1, 2022
@Muhammad-Altabba Muhammad-Altabba self-assigned this Nov 17, 2022
@Muhammad-Altabba Muhammad-Altabba mentioned this issue Nov 17, 2022
Merged
31 tasks
Muhammad-Altabba added a commit that referenced this issue Nov 18, 2022
@Muhammad-Altabba
update CHANGELOG.md files for #5373
78ed9c6
Muhammad-Altabba added a commit that referenced this issue Nov 23, 2022
@Muhammad-Altabba
enable the jsonrpc id to optionally be incremented starting from a …
b3cdf3f 
…number

 (Inspired by: #5373 (comment) and needed as a work around for blockchainsllc/in3#46)
Muhammad-Altabba added a commit that referenced this issue Nov 28, 2022
@Muhammad-Altabba
update CHANGELOG.md for #5373
0dfd5ab
@spacesailor24 spacesailor24 mentioned this issue Nov 30, 2022
Merged
Muhammad-Altabba added a commit that referenced this issue Dec 6, 2022
@Muhammad-Altabba
Test with injected external providers (#5652)
21b7649 
* fix: sending tx with injected provider (#5651)

Co-authored-by: Marin Petrunic <marin.petrunic@gmail.com>

* adding a test for using `ganache` provider

* enable the jsonrpc `id` to optionally be incremented starting from a number
 (Inspired by: #5373 (comment) and needed as a work around for blockchainsllc/in3#46)

* test with `in3` as a provider & skip `in3` test if it takes too long

* increase integration test timeout at web3 package

* add a test for using `hardhat` provider

* improve how legacy providers, such as hardhat, is used

* implement `isPromise` that works in the browsers

* refactor external providers tests

* update CHANGELOG.md
jdevcs added a commit that referenced this issue Dec 8, 2022
@spacesailor24 @avkos
@mpetrunic @Muhammad-Altabba @jdevcs
Release/4.0.1 alpha.2 merge conflicts (#5692)
75f4b3e 
* Update CHANGELOG version headers

* Contract call with tuple is missing param names (#5613)

* call special output type

* fix

* fix: enable outputs to have param names (#5624)

* hot fix

* add type if only one param

* overloaded inputs types

* fix resolver tests

* add type tests

* simplify types

* revert registry unit test

* test firefox

* revert firefox test

* update changelogs

Co-authored-by: Marin Petrunić <mpetrunic@users.noreply.github.com>

* Fix `isHex` and `isHexStrict` for some edge cases and enrich their tests (#5655)

* Change `isHex` to return true for negative numbers (for example `-123`)
* Change both `isHex` and `isHexStrict` to return `false` for `-0x`
* Change `isHex` to return `false` for empty strings ''.

* Remove erroneous set provider code for Contract constructor (#5669)

* Remove erroneous set provider code for Contract constructor. Add Contract constructor provider set test

* Update CHANGELOGs

* Debugging failing integration tests

* Apply suggestions from code review

* Use getSystemTestProvider instead of hardcoded string

* Refactor ternaries in Contract constructor to if statements

* Correct CHANGELOG entries

* Remove unneeded checks in if statements

* Test with injected external providers (#5652)

* fix: sending tx with injected provider (#5651)

Co-authored-by: Marin Petrunic <marin.petrunic@gmail.com>

* adding a test for using `ganache` provider

* enable the jsonrpc `id` to optionally be incremented starting from a number
 (Inspired by: #5373 (comment) and needed as a work around for blockchainsllc/in3#46)

* test with `in3` as a provider & skip `in3` test if it takes too long

* increase integration test timeout at web3 package

* add a test for using `hardhat` provider

* improve how legacy providers, such as hardhat, is used

* implement `isPromise` that works in the browsers

* refactor external providers tests

* update CHANGELOG.md

* Use Error ABI to parse errors when sending a transaction (#5662)

* use Error ABI when sending a transaction

* update CHANGELOG.md for #5587

* Remove merge conflict marker

Co-authored-by: Oleksii Kosynskyi <oleksii.kosynskyi@gmail.com>
Co-authored-by: Marin Petrunić <mpetrunic@users.noreply.github.com>
Co-authored-by: Muhammad Altabba <24407834+Muhammad-Altabba@users.noreply.github.com>
Co-authored-by: jdevcs <junaid@chainsafe.io>
spacesailor24 added a commit that referenced this issue Dec 8, 2022
@spacesailor24 @avkos
@mpetrunic @Muhammad-Altabba @jdevcs
v4.0.1-alpha.2 (#5664)
6c166b9 
* Version bump and build for v4.0.1-alpha.2

* Release/4.0.1 alpha.2 changelog update 3 (#5689)

* Update CHANGELOG version headers

* Update CHANGELOGs

* Revert package version bump for web3-packagetemplate

* Release/4.0.1 alpha.2 merge conflicts (#5692)

* Update CHANGELOG version headers

* Contract call with tuple is missing param names (#5613)

* call special output type

* fix

* fix: enable outputs to have param names (#5624)

* hot fix

* add type if only one param

* overloaded inputs types

* fix resolver tests

* add type tests

* simplify types

* revert registry unit test

* test firefox

* revert firefox test

* update changelogs

Co-authored-by: Marin Petrunić <mpetrunic@users.noreply.github.com>

* Fix `isHex` and `isHexStrict` for some edge cases and enrich their tests (#5655)

* Change `isHex` to return true for negative numbers (for example `-123`)
* Change both `isHex` and `isHexStrict` to return `false` for `-0x`
* Change `isHex` to return `false` for empty strings ''.

* Remove erroneous set provider code for Contract constructor (#5669)

* Remove erroneous set provider code for Contract constructor. Add Contract constructor provider set test

* Update CHANGELOGs

* Debugging failing integration tests

* Apply suggestions from code review

* Use getSystemTestProvider instead of hardcoded string

* Refactor ternaries in Contract constructor to if statements

* Correct CHANGELOG entries

* Remove unneeded checks in if statements

* Test with injected external providers (#5652)

* fix: sending tx with injected provider (#5651)

Co-authored-by: Marin Petrunic <marin.petrunic@gmail.com>

* adding a test for using `ganache` provider

* enable the jsonrpc `id` to optionally be incremented starting from a number
 (Inspired by: #5373 (comment) and needed as a work around for blockchainsllc/in3#46)

* test with `in3` as a provider & skip `in3` test if it takes too long

* increase integration test timeout at web3 package

* add a test for using `hardhat` provider

* improve how legacy providers, such as hardhat, is used

* implement `isPromise` that works in the browsers

* refactor external providers tests

* update CHANGELOG.md

* Use Error ABI to parse errors when sending a transaction (#5662)

* use Error ABI when sending a transaction

* update CHANGELOG.md for #5587

* Remove merge conflict marker

Co-authored-by: Oleksii Kosynskyi <oleksii.kosynskyi@gmail.com>
Co-authored-by: Marin Petrunić <mpetrunic@users.noreply.github.com>
Co-authored-by: Muhammad Altabba <24407834+Muhammad-Altabba@users.noreply.github.com>
Co-authored-by: jdevcs <junaid@chainsafe.io>

* Release patch/4.0.1 alpha.2/solve merge conflicts (#5697)

* Update CHANGELOG version headers

* Remove merge conflict markers

* Remove double CHANGELOG entry

Co-authored-by: Oleksii Kosynskyi <oleksii.kosynskyi@gmail.com>
Co-authored-by: Marin Petrunić <mpetrunic@users.noreply.github.com>
Co-authored-by: Muhammad Altabba <24407834+Muhammad-Altabba@users.noreply.github.com>
Co-authored-by: jdevcs <junaid@chainsafe.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Muhammad-Altabba Muhammad-Altabba

Labels
4.x 4.0 related Bug Addressing a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use Uuid for response id web3/web3.js
4 participants
@Muhammad-Altabba @ravenflores @jdevcs @mconnelly8