Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Fix for deployment transactions without a to address. #87

Closed
wants to merge 1 commit into from

Conversation

ricmoo
Copy link

@ricmoo ricmoo commented Mar 15, 2018

Fix for #425.

Transactions without a to should have null, not 0x0000...0000.

I believe this may be an issue for other values in this function, but this is the only one I have confirmed (and tested against. It is causing issues in ethers.js, since it is a valid address, the creates is not being populated properly.

@benjamincburns
Copy link
Contributor

@ricmoo thanks for this change! Before I accept, can you please add a test which would have failed before the change was made, but passes now that it is applied?

Aside - pending tests are tests which we've marked as ignored. They represent incomplete behavior, or behavior which we've decided to accept as temporarily broken, usually due to the fix being blocked by some other change that's already in progress.

Copy link
Contributor

@benjamincburns benjamincburns left a comment

Choose a reason for hiding this comment

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

Please add a test, per other comment.

@benjamincburns
Copy link
Contributor

Original submitter hasn't responded in several months - we'll keep the original issue around and work it into our backlog.

@ricmoo
Copy link
Author

ricmoo commented Aug 13, 2018

Thanks. I have not had time to create the required test cases.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants