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

Ganache shouldn't assume that every transaction with data is a contract call #117

Closed
bordalix opened this issue May 25, 2018 · 16 comments
Closed

Comments

@bordalix
Copy link

bordalix commented May 25, 2018

I think I found a bug, where Ganache assumes that every transaction with data must be a contract call, which forbids to send transactions with data to non-contract accounts.

Imagine I'm sending a transaction from address1 to address2 (both non-contract accounts) with some data on it.

The function _transactionIsContractCall will return true since the transaction has some data, defining it as a contract call (which is wrong).

On line 832 of the same file, since the _transactionIsContractCall function returned true, Ganache will now expects for the transaction to have some code, which it don't (since I'm not calling any function, I'm just sending some Ether to address2), so a new TXRejectedError will be raised, rejecting the transaction.

Shouldn't transactions to non-contract accounts be able to send some data?

Here is what the Yellow Paper states on point 4.2:

There are two types of transactions: those which result in message calls and those which result in the creation of new accounts with associated code (known informally as ‘contract creation’). Both types specify a number of common fields:
...
Additionally, a contract creation transaction contains:
init: An unlimited size byte array specifying the EVM-code for the account initialisation procedure, forma formally Ti.
...
In contrast, a message call transaction contains:
data: An unlimited size byte array specifying the input data of the message call, formally Td.

Is my understanding that the Ethereum protocol doesn't distinguish between message calls to External Owned Accounts (personal accounts) from message calls to Contract Accounts, so we should be able to send data in any message call. And if my understanding is correct, then this behaviour from Ganache should be considered as an incompatibility bug.

@bernardpeh
Copy link

I noticed that as well and submitted a pull request.

#119

@guilhermepozo
Copy link

I'm trying to send a transaction with some data to an account (not a contract) and i'm receiving:

Attempting to run transaction which calls a contract function, but recipient address 0x6e2e3accbe4b1320107118147a7cb6f5b689ea2f is not a contract address.

Ethereum "blocks/disable" transactions (Acc2Acc) with data? It's normal?

@davidmurdoch
Copy link
Member

davidmurdoch commented Aug 14, 2018

Another case where this contract check becomes a problem is if the specified blockNumber is propagated through to createFakeTransactionWithCorrectNonce like in area@6848a95

At first glance this commit looks like it should be correct. Unfortunately, some tests fail when we are doing an eth_call to a contract on a specific block at a point when this contract doesn't/didn't exist (like on the "earliest:" block). Specifically: https://github.com/trufflesuite/ganache-core/blob/4d157067dcc9445313af8dd719f609452e2ba63c/test/requests.js#L762

Additionally, calls to addresses that don't exist (and/or don't return a value) should return 0x, not 0x0 as we are doing now.

@emurmotol
Copy link

Any news about this? I'm also having this issue #878

@davidmurdoch
Copy link
Member

@emurmotol Not yet. It's on our high-priority list! We'll be sure to update this issue when the fix is in.

@benjamincburns
Copy link
Contributor

@bordalix et al:

@adibas03 kindly submitted a PR to correct this issue, however in the process of validating that such a change reflects the rest of the ethereum ecosystem I've failed to surface how it would be possible to make use of arbitrary data tacked onto a transaction sent to a simple (non-contract) account.

I can't see anything in the Yellow Paper so far which strictly prohibits adding in message call data to a transaction to a simple address, but I also can't see a mechanism which makes this a useful activity. That is, I can't find anything which allows you to read this data back out. That is, eth_getTransactionByHash and similar don't return the data field of the transaction input, and neither does eth_getTransactionReceipt.

Assuming I'm not missing something and there really is no direct way to read it back out from the RPC layer or from within a contract, I'd rather we continued to warn users off from it.

@benjamincburns
Copy link
Contributor

Just my luck! As soon as I wrote that comment @davidmurdoch & @seesemichaelj pointed out to me that the field I was looking for on the RPC response was input. I've accepted @adibas03's PR, and as a result I'm closing this issue.

@timohe
Copy link

timohe commented Nov 1, 2018

I am still having this issue using https://hub.docker.com/r/trufflesuite/ganache-cli/ or the mac application. Is this just not updated yet?

@fforbeck
Copy link

fforbeck commented Nov 2, 2018

I am getting the same issue with ganache-core: 2.2.1. I saw the fix (#159) was merged on 27 Aug but last release was 16 Aug. Is there an ETA for the new release?

@davidmurdoch
Copy link
Member

@timohe, @fforbeck We hope to push a release out this week and get into a regular release cadence from here on out.

@fforbeck
Copy link

fforbeck commented Nov 5, 2018

Cool! Thanks @davidmurdoch

@JordyBaylac
Copy link

JordyBaylac commented Nov 24, 2018

I am still having it guys, any news?

@timohe
Copy link

timohe commented Nov 24, 2018

Works for me now with the new docker release

@davidmurdoch
Copy link
Member

@JordyBaylac, what version of ganache-core?

@JordyBaylac
Copy link

Hi @davidmurdoch, not ganache-core, but in the Mac Installer version.

@davidmurdoch
Copy link
Member

@JordyBaylac, I haven't yet released a new Ganache UI with the fixes in ganache-core. There were some regressions in the 6.2.0 release that I've been working on that have kept me from releasing a UI update, but I hope to have these all resolved early this week so I can get a new UI release out.

If you can't wait or just want to try it out you can build directly from the develop branch of https://github.com/trufflesuite/ganache, just install npm then run npm install then npm start and it should start up after a minute or so (you'll also need to have a python 2.7 executable on your path otherwise the install will fail). Feel free to report any issues/bugs you may find!

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

Successfully merging a pull request may close this issue.

10 participants