Skip to content
This repository has been archived by the owner on Jan 21, 2022. It is now read-only.

Support 'explorer_getTransactions' RPC call [WIP] #45

Closed
wants to merge 1 commit into from

Conversation

axic
Copy link
Contributor

@axic axic commented Mar 24, 2016

WIP - do not merge yet

This is based on MetaMask/web3-provider-engine#49 just to get the ball rolling. Any input is welcome either here or over at the provider-engine.

@tcoulter
Copy link
Contributor

I'm worried about adding application specific functions that don't directly relate to the TestRPC. i.e., evm_snapshot and evm_revert exist to allow the TestRPC to execute unit tests faster by checkpointing and reverting state. However, we're now adding a function whose sole purpose is to make writing your application easier even though no real Ethereum client is likely to support it. I don't think this is within the scope of the TestRPC.

I think this feature is great for the provider engine as the provider engine is meant to provide features on top of what's available in your Ethereum client. But the TestRPC has a much narrower scope.

I'll add this comment to the provider engine as well.

@axic
Copy link
Contributor Author

axic commented Mar 25, 2016

I understand your concern. As you have noted over at provider-engine, it feels like a very useful feature.

In provider-engine we can have connectors to the blockchain explorers to return data. For TestRPC we do not have a block explorer and thus I think it would make real sense having this.

In this sense I don't think it falls in to the "make writing application easier" category, rather makes TestRPC a full replacement of a test network with blockexplorer. At least for Dapp/Wallet writing purposes.

@tcoulter
Copy link
Contributor

Block explorers are just dapps that run on top of an Ethereum client, correct? In your dapp, what's stopping you from getting all the blocks and aggregating the transactions yourself?

@axic
Copy link
Contributor Author

axic commented Mar 25, 2016

Time. I do not have time to write a block explorer. Nor I think it makes sense complicating matters that much.

@tcoulter
Copy link
Contributor

Tim. I do not have time to write a block explorer. Nor I think it makes sense complicating matters that much.

What problem are you trying to solve? Why do you need a block explorer?

makes TestRPC a full replacement of a test network with blockexplorer

The job of the TestRPC not to be a block explorer, and it's not meant to be any other dapp. At the very most it's meant to be an Ethereum client that processes blocks quickly. We should be very conservative in adding non-standard methods.

I don't want people writing code for the TestRPC that'll only work for the TestRPC. Although you understand when to use this method, other people will not. The TestRPC is supposed to be a faster, drop-in replacement for geth, and you should be able to switch them back and forth and run the same code. Coding for the explorer_ method means you won't be able to do that. The only exception that exists so far are the two methods I mentioned previously, which are meant specifically to be used in testing frameworks (hence where the TestRPC got its name).

I can think of many ways to get around this limitation and get the feature you desire:

  1. Write a block explorer. You said you don't have the time, but the least-effort block explorer is already provided by eth_getBlockByNumber. If you want to aggregate the transactions on a specific address, you stick this method in a for loop (realistically an async.x method).

  2. Use the provider engine. If the provider engine is going to provide this feature, write some code that uses the provider engine:

    var Web3 = require("web3");
    var ProviderEngine = require("web3-provider-engine");
    var Web3Subprovider = require("web3-provider-engine/subproviders/web3")
    
    // Location your TestRPC is running on
    var provider = new Web3.providers.HttpProvider("http://localhost:8545");
    
    var engine = new ProviderEngine();
    
    // You'll need to add the explorer_... subprovider, whatever that ends up being
    engine.addProvider(...)
    // Then add the web3 subprovider
    engine.addProvider(new Web3Subprovider(provider));
    engine.start();
    
    // Then you can make your call
    engine.sendAsync({
      jsonrpc: "2.0"
      method: "explorer_..."
      params: []
    }, function(err, result) {
      // ...
    });

    This code can work on your frontend or backend.

  3. Use something like this: https://github.com/etherparty/explorer . I've verified that it works with the TestRPC, but at the moment it only supports blocks - not transactions or addresses. I even added net_listening support for you in the last few minutes so you can check it out. Unfortunatly this block explorer isn't the greatest, but this is where block explorer development should happen, not within the TestRPC. Perhaps we can work together to make this better.

@axic
Copy link
Contributor Author

axic commented Mar 25, 2016

As you mentioned a naive implementation is to run eth_getBlockByNumber from 0 to eth_blockNumber.

I am afraid adding that as a subprovider (i.e. called blockexplorer) would be very risky, because it could encourage users to use it against live RPC nodes.

Running it against TestRPC is fine, but I don't think users of provider-engine should encouraged to use that code against anything else.

In the short term there would be two subproviders providing explorer_getTransactions:

  • etherscan (I'm using this one internally already)
  • etherchain

Not sure how the ecosystem evolves, but of course there could be changes to it.

@axic axic closed this Mar 25, 2016
@axic axic reopened this Mar 25, 2016
@tcoulter
Copy link
Contributor

I think the better approach is to build off of what Etherparty started. You could easily use their code that saves transactions in a database until it gets caught up with the network as some sort of initialization process. If done this way, this block explorer would work on any Ethereum client, including the TestRPC.

Regarding the Etherscan and Etherchain subproviders, these are third party services that aggregate this data, and you want the TestRPC to do the same. I think this is out of scope of the TestRPC. However, it'd be fairly easy to create something built on top of the TestRPC -- and hence, any Ethereum client -- that does the aggregation and then makes it available over an API, for which you could then create a subprovider. Still, this seems like it's an application-specific feature meant to help you list previous transactions for an address, like you'd want to do in Metamask. Until it's available in canon Ethereum clients I'm unlikely to add it here.

That said, the biggest question I had for you was "What problem are you trying to solve?". I'm assuming you want to list previous transactions, like in the case of Metamask. For the TestRPC, I think you're better off creating a subprovider that only looks at transactions from initialization of your dapp onward -- i.e., a subprovider that just watches all new transactions and aggregates those. That can probably get you the functionality you need while also giving you the confirmation that your code is working in a test environment. The fact that all transactions aren't listed since the beginning of time (i.e., TestRPC's short lifespan) is probably less important than being able to list transactions there in general.

@danfinlay
Copy link
Contributor

I think the basic problem here is really important, and as DAPP developers we are going to see this discussion more and more, and we should consider it carefully.

The problem is how do you build an application in a test environment that depends on a whole ecosystem of existing services?

Some of those services are singleton dapps, and having references to them in a test-space may actually require compiling a local test-version of those dapps on your test-blockchain, and that problem is being addressed by ethereum package-managers like Dapple.

Some of those services are on other protocols entirely, like a blockchain explorer.

And what do you do then, when your blockchain, an isolated island simulating the larger world (which will get larger and larger as the blockchain ecosystem grows) is more complicated than your test environment? The old way would almost certainly be to stub out those methods in testing, but for development? Can we do better? Can we take advantage of this explorer prefix to encourage a user-extensible ecosystem?

We could add this one method to testrpc, but then I do think we would set a dangerous precedent.

  1. Users who rely on that endpoint would require specialized RPCs in production, or a way to manage different providers by environment.
  2. Truffle has to keep adding 3rd-party endpoints to keep up with the main-net's environment.

It would be cool if an Ethereum framework like testrpc could emulate the user-space environment on the main-net, but as the ecosystem grows, it's going to be harder and harder to keep up.

It would be really nice if we could trust 3rd-party blockchain services to provide their own testing tools, but that would require them open-sourcing their money-maker, so it splits their incentives between user support and giving away their product.

The third option is basically just making sure it's easy to add all sorts of ecosystem middleware at every layer of the testing stack, and making sure we can add it wherever we need to.

@axic has a good point that a simple provider-engine blockchain explorer on the client side will only work on that blockchain (not a test-net), so there's probably a good place for adding extensibility to the backend per-environment.

I notice that testrpc is also built on provider-engine, so maybe this is a good place to run testrpc programmatically, adding testing provider middleware to that rpc script. @tcoulter that sounds easy, right? You can add providers to the programmatic server?

So I guess my suggested solution for the short-term is

  1. Have environment-specific middleware on the front-end
  2. Add stub middleware to testrpc by running it programmatically in dev/test environments.

Keep sharing what your needs are, I really do think these are harbingers of larger discussions to come, and these are problems all Dapp developers are going to run into if we don't solve them well.

@tcoulter
Copy link
Contributor

Couple responses.

First, @axic, I want to apologize if there was any negativity in my responses (hopefully there wasn't). I interpreted a comment from you as negative when it likely wasn't. Thank you text. :)

Second, etherchain and etherscan are centralized services. This explorer_ method effectively creates an RPC call into their centralized systems, which begets more centralized systems (in our case, in the TestRPC). I heartily agree that transaction aggregation is a huge blind spot in the Ethereum JSON RPC, but perhaps we should think about trying to solve this problem via web 3.0 - i.e., aggregating transactions on the blockchain itself, something akin to a transaction oracle. I haven't thought through it fully yet, but there's no reason why we should treat these centralized services any differently than we do other centralized services.

Last, I agree that adding these methods to the TestRPC is a slippery slope. The best thing to do is to build on top of the TestRPC mimicking the service you are trying to use. And we should make doing so as easy as possible.

@danfinlay
Copy link
Contributor

Can you confirm it would be easy to add providers when running webrtc programmatically, @tcoulter?

@tcoulter
Copy link
Contributor

@FlySwatter Could you elaborate? When you say webrtc, I'm assuming you mean websocket, is that correct? As far as I understand it, the JSON RPC will still exist whether send the data over http or websocket - so most everything would be the same except for the transport layer. If my understanding is correct no external APIs would change.

That said, websocket support for the TestRPC will definitely be added once that API is solidified.

@danfinlay
Copy link
Contributor

Whoops I meant testrpc not webrtc yikes!

@tcoulter
Copy link
Contributor

Ha, alright. Can you still elaborate then? I'm not sure I understand. :)

If I actually do understand, I don't think anyone should be messing with the TestRPC's internal provider stack. Instead, you should use the TestRPC as a subprovider within a stack of your own. i.e., like the code I added above.

Edit: You can also use TestRPC.provider() vs. TestRPC.server()

@danfinlay
Copy link
Contributor

I mostly meant so the middleware could be provided beneath the HTTP layer, not on the client side. Anyways I'm going back to this actual work now.

@tcoulter
Copy link
Contributor

This conversation has been stagnant, and I'm trying to clean up a bit. Is there still string feelings about this feature?

@axic
Copy link
Contributor Author

axic commented Jul 22, 2016

@tcoulter I still think this has merits and I do understand your reasoning.

Some progress might be made regarding the RPC API given Parity has introduced other debugging methods.

@benjamincburns
Copy link
Contributor

benjamincburns commented Oct 18, 2017

@axic & @danfinlay - I'm new (just starting today), but I'll be managing TestRPC for the Truffle team from here on out.

After discussing this with @tcoulter, we both agree that this feature is out of scope for TestRPC. The test for whether we'd implement nonstandard RPC calls would be whether it improves the utility of TestRPC as a testing facility (e.g. like the calls in the evm_ namespace). Given that this feature can already be implemented today by enumerating transactions block-by-block, we feel that this doesn't pass that test.

If you feel that this is a useful feature, I think TestRPC isn't the best place to lobby for it. Rather, it should likely be submitted over at ethereum/EIPs.

In the mean time I'll close this.

@benjamincburns
Copy link
Contributor

benjamincburns commented Oct 18, 2017

Also it's worth pointing out that we'll have this use case in trufflesuite/ganache as well. As such, we'll likely wind up lobbying for this RPC addition ourselves.

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.

4 participants