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

A contract call which triggers multiple events of the same name throws error (but tx is successful) #2542

Closed
HardlyDifficult opened this issue Mar 20, 2019 · 15 comments
Labels
Bug Addressing a bug

Comments

@HardlyDifficult
Copy link

Description

When a contract has an event defined and attempts to make an external call to another contract which emits an event with the same name, the call fails.

More specifically: we are creating a contract with implements ERC721. ERC721 has the following event:

event Transfer(address indexed _from, address indexed _to, uint256 indexed _tokenId);

Our contract then attempts to call transferFrom on an ERC20 token, which emits the following:

event Transfer(address indexed _from, address indexed _to, uint256 _value)

The issue occurs when that Transfer event is emitted.

I've tested this using Truffle, Web3 beta 37, and Web3 beta 50. All of them fail, but each in a slightly different way.

Expected behavior

Calling a function which triggers multiple events which happened to be named the same should work like any other call.

Actual behavior

Web3 beta 50 throws the error below. However if you catch the error and ignore it, the transaction is successful.

Steps to reproduce the behavior

I created a sample repo which tries to demonstrate the issue using as simple of contracts as possible: https://github.com/hardlydifficult/SolidityBugInvestigation_EventsWithTheSameName

Clone the repo and run

npm i
npm run test

And see test/Test.js

Error Logs

Error: Transaction has been reverted by the EVM:
{
  "transactionHash": "0xc008bda380d341e7705974e229e119177b4daf3c9e5962c62b9a26735a3f7096",
  "transactionIndex": 0,
  "blockHash": "0x83ebce6d311fecb0f794fe1966186b2f60e407cfcb75da53a8d62c5a509a649b",
  "blockNumber": 8,
  "from": "0xf17f52151ebef6c7334fad080c5704d77216b732",
  "to": "0x345ca3e014aaf5dca488057592ee47305d9b3e10",
  "gasUsed": 62139,
  "cumulativeGasUsed": 62139,
  "contractAddress": null,
  "logs": [
    {
      "logIndex": 0,
      "transactionIndex": 0,
      "transactionHash": "0xc008bda380d341e7705974e229e119177b4daf3c9e5962c62b9a26735a3f7096",
      "blockHash": "0x83ebce6d311fecb0f794fe1966186b2f60e407cfcb75da53a8d62c5a509a649b",
      "blockNumber": 8,
      "address": "0xf25186B5081Ff5cE73482AD761DB0eB0d25abfBF",
      "data": "0x0000000000000000000000000000000000000000000000000000000000000001",
      "topics": [
        "0xddf252ad1be2c89b69c2b068fc378daa952ba7f163c4a11628f55a4df523b3ef",
        "0x000000000000000000000000f17f52151ebef6c7334fad080c5704d77216b732",
        "0x000000000000000000000000c5fdf4076b8f3a5357c5e395ab970b5b54098fef"
      ],
      "type": "mined",
      "id": "log_0x78521e4079943976c14e5055429cb2063ab0fb1966d8153a3b01d288131edcd8"
    },
    {
      "logIndex": 1,
      "transactionIndex": 0,
      "transactionHash": "0xc008bda380d341e7705974e229e119177b4daf3c9e5962c62b9a26735a3f7096",
      "blockHash": "0x83ebce6d311fecb0f794fe1966186b2f60e407cfcb75da53a8d62c5a509a649b",
      "blockNumber": 8,
      "address": "0xf25186B5081Ff5cE73482AD761DB0eB0d25abfBF",
      "data": "0xfffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe",
      "topics": [
        "0x8c5be1e5ebec7d5bd14f71427d1e84f3dd0314c0f7b2291e5b200ac8c7c3b925",
        "0x000000000000000000000000f17f52151ebef6c7334fad080c5704d77216b732",
        "0x000000000000000000000000345ca3e014aaf5dca488057592ee47305d9b3e10"
      ],
      "type": "mined",
      "id": "log_0xe26a4a5aabbe8a48919466efeb66fc102036243870d8316df06591fe71fc8791"
    }
  ],
  "status": true,
  "logsBloom": "0x00000000000000000000010000000000010000000000000000000010000000000000000000000020008000000000000000000000000000000000000000200000000000000000000000000008000000000000000000000080000080000000000000000000000000000000000000000000000000000000000000000010000000000000000000010000000000000000000000000000000000000000000080000000020000000000000000000000000000000000002000000000000000000000000000000002000000000080000000001000000000000000000000000000000000000010000000000000000000000000000000000000000000000000000000000000",
  "v": "0x1b",
  "s": "0x3e65aa768ff40efe0caa3a1ed61ea577ff5f2b9b1f38034e006d5613d12a20f4"
}
      at SafeSubscriber._next (node_modules\web3-core-method\dist\web3-core-met
hod.cjs.js:1017:32)
      at SafeSubscriber.__tryOrUnsub (node_modules\rxjs\src\internal\Subscriber
      at SafeSubscriber.next (node_modules\rxjs\src\internal\Subscriber.ts:209:
14)
      at Subscriber._next (node_modules\rxjs\src\internal\Subscriber.ts:139:22)
      at Subscriber.next (node_modules\rxjs\src\internal\Subscriber.ts:99:12)
      at TransactionObserver.emitNext (node_modules\web3-core-method\dist\web3-
core-method.cjs.js:438:16)
      at _callee2$ (node_modules\web3-core-method\dist\web3-core-method.cjs.js:
408:24)
      at tryCatch (node_modules\regenerator-runtime\runtime.js:45:40)
      at Generator.invoke [as _invoke] (node_modules\regenerator-runtime\runtim
e.js:271:22)
      at Generator.prototype.(anonymous function) [as next] (node_modules\regen
erator-runtime\runtime.js:97:21)
      at asyncGeneratorStep (node_modules\@babel\runtime\helpers\asyncToGenerat
or.js:3:24)
      at _next (node_modules\@babel\runtime\helpers\asyncToGenerator.js:25:9)
      at process._tickCallback (internal/process/next_tick.js:68:7)

Gists

Versions

  • web3.js: Beta 37 and Beta 50
  • nodejs: 10.15.1
  • browser: n/a
  • ethereum node: ganache 1.3.1 and ganache-cli 6.4.1

This was originally filled with Truffle here: trufflesuite/truffle#1729
But after more investigation, it appears to be a Web3 issue.

@nivida
Copy link
Contributor

nivida commented Mar 20, 2019

Thanks for opening this issue! I'll test and fix it if required asap.

@angus-hamill
Copy link

angus-hamill commented Mar 21, 2019

I've opened a PR to fix the issue whereby web3 thinks a successful tx was a revert
#2548

@dileepfrog
Copy link

I just verified angus-hamill's fix, I am running into the same issue against an actual geth node vs ganache on beta 50

@SwissArmyBud
Copy link

Am having a duplicate issue, my TX receipt is {... "status": true ...} and parseInt(true) resolves to NaN, which is boolean falsy in JS. This happens for me with contract creation under betas 48 & 50. I fixed with Boolean(parseInt(receipt.status) || receipt.status) but @angus-hamill has a more appropriate method in the PR for production, it looks like.

@angus-hamill
Copy link

I found the real source of this issue if you check the PR. Turns out it is actually a web3js problem and not just caused by supporting ganache

@dileepfrog
Copy link

@angus-hamill would you mind re-opening this PR, currently web3 throws an error for all tx against a live Geth node which is a non-starter

@angus-hamill
Copy link

angus-hamill commented Mar 26, 2019

I don't have permission to unfortunately!
@nivida is the only one I think

@devstein
Copy link

@HardlyDifficult Also running into this issue

@SwissArmyBud
Copy link

This is an ongoing issue, parsing returned receipts is currently being done incorrectly. I'm handling it in my project as detailed with my comment here but the code can also be "fixed" by patching the responsible observer function.

My file at /node_modules/web3-core-method/lib/methods/transaction/AbstractObservedTransactionMethod.js just needed the following change:

// From this
hasRevertReceiptStatus(receipt) {
  return Boolean(parseInt(receipt.status)) === false && receipt.status !== undefined && receipt.status !== null;
}
// To this
hasRevertReceiptStatus(receipt) {
  if(receipt.status === undefined || receipt.status === null) return false;
  return Boolean(parseInt(receipt.status) || ( receipt.status === true )) === false;
}

@SwissArmyBud
Copy link

This also appears to be related to this issue which was apparently fixed (?) already. Possible revert?

@patitonar
Copy link

Running into this issue on Beta 50

@nivida
Copy link
Contributor

nivida commented Mar 27, 2019

@SwissArmyBud This isn't an issue of web3.js. The status field should have the value 0x1 or 0x0 as documented in the geth and parity documentation. I've tested it with geth and parity and a transaction.

https://wiki.parity.io/JSONRPC-eth-module#eth_gettransactionreceipt
https://github.com/ethereum/wiki/wiki/JSON-RPC
https://infura.io/docs/ethereum/json-rpc/eth_getTransactionReceipt
https://eips.ethereum.org/EIPS/eip-658

@nivida nivida closed this as completed Mar 27, 2019
@SwissArmyBud
Copy link

SwissArmyBud commented Mar 27, 2019

@nivida This is an issue with THIS line in the incoming receipt formatter being passed to THIS line which I mentioned in the PR post I put up.

Currently this is probably breaking the following Issues ( ONE, TWO, THREE, FOUR ) and is because Web3 is calling Boolean(parseInt(receipt.status)) on a boolean output from its own Formatter.js, linked above. My receipt from Geth is the following BEFORE the formatter:

{ blockHash:
   '0xb8008b497b168b19b0472f0ad642dd7a8185fb147dd4d3940f0cf4ff15c1a7f0',
  blockNumber: '0x3',
  contractAddress: '0x0dc37d909182ed9f0a78ade5d9227727bf174090',
  cumulativeGasUsed: '0x115e55',
  from: '0x8cea11ca35098a01c76be7c04075ab1a1a8523ac',
  gasUsed: '0x115e55',
  logs: [],
  logsBloom:
   '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
  status: '0x1',
  to: null,
  transactionHash:
   '0x4ccced375bf4fa3d7ebe5c7021348ab77b2090c1b9779d3807e9b3de8e643815',
  transactionIndex: '0x0' }

You will notice it follows the correct HEX encoding for the "status" field.

My receipt from Geth is the following AFTER the formatter:

{ blockHash:
   '0xb8008b497b168b19b0472f0ad642dd7a8185fb147dd4d3940f0cf4ff15c1a7f0',
  blockNumber: 3,
  contractAddress: '0x0DC37D909182Ed9F0a78ADe5d9227727Bf174090',
  cumulativeGasUsed: 1138261,
  from: '0x8cea11ca35098a01c76be7c04075ab1a1a8523ac',
  gasUsed: 1138261,
  logs: [],
  logsBloom:
   '0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000',
  status: true,
  to: null,
  transactionHash:
   '0x4ccced375bf4fa3d7ebe5c7021348ab77b2090c1b9779d3807e9b3de8e643815',
  transactionIndex: 0 }

The formatter has now turned my API correct HEX value into a Type(Boolean).

Web3 then uses that formatted receipt to check (in the AbstractObserver, linked above) the following Boolean(parseInt(receipt.status)) === false which appears to be leftover formatting code, because:

> parseInt(false)
NaN
> parseInt(true)
NaN
> Boolean(NaN)
false

So essentially, once the transaction receipt passes through the Formatter, the Observer will NEVER see the receipt as passing the EVM revert check. This is obviously incorrect behavior.

@SwissArmyBud
Copy link

Filed a PR, in the mean time anyone who doesn't mind compiling/using their own Web3 modules can grab a copy from the fix branch.

@Anaphase
Copy link

For people looking for a workaround:

As per this comment, it seems to work if you manually encode and send the transaction.

Instead of:

contract.methods.myMethod(...input).send()

Use:

web3.eth.sendTransaction({
  to: contract.address,
  data: web3.eth.abi.encodeFunctionCall(
    contract.jsonInterface.getMethod('myMethod').abiItem,
    input,
  ),
})

(Note that the snippets above are using web3@2.0.0-alpha.1 which has a slightly different ABI class/api I believe.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Addressing a bug
Projects
None yet
Development

No branches or pull requests

8 participants