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.

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

getTransaction failing in version 5 for contract deployments #572

Closed
naddison36 opened this issue Jul 29, 2019 · 8 comments
Closed

getTransaction failing in version 5 for contract deployments #572

naddison36 opened this issue Jul 29, 2019 · 8 comments
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.

Comments

@naddison36
Copy link

When calling getTransaction for a contract deployment transaction in version 4, the to field was returned with a null value.

In version 5, the to value is calculated from the nonce and from fields if the to field does not exist. see https://github.com/ethers-io/ethers.js/blob/ethers-v5-beta/packages/providers/src.ts/formatter.ts#L308

The problem is when the nonce is of odd length. eg 0x1 or 0xf. This is because the arrayify function validates that the hex string is of even length.
https://github.com/ethers-io/ethers.js/blob/ethers-v5-beta/packages/bytes/src.ts/index.ts#L115

The full stack trace from the error

Error: hex data is odd-length (argument="value", value="0x3", version=@TODO)
    at makeError (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/errors/index.js:109:17)
    at throwError (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/errors/index.js:120:11)
    at Object.throwArgumentError (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/errors/index.js:124:12)
    at Object.arrayify (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/bytes/index.js:76:20)
    at Object.getContractAddress (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/address/index.js:135:44)
    at Formatter.Object.<anonymous>.Formatter.contractAddress (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/providers/formatter.js:183:26)
    at Formatter.Object.<anonymous>.Formatter.transactionResponse (/Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/providers/formatter.js:259:40)
    at /Users/nicholasaddison/Documents/workspaces/eea-ethers/node_modules/@ethersproject/providers/base-provider.js:635:50
    at process._tickCallback (internal/process/next_tick.js:68:7)
@wighawag
Copy link

wighawag commented Sep 5, 2019

Hi, get the same issue, whenever I try to deploy a contract with v5

@ricmoo ricmoo added the investigate Under investigation and may be a bug. label Sep 5, 2019
@wighawag
Copy link

wighawag commented Sep 5, 2019

By the way, I can fix the issue by adding
, {allowOddLength: true} to the call to arrayify at

let nonce = stripZeros(arrayify(transaction.nonce));

But not sure it will remove some necessary validity checks

@wighawag
Copy link

wighawag commented Sep 7, 2019

Hey @ricmoo I wanted to link ethers as a git dependencies but because ethers use lerna, it can't be done by simply providing the git repo url to the package.json (as usual). The root folder being called "root" and this cause issue

Is there any way arround ?

The only thing left for merging my code to switch to ethers v5 is this issue and I made a fix here https://github.com/wighawag/ethers.js/tree/wighawag-patch-1 that would work for me.

@ricmoo
Copy link
Member

ricmoo commented Sep 8, 2019

I don’t quite understand what you are trying to do. What do you mean “link”?

I can add the repo in the package.json, but it doesn’t make sense to publish the repo to npm or anything. I could given it a name under the org, like @ethersproject/root, but I need to know more about the use case.

@ricmoo ricmoo added bug Verified to be an issue. next version on-deck This Enhancement or Bug is currently being worked on. and removed investigate Under investigation and may be a bug. labels Sep 8, 2019
@ricmoo
Copy link
Member

ricmoo commented Sep 8, 2019

This should be fixed now in 5.0.0-beta.157. Please try it out and let me know if there are any issues.

Thanks! :)

@ricmoo ricmoo removed the on-deck This Enhancement or Bug is currently being worked on. label Sep 8, 2019
@wighawag
Copy link

wighawag commented Sep 9, 2019

I can confirm it fixes it!

What I was asking was the following:

Normally if you have a npm package whose source code is on git. you can fork it make changes and then use your own fork git uri as the version to use for the npm package in question.

With lerna this is not possible and was wonderring if there was other options.
What I did was to extract the address package into its own repo so I could do that : https://github.com/wighawag/ethersproject-address

@ricmoo
Copy link
Member

ricmoo commented Sep 9, 2019

Oh! I don't think there is an easy way to do that with a Lerna based package. I may be able to add a main of ./packages/ethers/lib/index.js, not sure if that would work though. You can try in your own fork if you'd like. If it works I may consider adding it.

You can also use the normal npm link though. Clone the project, and then from the project folder, run npm install && npm run bootstrap. This will configure all the internal symlinks for you.

Then you can, for example, cd ethers/packages/bytes && npm link to create a local link to the bytes package, and from your own package, use npm link @ethersproject/bytes. This will now use your local changes.

This doesn't really work on environments like Heroku, but for local builds helps.

@ricmoo
Copy link
Member

ricmoo commented Sep 9, 2019

Closing this now, but if there are any more issues, please feel free to re-open.

Thanks! :)

@ricmoo ricmoo closed this as completed Sep 9, 2019
@ricmoo ricmoo added the fixed/complete This Bug is fixed or Enhancement is complete and published. label Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified to be an issue. fixed/complete This Bug is fixed or Enhancement is complete and published.
Projects
None yet
Development

No branches or pull requests

3 participants