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

Contract function call fails #21

Closed
iherger opened this issue Jun 5, 2019 · 11 comments
Closed

Contract function call fails #21

iherger opened this issue Jun 5, 2019 · 11 comments
Assignees
Labels
adoption-blocker bug Something isn't working

Comments

@iherger
Copy link

iherger commented Jun 5, 2019

The following call fails with an error on Alchemy (but seems to work fine on Infura):
Contract: 0x280ba3da0b8b3601c19870e5db9126a62449dcdc
Function: calcGav
Block: 7648899
(other contracts in other blocks may be affected as well)

The handler code is: https://github.com/melonproject/melon-subgraph/blob/master/src/mappings/PriceSource.ts#L106

The error message from Alchemy is
Trying again after eth_call RPC call failed (attempt #140) with result Err(CallError(ErrorMessage { msg: "RPC error: Error { code: ServerError(-32015), message: \"VM execution error.\", data: Some(String(\"Reverted 0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000125072696365206973206e6f742076616c69640000000000000000000000000000\")) }" })), block_hash: 0xb5896d5973cd0f85084e2be924a3af08e36c39e5259579e52f00c15cb1b97317, block_number: 7648899, updates: 657

@Jannis
Copy link
Contributor

Jannis commented Jun 5, 2019

Turns out that Infura (and other nodes) return the same error, just in a less obvious form:

They return

{"jsonrpc":"2.0","id":1,"result":"0x08c379a0000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000125072696365206973206e6f742076616c69640000000000000000000000000000"}

The 08c379a stands for "Revert reason". The rest of the data is actually the bytes representation of a string that says "Price is not valid".

So this particular calcGav call does fail indeed, on all nodes.

@iherger
Copy link
Author

iherger commented Jun 5, 2019

We had some issues with prices in the past indeed, and that may be the reason.

Funny enough, the call didn't seem to fail the subgraph when using infura, but it may have returned weird numbers, see the lines just below the original function call: https://github.com/melonproject/melon-subgraph/blob/1901770acb5379a7204e6bd5b4bacf693a77107c/src/mappings/PriceSource.ts#L108

@Jannis
Copy link
Contributor

Jannis commented Jun 5, 2019

I wish AssemblyScript supported either union types or try/catch. Then we could return the error back to the mapping. Right now our only option would be to return the result or null. But null is ambiguous, it could also be a correct 0 int value.

@Jannis
Copy link
Contributor

Jannis commented Jun 5, 2019

The one workaround I can think of right now is hard-coding block hashes / contract address pairs where you know that calcGav will fail. But that's not great. And there may be many?

@iherger
Copy link
Author

iherger commented Jun 5, 2019

There are quite a few blocks. I suspect that the errors we are seeing here are the errors which lead to #17 (same block, different function, which calls calcGav then).

A workaround could be to return "3963877391197344453575983046348115674221700746820753546331534351508065746944" which is what we got previously.

@Jannis
Copy link
Contributor

Jannis commented Jun 7, 2019

Assertions failing can happen in more places and it can even be desired. We need a way to allow users to handle failed calls gracefully, perhaps even do something with the error returned.

In the absence of union types, try/catch and closures, the easiest and most convenient way to make this possible could be with a Result<T,E> type akin to Rust's https://doc.rust-lang.org/core/result/enum.Result.html.

@Arachnid
Copy link

Arachnid commented Jul 1, 2019

It'd be really handy to have this, so we can call contracts we don't trust that may revert.

@Jannis Jannis self-assigned this Jul 2, 2019
@Jannis
Copy link
Contributor

Jannis commented Jul 2, 2019

I'm thinking of providing what I described above by adding a special

let result = contract.try_someFunction(...)

variant for all callable functions, so developers can choose between handling errors themselves or not.

The result API could be something like:

// Unpack `result`, throw if it is an error
let value = result.value

// Check whether `result` holds a value
if (result.isOk()) {
  let value = result.value
  ...
}

// Log `result` error
if (result.isError()) {
  log.warning('Failed to call someFunction: {}', [result.error.toString()])
}

@leoyvens
Copy link

My concern about this is making the behaviour reliable across different ethereum nodes that may be used by indexing nodes, since the response format seems to vary.

@Jannis Jannis assigned leoyvens and unassigned Jannis Jul 28, 2019
@leoyvens leoyvens added bug Something isn't working and removed bug Something isn't working labels Aug 21, 2019
@Jannis
Copy link
Contributor

Jannis commented Sep 3, 2019

@Jannis Jannis closed this as completed Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
adoption-blocker bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants