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

[FIX] Refine isContract call to support Present design #158

Closed
wants to merge 1 commit into from

Conversation

adibas03
Copy link
Contributor

Fixes #117
@bernardpeh created #119 which completely removes the contract call check, which was closed as tests were not passing.
This handles the possible scenarios under which it is a contract call and only checks for for code presence if it should already exist.

@adibas03
Copy link
Contributor Author

adibas03 commented Aug 24, 2018

@benjamincburns I don't know if this would be a preferred solution or just completely removing as in #119

@benjamincburns
Copy link
Contributor

Thanks for the contribution, @adibas03! I think the right way to fix this is to remove the check, as in #119. My desire in adding the check in the first place was to give people feedback during development when they sent would-be contract call transactions to simple (non-contract) addresses. It seems that this check has caused more pain than it has alleviated, however.

}
});
// Contract just being deployed, code yet to exist at adress
if (!rawTx.to || rawTx.to === '0x0' || rawTx.to === '0x' || rawTx.to === '0') {
Copy link
Contributor

Choose a reason for hiding this comment

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

FWIW, I think contract deployment only happens when the to field is completely missing. It's completely valid to send a transaction with data to address zero.

Copy link
Contributor

Choose a reason for hiding this comment

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

That said, to is a DATA field (per the RPC wiki), so the zero-length value of 0x should probably also be considered a contract deployment.

@benjamincburns
Copy link
Contributor

@adibas03 I'm going to close this one for now but I strongly encourage you to submit a new PR that picks up where #119 left off.

@adibas03
Copy link
Contributor Author

@benjamincburns
Thanks, done just that. #159

@adibas03 adibas03 deleted the fix/not-contract branch August 25, 2018 08:25
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.

Ganache shouldn't assume that every transaction with data is a contract call
2 participants