search

dethcrypto / TypeChain

🔌 TypeScript bindings for Ethereum smart contracts
MIT License
2.77k stars 365 forks source link

Support for web3 1.0.0 #25

Closed linusnorton closed 6 years ago

linusnorton commented 6 years ago

Although web3 1.0.0 is still in beta it is now the default install candidate for npm so trying to use the default web3 with typechain doesn't work.

The specific problem seems to stem from a backwards incompatible API change they've made:

TypeError: web3.eth.contract is not a function
    at new TypeChainContract (typechain-runtime.js:5)
linusnorton commented 6 years ago

I have created #26 for this

krzkaczor commented 6 years ago

Hey, thank you very much for working on this issue. When I started developing TypeChain I also wanted to target 1.0 release (promises ftw) but it turned out to be buggy and we switched to 0.20.

I agree that support for 1.0 is great but I would prefer either to introduce something like --target flag and support both versions at the same time (can result in more complicated code generatation) or work on support for 1.x on a separate branch (something like "next") and when web3 1.0 is finally released make it stable. In Neufund we agreed to target 0.20.x for all our project so we need support for it anyway (at least for now).

linusnorton commented 6 years ago

I'd recommend you maintain separate branches and pin the version to the same version as web3. I would also tag the 1.0 branch as beta (https://docs.npmjs.com/getting-started/using-tags) while web3 is in beta. Coincidentally this is what they should have done, but have not.

Let me know if you don't want this and I will withdraw the PR and maintain a separate fork

krzkaczor commented 6 years ago

I would suggest following:

  1. split your PR in 2
  2. first PR would introduce general changes like switching to solcjs
  3. second would be about switch to 1.0 web3 and it would target new branch next instead of master (i think i need to create this remote manually)

What do you think?

As a side note: I thought about changing the way we generate implementation and during build time just create interfaces, implementation would be generated in runtime. This would allow to easily support multiple targets because the interface that TypeChain contracts provide wouldn't change — only implementation part.

linusnorton commented 6 years ago

Yeah sounds like a good plan, I will see if I can tidy it up tomorrow. What do you think we should do with the BigNumbers? Return them as string or add some code to cast them to BigNumber?

krzkaczor commented 6 years ago

@linusnorton did you find anything about the reasoning for removing BigNumber? As I mentioned before, I would prefer that both targets share the very same API. So we will need to manually wrap strings in BigNumber instances.

linusnorton commented 6 years ago

Unfortunately I only found out by trial and error. There is a mention of it here:

https://github.com/ethereum/web3.js/issues/1042

It seems like it is so that people can choose to use BN or BigNumber.js

krzkaczor commented 6 years ago

That's so typical for web3js :D Yeah, I am still up for supporting BigNumber anyway. That at least should be straightforward.

linusnorton commented 6 years ago

Great, I will try to take a look at revising the PR tomorrow evening.

linusnorton commented 6 years ago

@krzkaczor the PR is now split into two parts. If you could make a next branch then I can re-target the web3 1.0 PR. I decided to switch BigNumbers to strings in order to reflect the changes made to web3 1.0.

krzkaczor commented 6 years ago

@linusnorton i have created the next branch.

I decided to switch BigNumbers to strings in order to reflect the changes made to web3 1.0.

I believe it won't work for a long term as we want to keep TypeChain interface stable between different web3 versions etc. For now, it's fine since it's the very beginning of work on support for web3 1.0

When it's merged I am gonna release it to npm with appropriate version/tag. I will let you know.

First, Let's merge pr to master. Then I will update next branch and then we will merge the other pr. Thanks for your time!

linusnorton commented 6 years ago

Okay, when you are happy with the solcjs branch and it gets merged, I'll update this PR and we can merge it too.

krzkaczor commented 6 years ago

@linusnorton thanks for the first pr, good work! I fixed docs, and updated travis file in separate PR (already merged). Also, next branch is in sync with master now. Please change target branch of your second PR and we can work on merging it.

0xean commented 6 years ago

Is there an expected release date for compatibility with web3 1.0?

krzkaczor commented 6 years ago

@pelsasser please check out next branch.

I plan to rewrite TypeChain in a more modular way in a couple of next days. This will allow people to provide they own templates and generate typings for web3 1.0 or ethers.js or whatever is needed.

0xean commented 6 years ago

awesome, that is great to know. Any estimated release date?

krzkaczor commented 6 years ago

@pelsasser I decided to write down Typechain Masterplan here: https://github.com/krzkaczor/TypeChain/issues/71

I am on holidays now and this is my #1 priority so gimme few days :->

0xean commented 6 years ago

@krzkaczor - thanks! sorry to bug you on vacation! If we can assist, please let us know. We are enjoying using typechain!

krzkaczor commented 6 years ago

@pelsasser no worries ;) Glad to hear that you like it!

There will be some breaking changes when ts-gen is done, but I think it brings some really great benefits. I am gonna keep you posted. For sure, I am gonna need some help around providing and maintaining templates for web.js 1.0.

Btw. after the migration is done you would configure Typechain (and any others plugins) like this: https://github.com/krzkaczor/ts-gen/blob/master/example/ts-gen.json (and btw ts-gen already works with typechain!) I would appreciate some feedback round from users before its stable.

vs77bb commented 6 years ago

Hi @krzkaczor this issue has come up as a Gitcoin Request, which means @StevenJNPearce would like to work on the issue for $200. Would you mind if we fund it from our budget?

StevenJNPearce commented 6 years ago

@vs77bb I was acutally hoping to get this funded and have someone else pick it up, or @krzkaczor could take the funding, as personally I'm not entirely clear on the work that needs to be done to implement it.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to it.

vs77bb commented 6 years ago

Thanks for clarifying @StevenJNPearce... my mistake, thanks for simply bringing this to our attention. Great candidate for a Gitcoin bounty.

@krzkaczor Just funded this one! If you'd like a contributor to pick it up, I hope some Gitcoiners will be on the way soon. However, you could also 'Start Work' yourself and claim the bounty once this PR is merged.

If you'd furthermore like to discuss funding options for TypeChain via Gitcoin, happy to discuss on Slack! Hope you're doing well 🙂

krzkaczor commented 6 years ago

Hey, thanks! I will describe the details of what needs to be done when I work on my computer.

rsercano commented 6 years ago

Hello @krzkaczor I'm interested in this, would you mind telling the details to me ? (for what has to be done)

Naggertooth commented 6 years ago

Hi, guys. At the gitcoin this issue described as Traditional It means that worker should be approved and he will have a chance to get the bounty But @vs77bb said

However, you could also 'Start Work' yourself and claim the bounty once this PR is merged.

It means that type of this issue is contest. And if you need it to be done sooner - there is an option for setting expires date

I mean for somebody (for example: for me) this can be important. @krzkaczor, can you please take a look of it?

krzkaczor commented 6 years ago

First of all, thanks for funding this issue @StevenJNPearce.

As for what needs to be done:

I will mark this issue as something I work on till 10.09 (Monday). If not done until then I will ask gitcoin contributors for help.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 11 months, 4 weeks from now. Please review their action plans below:

  1. krzkaczor has been approved to start work.

    As described here: https://github.com/krzkaczor/TypeChain/issues/25#issuecomment-413014871

Learn more on the Gitcoin Issue Details page.

vs77bb commented 6 years ago

@krzkaczor Sounds great!

@Naggertooth This is indeed a traditional bounty - one person works, one gets paid out. We just knew we would approve @krzkaczor to work on it first if he'd like, because he is the maintainer of the repo. Be on the lookout for more issues + info on the thread here as things progress 🙂

krzkaczor commented 6 years ago

@vs77bb Progress can be tracked here: https://github.com/krzkaczor/TypeChain/pull/88 I will still need few more days (or more like afternoons ;)) to close it but basics are already done.

The first version will require ugly cast to force contract type. I will need to create PR to @types/web3 to fix this.

vs77bb commented 6 years ago

@krzkaczor Awesome, thanks for the update! CC @StevenJNPearce for his reference as well 👍

krzkaczor commented 6 years ago

It's done and released as 0.3.2 📦 CC: @StevenJNPearce, @vs77bb, @pelsasser.

As I mentioned before, if you used typechain before 0.3 version (legacy target) you need to manually fix your code. Basically now typechain generates pure typings on target library, no wrappers. Hopefully, it should be easy process — tsc should find all breaking changes for you.

0xean commented 6 years ago

WOOT! Nice work! cc: @MARKETProtocol/core

krzkaczor commented 6 years ago

@pelsasser great to hear that! Please, let me know if you encounter any problems using it. I am gonna submit changes to @types/web3 in upcoming days to get rid of the ugly cast.

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work for 200.0 DAI (200.0 USD @ $1.0/DAI) has been submitted by:

  1. @krzkaczor

@vs77bb please take a look at the submitted work:

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @krzkaczor.

vs77bb commented 6 years ago

@krzkaczor Great work! This was a fun use of Gitcoin for us, please do feel free to use Gitcoin Requests in the future if something comes up on your repo and you could use the help externally 🙂

gitcoinbot commented 6 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to this issue has been approved & issued to @krzkaczor.

krzkaczor commented 6 years ago

@vs77bb wow, thanks! It's the very first time that I used gitcoin and I am surprised how pleasant process it was 👍