Web3.js planning

[sshelton76]

Description

I developed a project based on web3js and @nivida keeps fast tracking breaking changes. The cost to our development budget thus far since he has taken the lead is in excess of $150k

Expected behavior

When a project, especially a framework or tool that the entire world is trying to build upon, is in beta, the expectation when pulling in the beta release is to fix existing bugs without making ANY breaking changes unless they are immediate, severe security concerns that cannot be addressed in any other way.

If something is broken and simply cannot be fixed without breaking contract / API, then you are supposed to deprecate the broken thing and give developers time, such as 90 days to upgrade their code to the new, shiny hopefully unbroken thing. This has not been occurring on this project and the costs to my development budget alone have exceeded $150k since December just trying to keep up.

Actual behavior

It appears that @nivida has been making broad sweeping, breaking changes without any consultation with the community which is trying to build on top of this thing. The most recent one effecting my projects is #2570 wherein a change was made to suddenly start returning the BigNumber object again, instead of the number as string as it had been for at least the past year.

This change causes enormous problems for us, and I am certain it does for anyone who is putting serious time, money and effort into developing on top of web3js. Yet the only justification is he couldn't get react to build.

First off, not everyone is using webpack, or react as part of the build process.
Secondly, most of the people who are using react already had workarounds in place as evidenced by the large body of working react code @nivida managed to break.
Thirdly a long time ago it actually used to return the BigNumber object and this was changed for good reason.

The reasoning for the original change as I recall is that strings are easier to deal with and less messy to marshal around until you actually have to do math, at which time there are a number of libraries at your disposal if you have a string representation, but with BN you are stuck with BN and all of its shall we say "quirks". #2675
Did something fundamental change to remove those concerns?

In the meantime those of us who are trying to stay current while building things people and systems will interact with everyday, must now, yet again refactor enormous amounts of code and tests just to have the privilege of having some actually important bugs like transaction signing fixed.

This is not the only example...

There are many people who are still stuck on beta-37 after an arbitrary decision was made to eliminate the minified version from the release cycle, again with literally 0 warning, just a note to someone who asked where it went saying "we don't support it anymore". Then a further comment some months later saying it would return because he didn't realize how vital it was to people. #2623
It has yet to return

What is even the point of cutting a release, beta or otherwise if it must be built by the end user?
If someone wants to build from source they can just checkout the project on github and get the latest and greatest "bleeding edge" code to play with.

I'm not saying there aren't some security issues from shipping a completed and minified version, but those are minor in comparison to everyone + dog needing to build locally. At least with a minified version on a CDN, one can check the hash to verify its authenticity, directly in the browser before it loads. https://www.w3.org/TR/SRI/
Which frankly you're insane if you're not doing it already.
https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity

Not everyone wants to use webpack, it's huge, it slows down development and 90% of the things it's used for, you'd be better off using a simpler solution like yarn or insert tool dejour

But at present, without webpack there is no way to get a recent minimized version for testing. This is causing many people to fall behind and also alienating a significant number of people who could be helping to test if there were just a single thing to import, instead of having to rebuild world every 7 to 10 days.

I could go on and on, but the point is...
This is a BETA. Beta is not the time to be bold, move quickly and break things. Beta is a time to go slowly and carefully over the known bugs in order of severity and hammer them out. Either one by one or as part of a larger class of fixes. It is a time for bug reports and bug fixes, but not breaking changes.

Yes one could argue that code not quite meeting a documented spec counts as a bug. But if you're breaking things people are currently using and have been using successfully for over a year, you're doing it wrong, so please stop doing it.

Steps to reproduce the behavior

1. Look at the issues since beta-32 that have been addressed by @nivida

2. Congratulate him on the items he managed to fix without breaking other things, there are hundreds. But every release cycle something has managed to break and usually that something was working before. This is a serious problem.

3. Caution him to slow down, look long and hard at the problems and the potential solutions and be more careful in the future. We don't need a release every 7 to 10 days. We should be shooting for a max of one a month, well tested with nothing new getting broken in the process.

4. Encourage him to involve the community in any decision / fix / whatever that breaks existing functionality and to seek alternatives even when he's certain there are none.

Recommended Fix

Honestly, at this point I'm just going to maintain my own fork of web3.js in order to focus on stability. I'll just back port fixes when they actually fix their intended target without breaking existing functionality. In the meantime I'd like to see the community take a more formalized approach to software development in general that looks something like this.

1. An updated roadmap to 1.0-RELEASE needs to be discussed, formalized, adopted and released by the community. This roadmap should list all existing functionality and any planned functionality preferably in as much detail as possible.

2. A code freeze needs to be called for a short time, no new features, no breaking of existing features. Unit and integration and regression tests need to be written and updated for all functionality (currently showing 95% coverage which isn't bad, but how in depth are those tests and why is there 5% of the codebase without coverage?). Once tests have been written they need to be an integral part of the build process and commits which break any test should not be pulled in at all. From here an unfreeze can be called to implement remaining features on the roadmap, ensuring that tests are always maintained and essentially frozen in time from the developers perspective. This gives the developers using the code a contract that they can rely on to build first class products to showcase the power of the platform.

3. If a unit test needs to be updated, there needs to be discussion about what the change is and why it is necessary. A simple "this test no longer reflects the reality of our implementation", should be enough to start the conversation, but we should be talking about how and why the implementation eventually differed from the roadmap and test. What is the impact of this change? If it turns out the test was implemented incorrectly or only partially implemented then of course it should be updated. Otherwise this can be assumed to be a breaking change and should be roadmapped to a later release.

4. A roadmap to 2.0-RELEASE needs to be discussed, formalized etc as in step 1. Anything found during development of 1.0 that would break 1.0-RELEASE's testing regime should be scheduled for a 2.0 target.

The only exception to steps 1..4 should be verifiable emergency security fixes where money or other items of value are put at risk if the fix is not in immediately.

This is important, as time progresses the current development style on this project makes it harder and harder to actually use reasonably current versions. Consider for a moment, that metamask is still using 0.2x and a large part of their reasoning is exactly this moving target approach that has been the norm for so long that they literally stopped trying.

Thanks for taking the time to read this. I do appreciate @nivida and his hard work on this project. I just think it's time to formalize certain aspects of development which will lead to a stronger product overall. If nothing else I'd like to implore @nivida to open issues when he uncovers the need for potential breaking changes, but instead of shooting from the hip he should gather community feedback on the necessity of the change and try to identify potential alternatives, then give a reasonable timeframe for anything that would require deprecation.

Doing so would be a huge help to all the people who are serious about building on top of this.

[nivida]

The most recent one effecting my projects is #2570 wherein a change was made to suddenly start returning the BigNumber object again, instead of the number as string as it had been for at least the past year.

I had to return the BigNumber object because of another issue with a React native production build.
bignumberObj.toString() and you will have the same return value as before. I think it's very important that a production build with the widely used React native is working.

Thirdly a long time ago it actually used to return the BigNumber object and this was changed for good reason.

The complete BigNumber handling in the entire space has problems because some projects are using BN.js and some others are using Bignummber.js object. My goal is to change to the native BigInt implementation and to talk with Richard from ethers.js to do that too. (we use the AbiCoder from ethers.js)

again with literally 0 warning, just a note to someone who asked where it went saying "we don't support it anymore".

This was announced here: https://twitter.com/web3_js/status/1088875148206358528

If someone wants to build from source they can just checkout the project on github and get the latest and greatest "bleeding edge" code to play with.

Generated files like a minified full bundle will never be a part of this GitHub repository but I will definitely add it to the NPM package. I've removed it because of security reasons and the bundle size. I would be happy to have full control over the used dependencies in my DApp.

But at present, without webpack there is no way to get a recent minimized version for testing.

It's possible to use Web3.js without webpack on a node.js environment :-)

Beta is a time to go slowly and carefully over the known bugs in order of severity and hammer them out.

The feedback I got until now was mostly the other way around because we all are waiting for a long time on the stable release.

Could you give me a list of breaking changes by side of these two?

We don't need a release every 7 to 10 days.

We need that otherwise, we will not have a stable release in the first half of this year. I was not just doing releases because of fun. Mostly there are 100-200 commits and 10-20 closed issues. I was working as much as I can to improve this library.

Encourage him to involve the community in any decision / fix / whatever that breaks existing functionality and to seek alternatives even when he's certain there are none.

I invited the whole community in the beta.38 release to be involved in the draft gist files I've referenced there and I've also invited the community to give feedback over the issue list. I'm not sure if there is anything more I could do.

An updated roadmap to 1.0-RELEASE needs to be discussed, formalized, adopted and released by the community. This roadmap should list all existing functionality and any planned functionality preferably in as much detail as possible.

This is explained here: https://twitter.com/web3_js/status/1088875148206358528

If a unit test needs to be updated, there needs to be discussion about what the change is and why it is necessary.

This is possible but would slow down the process extremely. Feel free to be a part of the PR processes as some others already doing. I'm still wondering about which list of breaking changes you are talking.

A roadmap to 2.0-RELEASE needs to be discussed, formalized etc as in step 1. Anything found during development of 1.0 that would break 1.0-RELEASE's testing regime should be scheduled for a 2.0 target.

There are no breaking changes planned. The BigNumber breaking change did happen because I fixed a bug.

The only exception to steps 1..4 should be verifiable emergency security fixes where money or other items of value are put at risk if the fix is not in immediately.

Or a non-working production build of a widely used framework.

This is important, as time progresses the current development style on this project makes it harder and harder to actually use reasonably current versions.

Should we go back to the old development style where close to no one worked on this project?

Consider for a moment, that metamask is still using 0.2x and a large part of their reasoning is exactly this moving target approach that has been the norm for so long that they literally stopped trying.

Metamask is still using 0.2.x because they want to use the last stable version for their in production used product.

It's so hard to do the correct thing for everyone. Some months ago anyone was blaming me because of the missing process and now I get blamed for fixing and improving the library. I really don't know how big my motivation will be in the next six months if this blaming doesn't stop.

[szerintedmi]

I think it's in everyone's interest to make this work and everyone is trying hard. Going personal doesn't help. But closing a discussion in 5 hours won't solve the underlying issues either.

I can certainly relate to the general issue: We regularly try to upgrade because we want fixes in the newer versions (e.g. websocket connections dropping in every 30-60 minutes) . But with each release we face different blocking issues so we stuck at beta33 and 36 in our modules.

In general:

• give time for discussions about changes. I can see a lot of experienced developers here - just listen to their input
• I didn't dig into the web3 codebase and tests but I'm surprised that some of these breaking changes didn't break the tests. Are the tests in sync with docs and with expected/documented behaviour? Do you have integration tests with ganache? I would start with tests so that any refactor could be much safer. I seen people offering help in writing tests, myself did that too.

Some specific examples:

• ganache confirmations stopped working from beta49: calling contract.method.send() only triggers .on('transactionHash') #2680, Increasing of the block number for the confirmation workflow over HTTP is wrong. #2661, .Then() and await on .Send() never completing in VueJS + MetaMask #2640, Returning of BigNumber objects instead of strings #2570: "set transactionConfirmatinNumber to 1" is not resolving the underlying issue. It was working before and our tests are checking confirmations for a reason. If you decide to drop ganache confirmations support
  then we can code it in our tests. But why drop support when it seems it's a bug and was found by @eduardonunesp

• PromiEventissues: Contract deploy promise never returns in beta.52 #2681, Tx never resolves on ganache and no confirmation or receipt event triggered beta50-52 #2652, .Then() and await on .Send() never completing in VueJS + MetaMask #2640, Can't get tx receipt of reverted transaction #2612, Contract deploy promise never returns in beta.52 #2681. Not critical but very confusing. Maybe implement some basic tests first and discuss that ?

• We didn't brave into the effect of the BigNumber change yet - all our tests are breaking before even getting to that part. I want to see community consensus about solution before we change everything (again.. see utils.fromWei & toWei returns string when I pass BigNumber #1042)

• defaults are breaking in different parts with each version (gas, networkId, options etc.). Eg. signTransaction fails with Method eth_chainId not supported. (beta51) #2610 Usually we find workarounds but it is a pain to debug from the not very helpful error messages / stack trace

[nivida]

But closing a discussion in 5 hours won't solve the underlying issues either.

This is true but the base of the discussion was not good. We can create after the discussion here a clean issue which is addressing the transparency issue of the Web3.js project planning.

But with each release we face different blocking issues so we stuck at beta33 and 36 in our modules.

All changes from 33-37 are changes I've done in the order of the old maintainer and I didn't release them.

Are the tests in sync with docs and with expected/documented behaviour?

Yes, they are in sync.

Do you have integration tests with ganache?

Web3.js never had integration tests but I will definitely create them as I've mentioned in the release announcement of beta.38.

I would start with tests so that any refactor could be much safer.

There will be no other refactoring until the stable release. I've refactored the code to have a clean and maintainable software architecture which provides more stability.

I seen people offering help in writing tests, myself did that too.

Yes, some days ago a friendly contributor contacted me over Twitter to start with integration tests.

give time for discussions about changes. I can see a lot of experienced developers here - just listen to their input

I'm always listening to the feedback of all developers here because the project should go in a direction the community want's to have it. I'm also sharing my knowledge and I hope anyone else here is doing it too. :)

ganache confirmations stopped working from beta49: #2680, #2661, #2640, #2570: "set transactionConfirmatinNumber to 1" is not resolving the underlying issue. It was working before and our tests are checking confirmations for a reason.

There isn't an underlying issue. I've fixed the issue #2062 which enforces to use the Promise or the EventEmitter but not both in combination.

If you decide to drop ganache confirmations support
then we can code it in our tests. But why drop support when it seems it's a bug and was found by @eduardonunesp

I didn't decide to drop support for ganache. Web3.js is following up the latest JSON-RPC spec and ganache, for example, doesn't have implemented the chainId method which got released some years ago in Geth and parity. The confirmations are working over a WebSocket connection and we only have the issue with increasing of the block number over an HTTP connection. But this will get fixed this week.

PromiEventissues: #2681, #2652, #2640, #2612, #2681. Not critical but very confusing. Maybe implement some basic tests first and discuss that ?

As explained in close to every issue regarding this was it required to enforce the developers to use Promise or EventEmitter based return values and not a combination of both.

We didn't brave into the effect of the BigNumber change yet - all our tests are breaking before even getting to that part. I want to see community consensus about solution before we change everything (again.. see #1042)

I would like to see a community consensus too. But currently, there are just a few involved contributors as for example @levino, @OFRBG, @joshstevens19, @princesinha19, and you.
The production builds with React native and others which are mangle all the object names were broken and that's why I've done that breaking change.

defaults are breaking in different parts with each version (gas, networkId, options etc.). Eg. #2610 Usually we find workarounds but it is a pain to debug from the not very helpful error messages / stack trace

The JSON-RPC method eth_chainId got implemented some years ago in Geth, Parity, and ETC related nodes. I've told ganache about that before I've released it:

[levino]

I do not agree with everything OP says (especially the part about test coverage, going for 100% is a waste of money) but I also do not find that OP is totally out of bound. It would of course be easier if he could focus his criticism on an organisation instead of a single person but currently it looks like you are the only one to go to @nivida .

What he is right about is that currently this project is not getting more stable but more unstable. More issues are created than resolved. This is not an uncommon thing for a large software project because getting things stable is hard. One has to be very careful and to resist the urge to redesign code (and as such break a lot of things).

I will not comment on the individual issue with bignumbers, PromiEvents and so on here because I like to keep things focused. Any discussion about these issues in this issue here should stop immediatly. However I agree with OP that it is necessary to carefully and in detail write down what exactly is required to consider web3@1.0.0 to be finished and production ready in regards of features and then get community consensus and then freeze that document and start working on impelementing the agreed upon features. Until today I do not know where to look for this roadmap.

And also one remark by me: There must only be one point of truth. One single page or document. Twitter, Gitter chats etc. are not okay. It is not public or easily overlooked. I refuse to use Twitter, it is a never ending noise for me. I also wont screen Gitter chat logs for potentially interesting information about breaking changes to web3.

So I think that OP could have done this in a less personal manner but his baseline is correct: Please stop implementing features that have not been declared as "to be implemented" a long time ahead. Focus on writing down all the things that are to be implemented for web3@1.0.0.

[szerintedmi]

I agree to split specific issues from here - maybe create a placeholder for each to be discussed
individually (eg. BigNumber handling, PromiEvent expected/actual behaviour, Integration tests etc..)?

While those being discussed I'm more than happy to join in to help with some basic integration tests as it's crucial for stabilising. @micahriggan idea on twitter to dockerise them is great, I'm chipping in there.

[nivida]

I've made a checklist for the stable release some months ago:

Until stable:

1.0-rc1

• EthereumProvider (EIP-1193)
• Last features (has to be defined)
• Bug fixes (all issues with the label bug or clarification needed)
• Documentation improvements
• Integration tests
• Makes arrays and strings and longer than bytes32 filterable via topics: Arrays and strings as event topics #344
• Contract events string bytes and arrays will store only sha3 for indexed Params.
• Implement polling for missed logs after reconnecting.
• showcase light client limitations

1.0 LTS - 15.06.2019

• Bug fixes

Release cycles:

Patch cycles LTS:

• Each month smaller patch releases with the current merged PR's and bug fixes. (expect hot-fixes)

Minor cycles LTS:

• All three months a minor release with possible smaller breaking changes and new features.

End-of-life

• 30.06.2019 0.20.x

[nivida]

Let us first talk about my list. We can later create a Web3.js planning issue where we just have a discussion about the planning.

[levino]

What about just using the project board here in github? Kick out all issues and PRs (imo PRs should not be in the kanban board) and then readd the ones you want to finish. We can then go into the issues, discuss whether or not they are ready for dev and also whether or not it makes sense to include them into the stable release. Maybe also add two columns in front of "To do": Proposed and accepted. Then we can start on working them off.

[levino]

All coding should be reduced to bugfixing while this process is not finished. Once all issues have been moved to "accepted" or kicked out again we can make one big issue where we list all the issues that were agreed upon so to freeze the list.

[levino]

BTW: I think that most issues are not ready for dev so that will also take some work and time to make them so. Baseline is: Every developer must understand what is to be done (requirements) and how the result can be verified (QA) from the issue without having any "deeper knowledge". I call this "context freeness".

[nivida]

@levino I've added close to all issues to the to-do list because I thought to fix all of them. But yes it's probably better to discuss them first. @szerintedmi @sshelton76 do both of you like this idea?

[nivida]

Updated the GitHub project: https://github.com/ethereum/web3.js/projects/1

[levino]

Please add a new column "todo". I fear that you will start working on issues in "accepted todos" before all "proposed todos" have been adressed and then we will never be able to freeze a list. This is why I suggested five columns in total:

1. Suggestion
2. Accepted
3. To do
4. In progress
5. Done

[nivida]

I thought I will go back to coding when we have a frozen list of issues and until then I will write the Web3 Module API documentation etc. @levino

[levino]

Are you sure you want us to review 106 issues? I mean obvious bugs do not need to be discussed. We should only discuss new feature requests...

[nivida]

No, I will move them later. I'm currently finishing the open PR for the Web3 Module API documentation. Feel free to add a comment to the issues with a label enhancement or feature request if you would like to have one of them in the accepted To Do list.

I will later write all of them down in an issue as a first draft to start a bigger discussion about the next steps until the stable release.

[sshelton76]

@nivida I like this progress. It is much better. Please don't feel that I am or was blaming you. You are front and center because it's your name on the release, nothing more. Please do not let my criticism weigh on you. There was no intent to blame you, I'm blaming process here. This is a big project, you have done excellent work and it isn't you I'm having problems with, it's just the process. While we should never put process before people, we should have a process in place so that people who are relying on this project can feel safe bringing in each release so they can help with testing without worrying about their entire code base needing to be modified.

I disagree with you on the bignumber issue for react, I feel there is/was a better way to address it. However I digress, you are the leader on this project and therefore the final say is yours. I only ask that if a breaking change must occur in the future that you solicit feedback and discussion for a few days before considering whether or not to implement it.

The steps you mentioned here address all the issues in the topic, so to my mind continuing on as you propose is a good "fix" for the reported bug :)

[nivida]

I like this progress. It is much better. Please don't feel that I am or was blaming you. You are front and center because it's your name on the release, nothing more. Please do not let my criticism weigh on you. There was no intent to blame you, I'm blaming process here. This is a big project, you have done excellent work and it isn't you I'm having problems with, it's just the process. While we should never put process before people, we should have a process in place so that people who are relying on this project can feel safe bringing in each release so they can help with testing without worrying about their entire code base needing to be modified.

Thanks! I got that probably wrong. Sorry for that.

I disagree with you on the bignumber issue for react, I feel there is/was a better way to address it.

Yes, the other solution was to rely on the private property of the BigNumber object ethers.js has. I've tested several ways and there was always a case where it didn't work as expected.

However I digress, you are the leader on this project and therefore the final say is yours. I only ask that if a breaking change must occur in the future that you solicit feedback and discussion for a few days before considering whether or not to implement it.

Yes, this is true. I'm just asking my self how to get enough feedback on proposed changes. Tweeting about each issue where a discussion is required feels like spamming.

The steps you mentioned here address all the issues in the topic, so to my mind continuing on as you propose is a good "fix" for the reported bug :)

Great! Thanks! Feel free to add a comment to the issues in the Proposed To Do's column here if you think they have to be closed for the stable release.

[biocrypto730]

A few months ago I considered opening an issue like this. But instead I had this brilliant conspiracy that the Parity team still ran Web3, and they were mad about the lost funds, so were releasing a bunch of backwards incompatible changes to Web3 to screw over the community. I just accepted this as fact and stopped developing for Ethereum completely.

But now the chain_id is still not supported by Ganache, so it seems my problem is caused by another development team not doing their job. And i believe @nivida that returning the BigNumber object could be the best option in the case he described.