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

internal/ethapi: allow eth_call with custom code #19836

Closed
fjl opened this issue Jul 15, 2019 · 18 comments
Closed

internal/ethapi: allow eth_call with custom code #19836

fjl opened this issue Jul 15, 2019 · 18 comments
Assignees
Milestone

Comments

@fjl
Copy link
Contributor

fjl commented Jul 15, 2019

@chriseth wants this: It would be nice to have a version of eth_call where the caller supplies the EVM code to be executed.

  • Contracts could be lighter on-chain because some accessor functions are needed only for eth_call purposes.
  • It allows debugging contracts in ways that aren't possible otherwise.
@chriseth
Copy link

An extended version of this would provide an address->code map to swap out the code of multiple contracts which can be useful for debugging proxy contracts, but only swapping out the code of the called contract would already be of much help!

@karalabe
Copy link
Member

@chriseth @fjl Any ideas on what API would look better? I can see two options: a) add a new field to the transaction parameter in eth.call, or b) add a new parameter for the entire method.

Given that eth_call already has an optional second parameter, adding a third optional one could get messy (how to specify the override map, but not the block number). However, adding a new field to the transaction seems a bit weird too, since it's not really part of the tx, rather a modification to the environment.

Yet a third way could be to convert the currently optional block number into a possible optional struct that can accept both a block number and an override map. Maybe this last is the best. What's your take?

a) eth.call({to: "0x...", data: "0x..."}, "latest", {"0xaddr1...": "0xcode1", "0xaddr2...": "0xcode2"})
b) eth.call({to: "0x...", data: "0x...", override: {"0xaddr1...": "0xcode1", "0xaddr2...": "0xcode2"}})
c) eth.call({to: "0x...", data: "0x..."}, {block: "latest", override: {"0xaddr1...": "0xcode1", "0xaddr2...": "0xcode2"}})

@chriseth
Copy link

I think I like option c best, you are right about separating the actual tx data and the 'environment'. As far as naming is concerned, override may be a bit too generic, though.

@karalabe
Copy link
Member

Yeah, it was just a quick first guess, open to suggestions :)

@rjl493456442
Copy link
Member

Since the format of the API parameter is changed. An EIP is necessary for this feature :)

@karalabe
Copy link
Member

karalabe commented Aug 5, 2019

Not really. For example debug.traceTransaction can accept 2 types of options for the second parameter, so we can definitely make new functionality as long as we support the old format too.

@karalabe
Copy link
Member

karalabe commented Aug 8, 2019

Hey @chriseth, we've merged onto master an implementation for this. Could you take a peek? In the end we went with a 4th API, because the block number was mandatory according to the spec. The format is:

eth.call({to: "0x...", data: "0x..."}, "latest", {0xaddr: {nonce: 1, balance: "0x1", code: "0x...", state: {0xkey: "0xval"}, stateDiff: {0xkey: "0xval"}}}})

Essentially, you have an optional 3rd parameter being a map of overrides. Each element is an address -> override mapping, where each override is a {nonce, balance, code, state, stateDiff} map. All the fields are optional (so you can override only the code for example).

The difference between state and stateDiff is that if state is specified, the entire storage of an account is overridden, vs. stateDiff will only override individual slots that you specify. They are both optional, but mutually exclusive.

We've tried to make it as flexible as possible to cover present and future use cases. Could you please check if this meets your requirements?

@chriseth
Copy link

chriseth commented Aug 8, 2019

Oh that's wonderful, thanks a lot! I think it's very useful like that! Now we only need to publicize that so that people can use it! Do you know if something like that will automatically be available on web services once they update?

@karalabe
Copy link
Member

karalabe commented Aug 8, 2019

I assume yes. I don't think Infura or Cloudflare are doing any filtering or caching or processing on eth_calls.

@karalabe
Copy link
Member

karalabe commented Aug 8, 2019

But I would appreciate if you could take it for a run before we announce it to everyone :) Just to make sure it actually works the way it's supposed to.

@chriseth
Copy link

Somehow I cannot get it to work properly. I used

> curl localhost:8545 -d '{
    "jsonrpc": "2.0",
    "id": 13,
    "method": "eth_call",
    "params": [
        {
            "to": "0x0000000000000000000000000000000000000001"
        },
        "latest",
        {"0x0000000000000000000000000000000000000001": {"code": "0x6080604052600760005260206000f3fe"}}
    ]
}' -H 'content-type: application/json'

which results in:

{"jsonrpc":"2.0","id":13,"result":"0x"}

The assembly of the code is:

mstore(0x40, 0x80)
mstore(0, 7)
return(0, 32)

The code should return 7, but it returns the empty string.

Note that web3.js will automatically prune the new positional argument, so be aware if you test it through that.

The geth version is the following:

> ./geth version
INFO [08-12|12:53:50.155] Bumping default cache on mainnet         provided=1024 updated=4096
Geth
Version: 1.9.2-unstable
Git Commit: 36994e4e0b2be1362f99048d08b907628088e2bb
Git Commit Date: 20190812
Architecture: amd64
Protocol Versions: [63]
Network Id: 1
Go Version: go1.12.7
Operating System: linux
GOPATH=
GOROOT=/home/travis/.gimme/versions/go1.12.7.linux.amd64

@rjl493456442
Copy link
Member

rjl493456442 commented Aug 12, 2019

@chriseth 0x0000000000000000000000000000000000000001 is the address of precompiled account. Actually, it's a design question. In the normal contract execution, all contracts will be overridden if it's a precompiled contract defined in the codebase. Now the extended eth_call follows the same rule. Although we override the specified contract with customized code, later it is overridden again by the precompiled rules. Does it make sense?

$ curl localhost:8545 -d '{
    "jsonrpc": "2.0",
    "id": 13,
    "method": "eth_call",
    "params": [
        {
            "to": "0x0000000000000000000000000000000000000111"
        },
        "latest",
        {"0x0000000000000000000000000000000000000111": {"code": "0x6080604052600760005260206000f3fe"}}
    ]
}' -H 'content-type: application/json'
{"jsonrpc":"2.0","id":13,"result":"0x0000000000000000000000000000000000000000000000000000000000000007"}

@fjl
Copy link
Contributor Author

fjl commented Aug 12, 2019

@chriseth and I tried out this feature together and it was really hard to debug why it didn't work.
We used web3.js initially and it turns out the third parameter in eth.call(..., ..., ...) was removed silently. We tried a few other things but couldn't figure it out. Call tracing doesn't support the overrides so it was not an option.

I think it would be better to add the new parameter into the call object (first parameter), simply because it will be easier to integrate with libraries and tracing. In go-ethereum, that would mean modifying ethereum.CallMsg.

{
    "jsonrpc": "2.0",
    "id": 13,
    "method": "eth_call",
    "params": [
        {
            "to": "0x0000000000000000000000000000000000000001",
            "state": {"0x0000000000000000000000000000000000000001": {"code": "0x6080604052600760005260206000f3fe"}}
        },
        "latest"
    ]
}

@karalabe karalabe modified the milestones: 1.9.2, 1.9.3 Aug 13, 2019
@karalabe
Copy link
Member

This was shipped in 1.9.2. We can discuss, propose and implement changes to the APIs, but in the mean time I'll close this issue to get rid of it.

@chriseth
Copy link

@karalabe can you get a pointer to the tests for this feature? The 0x000...001 was just a simplification I added later on, it also did not work with the actual contract's address, nor with 0x00 (which should not be a precompiled contract, but might have other peculiarities).

@rmeissner
Copy link

Hey, does geth require any special flag to support this?

@rmeissner
Copy link

Ahh never mind it is just that the example here has the wrong syntax, state should be the third element in the call params (the docs specify it correctly: https://geth.ethereum.org/docs/rpc/ns-eth):

{
        "jsonrpc": "2.0",
        "method": "eth_call",
        "params": [
            {
                "to": "0x16baf0de678e52367adc69fd067e5edd1d33e3bf"
            },
            "latest",
            {
                "0x16baf0de678e52367adc69fd067e5edd1d33e3bf": {
                    "code": "0x6080604052348015600f57600080fd5b506040517f08c379a000000000000000000000000000000000000000000000000000000000815260040180806020018281038252600d8152602001807f4e6f7420737570706f727465640000000000000000000000000000000000000081525060200191505060405180910390fdfea2646970667358221220822d1fa73c1b29b6891e98d83f3aa09b8ad6ccd4c0025cfda39942b649276a1e64736f6c63430006040033"
                }
            }
        ],
        "id": 1
    }

@ChristianCoenen
Copy link

Hey 👋 Am I correct that this functionality is not available via GraphQL? If it is, could someone provide an example here?

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

No branches or pull requests

7 participants