Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Conversation

@spacesailor24
Copy link
Contributor

@spacesailor24 spacesailor24 commented Jun 15, 2022

To preview migration guide:

  1. cd web3.js/docs
  2. yarn start
  3. Navigate to Web3Eth Migration Guide

Remaining methods to check for breaking changes:

  • sendSignedTransaction
  • sign
  • signTransaction
  • call
  • estimateGas
  • getPastLogs
  • getWork
  • submitWork
  • requestAccounts
  • getChainId
  • getNodeInfo
  • getProof

@spacesailor24 spacesailor24 added the 4.x 4.0 related label Jun 15, 2022
@spacesailor24 spacesailor24 self-assigned this Jun 15, 2022
@nikoulai nikoulai marked this pull request as ready for review June 21, 2022 13:45
@nikoulai
Copy link
Contributor

BatchRequest is not documented ( I'll copy it from web3 migration guide after it get ok from reviews)

Copy link
Contributor

@nazarhussain nazarhussain left a comment

Choose a reason for hiding this comment

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

I have following observations:

  1. Pleae make the change for the bigint everywhere. All the numbers data type are consistenlty should return the bigint . If not then it's a bug and we should fix it.
  2. It does not feel like productive to me to document those breaking changes in that way. Will be tedious to review and maintain. Instead we can just document the list of funcitons which now will return bigint for all number type attributes.


- `givenProvider` default value is `undefined` instead of `null`
- `currentProvider` will never return `null`, provider required upon instantiation as opposed to being optional in 1.x //todo maybe this is not true after after the recent discussion/change
- `web3.eth.defaultAccount` default value is `undefined` instead of `null`
Copy link
Contributor

Choose a reason for hiding this comment

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

That applies to everywhere. So insted of listing it for individual values, we should highlight it overall.

All the API level interfaces returning or accepting null are replaced with undefined

Copy link
Contributor

Choose a reason for hiding this comment

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

is extended implemented too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Regarding point 2. Sound good to me.
Let's wait for input from the rest of the team for their opinion and to see how they have structured docs.

Copy link
Contributor

@avkos avkos left a comment

Choose a reason for hiding this comment

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

approved. except for Nazar comments

@spacesailor24 spacesailor24 force-pushed the wyatt/4.x/5100-web3-eth-migration-guide branch from 4de5505 to 6b04afc Compare July 12, 2022 05:18
spacesailor24 and others added 8 commits July 12, 2022 16:02
* fix decode function. migration guide. fix unit tests

* 🏷️ Fix the contract types (#5213)

* fix contract integration tests

* add to changelog.md

Co-authored-by: Nazar Hussain <[email protected]>
* Throw error for not found tx in getTransaction

* Fix failing tests

* Move mock values to fixtures

* Throw error for not found tx in getTransactionReceipt

* Create dedicated error

* Create dedicated error code
* ⬆️ Update ethereum-cryptography package

* Apply suggestions from code review

Co-authored-by: Junaid <[email protected]>

* 🎨 Update the code as per feedback

Co-authored-by: Junaid <[email protected]>
* Init commit with defaultAccount

* Add some more properties

* Add more properties

* Refactor docs

* Apply some changes from reviews
* ⬆️ Update ethereum-cryptography package

* Apply suggestions from code review

Co-authored-by: Junaid <[email protected]>

* 🎨 Update the code as per feedback

Co-authored-by: Junaid <[email protected]>
* Init commit with defaultAccount

* Add some more properties

* Add more properties

* Refactor docs

* Apply some changes from reviews
@spacesailor24 spacesailor24 force-pushed the wyatt/4.x/5100-web3-eth-migration-guide branch from 68af2c4 to e595585 Compare July 13, 2022 02:06
@spacesailor24 spacesailor24 merged commit 2511583 into 4.x Jul 13, 2022
@spacesailor24 spacesailor24 deleted the wyatt/4.x/5100-web3-eth-migration-guide branch July 13, 2022 02:26
Muhammad-Altabba pushed a commit that referenced this pull request Jul 19, 2022
* Init web3_eth_migration_guide

* WIP migration guide

* WIP migration guide

* Add more methods

* WIP migration guide

* Add request accounts

* Apply some changes from reviews

* Add getHashrate as deprecated

* Remove getAccounts, as it will be fixed

* Fix hex string to bigint in docs

* Migration guide refactors

* fix decode function. migration guide. fix unit tests (#5210)

* fix decode function. migration guide. fix unit tests

* 🏷️ Fix the contract types (#5213)

* fix contract integration tests

* add to changelog.md

Co-authored-by: Nazar Hussain <[email protected]>

* Incompatible return types for 4.x implementations (#5205)

* Throw error for not found tx in getTransaction

* Fix failing tests

* Move mock values to fixtures

* Throw error for not found tx in getTransactionReceipt

* Create dedicated error

* Create dedicated error code

* Upgrade ethereum-cryptography to 1.0  (#5211)

* ⬆️ Update ethereum-cryptography package

* Apply suggestions from code review

Co-authored-by: Junaid <[email protected]>

* 🎨 Update the code as per feedback

Co-authored-by: Junaid <[email protected]>

* add net to web3 eth interface (#5217)

* web3-common/core defaults documentation (#5198)

* Init commit with defaultAccount

* Add some more properties

* Add more properties

* Refactor docs

* Apply some changes from reviews

* Upgrade ethereum-cryptography to 1.0  (#5211)

* ⬆️ Update ethereum-cryptography package

* Apply suggestions from code review

Co-authored-by: Junaid <[email protected]>

* 🎨 Update the code as per feedback

Co-authored-by: Junaid <[email protected]>

* web3-common/core defaults documentation (#5198)

* Init commit with defaultAccount

* Add some more properties

* Add more properties

* Refactor docs

* Apply some changes from reviews

Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Nikos Iliakis <[email protected]>
Co-authored-by: Junaid <[email protected]>
Co-authored-by: Oleksii Kosynskyi <[email protected]>
Co-authored-by: Nazar Hussain <[email protected]>
Signed-off-by: Muhammad-Altabba <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

4.x 4.0 related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants