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

ethereumProvider API #16

Open
frozeman opened this issue Mar 30, 2017 · 80 comments
Open

ethereumProvider API #16

frozeman opened this issue Mar 30, 2017 · 80 comments
Labels

Comments

@frozeman
Copy link

frozeman commented Mar 30, 2017

Abstract

This draft is to discuss the ethereum API, an object exposed by ethereum native browsers and tools like Mist, MetaMask and status.im.

The API should be small and clean and allow all kind of libraries to attach to it.

The class of the object should be EthereumProvider, the object name ethereum.

API

The ethereum should inherit from a EventEmitter type class, and provide add/removeListener, on/once/off etc.

// callback: a error first callback
ethereum.send(methodName, params, callback)
> undefined

// CALLBACK return
> err, result
// returns subscription notification,
ethereum.on(subscriptionType, handler)

subscriptionType = `eth_subscription`

// HANDLER return
> {
       "subscription":"0x2acffa11d68280c2954daeb77cb849d9",
       "result": {...}
    }
// react on connection start
ethereum.on('connect', handler)

// HANDLER return
null
// react on connection end
ethereum.on('end', handler)

// HANDLER return
Error Object with message (timeout, error, drop, etc)
@frozeman
Copy link
Author

@kumavis @jarradh

@kumavis
Copy link
Member

kumavis commented Mar 30, 2017

I suggest that connect() and the relevant events are removed from the standard.
If a special subclass of ethereumProvider needs it, it can implement it.

Why did you include those?

Any thoughts on returning a Promise in ethereumProvider.send(payload, callback)?

Looks good!

@kumavis
Copy link
Member

kumavis commented Mar 30, 2017

small nitpick ethereumProvider.on('timeout', callback) the callback reference here should be called handler, as callback implies that it should only be called once

@frozeman
Copy link
Author

frozeman commented Mar 31, 2017

changed to handler.

Hm yeah, if the provider always takes care of connecting and re-connecting automatically we wouldn't need connect()

Concerning promise. i would like to keep this lib small and a promise is an extra dependency.
Apart from that i'd follow node.js defaults here, which is error first callback :)
But im open for debate..

@kumavis
Copy link
Member

kumavis commented Mar 31, 2017

here's a version with the connection semantics in a separate subclass

EthereumProvider : EventEmitter

// payload: is a valid jsonrpc 2.0 object
// callback: a error first callback
ethereumProvider.send(payload, callback)
> undefined
ethereumProvider.on('data', handler)
> undefined

// The subscription received in the handler:
{
  "jsonrpc":"2.0",
   "method":"eth_subscription",
   "params": {
       "subscription":"0x2acffa11d68280c2954daeb77cb849d9",
       "result": {...}
    }
}

EthereumWebsocketProvider : EthereumProvider

// Will try to connect, the ethereumProvider should be already connected when the webpage is loaded and firse the "connect" event at start.
ethereumProvider.connect()
> undefined
// react on connection start
ethereumProvider.on('connect', handler)
// react on connection end
ethereumProvider.on('end', handler)

@tcoulter
Copy link

My biggest ask is that dapp developers not have to code for specific provider types. For instance, a dapp developer should not have to maintain a specific code block for Metamask-style providers where no connection information needs to be managed, and another code block for Mist that, say hypothetically issues a provider where connection information does need to be managed. (If unclear, I interpret @kumavis's suggestion above as requiring dapp developers to do exactly that).

Creating a library that supports that ask would require either we:

  1. Expose a connection abstraction that every provider must implement, for all provider types; or
  2. Expose no connection information at all.

My vote is on a modified hybrid approach, as described below, as I think the current connection abstraction is too specific to a certain connection type, and exposes too many temporary error states. Instead, I think we should focus on whether a provider has been "started" and "stopped" -- i.e., is it ready to provide you the information you need, or not. Here's what I propose:

// Implemented by all providers; is a no-op for providers that don't need to 
// connect to anything or don't require any type of polling. If there's base
// code we decide on, it could implement this no-op by default.
//
// The callback is fired when the provider is successfully started.
// This is important for providers that work over an established connection
// or that are in-process and connect to a database backend.
ethereumProvider.start(callback)

Similarly, we could implement a stop method that can also be a no-op if not necessary.

// The callback is fired when the connection is successfully stopped.
// Important for established connections as well as database backends.
ethereumProvider.stop(callback)

Sending data is the same as the examples above, but the handler has changed (more on that below):

// payload: is a valid jsonrpc 2.0 object
// callback: see below
ethereumProvider.send(payload, callback)

Also, on('data') can be more generic. Instead of being limited to subscriptions, which assume a specific transport, on('data') can provide a handler for all data coming through the pipe. See the note on handlers below.

// returns payload and response, see handler details below
ethereumProvider.on('data', handler)

As well, theon('error') event has been modified:

// Will return an error as a result of any payload; this includes connection
// errors as well as rpc errors. See below for handler details.
ethereumProvider.on('error', handler)

All callbacks and handlers on send, on('error'), on('data') are of this form:

function(err, payload, response) {
  // connection error
  // payload (request)
  // response: response body returned by the RPC server
}

Note that err above would only include connection errors as, if RPC clients are following the specification, RPC errors are part of the response body.

An http provider pointed to an invalid non-responsive or incorrect url, for example, would expose connection errors in the err object when a request 400's or 500's. Websocket errors would return a connection err (err) when the connection hasn't reconnected after a specific period of time. The TestRPC provider would return an err when there's a runtime exception that should never have occurred (i.e., the TestRPC provider would run in the same process, so wouldn't need to make use of this error path as a connection error).

I see all of this discussion as akin to leveldown. An output of this discussion should not only be an abstraction that works for any connection type (or none at all) with the littlest effort on its users; but we should also produce an abstract-leveldown-like codebase (we'd call it abstract-ethereum-provider) that lets any data layer fit in and the connection type inconsequential.

@kumavis
Copy link
Member

kumavis commented Apr 3, 2017

I interpret @kumavis's suggestion above as requiring dapp developers to do exactly that

@tcoulter you seemed to have misunderstood my proposal. I specified a standard EthereumProvider and an example subclass of that standard EthereumWebsocketProvider. The standard is what dapp developers would expect to be injected by the browser. The subclass would be for the case where the there was no standard api object injected and the dapp dev wanted to set up their own fallback that would necessarily require connect configuration be handled by the dapp dev.

@kumavis
Copy link
Member

kumavis commented Apr 3, 2017

Also, on('data') can be more generic.

Instead of being limited to subscriptions, which assume a specific transport,

its not a specific transport, its a feature of the ethereum json rpc built on top of json rpc notifications.

on('data') can provide a handler for all data coming through the pipe. See the note on handlers below.

its nice having just a handler for server sent notifications, but interesting idea

@kumavis
Copy link
Member

kumavis commented Apr 3, 2017

Creating a library that supports that ask would require either we:

for the sake of simplicity I advocate for 2. Expose no connection information at all. in the base standard

@tcoulter
Copy link

tcoulter commented Apr 4, 2017

for the sake of simplicity I advocate for 2. Expose no connection information at all. in the base standard

I went down that path too when I originally started writing my original comment. If you as a dapp developer want anything extra it means you have to explicitly code for a specific environment (i.e., the websocket provider), which is why I later chose the hybrid/more general abstraction approach. However, if the custom provider classes are meant to only be used as a fallback, then I could get into that.

its not a specific transport, its a feature of the ethereum json rpc built on top of json rpc notifications.

👍 Good to know, thanks. I find the general on("data") to more useful (at least theoretically), but could go either or as the behavior I'm looking for could easily be shimmed.

@frozeman
Copy link
Author

Ok sorry for the late response, but i was on a holiday. @tcoulter @kumavis

So concerning exposing no connection data:
IMO thats wrong. I do agree we can consolidate "timeout" and "error" to simply "end". But a library like web3.js can and must be able to know about connection drops, which can happen in mist, as well as any internet connected device. Web3.js and other libraries in this case must know about it and provide subscription regains strategies and lost connection behaviour.

Or the dapp developer itself must be able to act on connection drops as well.

Its not feasable to try to mitigate all of that under the hood in the EthereumProvider itself.

Concerning the generic on("data"):

This makes building libs like web3.js a lot more complicated, as each of those libraries needs to handle the mapping between request and responses and do the mapping of JSON rpc IDs themselves. This is easily taken away by ethereumProvider.send(payload, callback), and i think thats a good thing and makes it more fun to build web3.js like libraries.

Concerning the ethereumProvider.connect()

This is necessary to force regaining connection tries, but we can get rid of it, if we make sure to call the "connect" event on reconnect, and try to reconnect under the hood automatically.

Concerning Separate WS provider

The ethereumProvider above is on purpose generic for all type of providers, be it HTTP, IPC or WS under the hood. Its kind of its own type and can be used in web3 like libs, as they would a WS provider.

If ethereumProvider is not defined the dapp has to load its own WS or whatever provider to get a connection. The browser can't take care of this.

So given the above feedback i would suggest the following API:

// payload: is a valid jsonrpc 2.0 object
// callback: a error first callback
ethereumProvider.send(payload, callback)
> undefined
// returns subscription notification,
ethereumProvider.on('data', handler)

// The subscription received in the callback:
> {
  "jsonrpc":"2.0",
   "method":"eth_subscription",
   "params": {
       "subscription":"0x2acffa11d68280c2954daeb77cb849d9",
       "result": {...}
    }
}
// react on connection start
ethereumProvider.on('connect', handler)
// react on connection end
ethereumProvider.on('end', handler)

@danfinlay
Copy link

What happened to your PromEvent pattern? I was fond of that. I'm not as big a fan of a global data event.

I'd much rather that each individual query is subscribable, allowing reactive state updates.

Example:

token.getBalance(myAccount)
.on('update', updateUI)

Also, I think a single PR is too simplistic a form to discuss an entire spec. I would propose the spec proposal be a repository, and then individual features & changes could be pull requests, discussed individually.

@frozeman
Copy link
Author

frozeman commented Apr 24, 2017

This here is about a basic provider provided by mist, metamask and others, not how web3.js or any other library on top will work ;)

Off Topic:
Tho as the subscriptions we currently have are rather limited in the RPC, things like you propose there, are nice but won't be available in web3.js 1.0.
The PromiEvent is only working on the send function right now: http://web3js.readthedocs.io/en/1.0/web3-eth.html#id62
And for contract methods:
http://web3js.readthedocs.io/en/1.0/web3-eth-contract.html#id21

@frozeman
Copy link
Author

frozeman commented May 2, 2017

To get back to this and finalise it soon. @kumavis @tcoulter

Here is an idea from @MaiaVictor which is what you @kumavis probably mentioned in one of these calls, to have a simple function object which can handle the calls, without exposing formatting the RPC calls yourself.

I could imagine something like this, but with the subscription addition:
https://github.com/MaiaVictor/ethereum-rpc

@frozeman frozeman added ERC and removed enhancement labels May 2, 2017
@frozeman
Copy link
Author

frozeman commented May 3, 2017

@kumavis @tcoulter i talked to @MaiaVictor and took in his suggestions to strip away the RPC itself, so that we can in the future change the transport layer without the need for changes in this API here.

The provider would take care of generating valid JSON and IDs and you only pass in RPC method names and get results back.

The end and connect is still there as its necessary for dapp developers and high level libraries like web3.js to detect connection drops and handle them well, or for apps to show connectivity statuses to their users, and/or warnings.

So i would propose the following:

// payload: is a valid jsonrpc 2.0 object
// callback: a error first callback
ethereumProvider.send(methodName, params, callback)
> undefined

// CALLBACK return
> err, result
// returns subscription notification,
ethereumProvider.on(subscriptionType, handler)

subscriptionType = 'eth_subscription' or 'shh_subscription'

// HANDLER return
> {
       "subscription":"0x2acffa11d68280c2954daeb77cb849d9",
       "result": {...}
    }
// react on connection start
ethereumProvider.on('connect', handler)

// HANDLER return
null
// react on connection end
ethereumProvider.on('end', handler)

// HANDLER return
Error Object with message (timeout, error, drop, etc)

@VictorTaelin
Copy link

VictorTaelin commented May 3, 2017

I agree with that spec. I understood on("data") wrong previously and it is actually fine like that. I'd personally have a simpler API (i.e., eth.method(a,b,c) or eth("method", [a,b,c]), and leave the on("connect") and on("end") events out as I think it would look prettier to just have it on the browser. In practice, that'll just cause each DApp to implement its own "connection lost" message on top. I don't see much critical use to it. But I can live with both of those things. This proposal is simple enough to feel safe depending on it.

Just a small refinement, what you think of:

ethereumProvider.on("eth_subscription", function (subscription, result) { ... })

Or something like that? So it has a direct pairing with eth.send("method",...).

~~

Looking forward to see what the other guys think, so we can just go ahead and work on top of this standard.

@frozeman
Copy link
Author

frozeman commented May 3, 2017

I like the idea of ethereumProvider.on("eth_subscription", ..., as we also have ethereumProvider.on("shh_subscription", ... and so on. Will add this.

@tcoulter
Copy link

tcoulter commented May 4, 2017

As I'm thinking about this, all the proposals above (even mine) seem to extremely complicate the task of making an ethereumProvider. I'm starting to think we should have ethereumProvider.send(...) be the only requirement for the ethereumProvider spec. Given the most recent spec above, we're currently asking all ethereumProvider writers to support the following:

  • A method calling function:send()
  • An event system for subscriptions (that handles a subset of all available provider methods): eth_subscription and shh_subscription
  • An event system for transport level state changes that don't apply to all possible transport types (i.e., transports like RPC, REST, in-memory providers like the TestRPC, etc. could care less about these state changes; it really only applies to websockets)

If I wanted to create a simple provider for testing, i.e., as a mock provider within my application tests, there's a lot I'd have to create just for it to be compliant. I think this is too much of a burden.

Instead, we should rally around one method, and have it be the only requirement to write an Ethereum provider: ethereumProvider.send(method, params, callback).

I agree with your concerns that transport level information is important. To get around this, I suggest we do two things:

  1. Add more provider methods like transport_connected, that can be called from within a dapp, which can provide the dapp with information about whether or not the transport is connected. This will have to be polled on a regular basis via a timeout, but it won't require a network request for all transport types; i.e., this will be answered by the transport itself in the case of websockets, or in the case of a mock provider it can simply return true. Some providers, like an http provider, may also choose to simply return true, as the send method will fail if a specific request fails.

  2. For events, and anything you'd like to be subscription-based, this can easily be added via a wrapper around the ethereumProvider. e.g., it's not hard to write an object that inherits from EventEmitter and wraps a provider's send method, emitting an event after each callback is triggered for the methods that you're interested in (i.e., eth_subscription and shh_subscription). In fact, using this model, I can easily write a provider that triggers an event for every method, which suddenly makes my request-and-response-type dapp into event-based dapp:

var inherits = require("util").inherits;
var EventEmitter = require('events');

inherits(EventedProvider, EventEmitter);

function EventedProvider(provider) {
  this.provider = provider;
}

EventedProvider.prototype.send = function(method, params, callback) {
  var self = this;
  this.provider.send(method, params, function(err, result) {
    callback(err, result);
    // Send all the information just in case a handler could use it.
    self.emit(method, params, err, result); 
  });
}

Of course, the above code is for Node, but it's very easily bundled for the browser.

The above code would provide subscription events for eth_subscription and shh_subscription, as well as events for every other provider method ever called which is super handy. And people could use the EventedProvider if they'd like their dapp to have events (perhaps using some evented wrapper, of course) -- and they won't need to implement an event system for providers that don't need events, like the ones I mentioned above. This could easily be extended to poll transport_connected under the hood to provide the connect and end events, and the poll interval desired. Now, I know "polling sucks", but remember, most of the time this will be answered by the transport itself, which means it will all happen within the current execution environment (i.e., no network requests, etc.). Rarely if ever should transport_connected send a request down to the Ethereum client.

As an added bonus of this much simpler specification, it means providers can be composed. For instance, I can make a caching provider that takes a different provider as input, and suddenly I have a caching layer with no extra work. Adding an event system and a mechanism for transport state change handling makes this much harder, as if you were to compose them you'd have to propagate those events all the way up.

To sum up, the features desired by the above spec are good, but if we simplify the spec further we can reduce overhead and get a lot more benefit. We can still have all our cake and eat it too with regards to transport level changes, though, by adding the transport_connected provider method, and any other transport-related methods required.

@VictorTaelin
Copy link

I agree with all you said. I mean, if you read my proposal, it is exactly what you proposed, ethereumProvider.send(method, params, callback) (except I also removed the .send). In other words, ethereumProvider becomes basically a global function that calls the RPC.

Fabian, though, argued we need on("data") because of streams. I completely agreed with his point: if ethereumNode only offered pooling-like operations, then we couldn't get stream-like performance from it. Now, reading what you said and under further thoughts, I noticed this is false. We can still have the exact same performance characteristcs when PUB/SUB streams are available, even if we don't have on("data"). Or, in other words, given an ethereumProvider with only direct RPC methods, we can reconstruct one with on("data") with the same performances characteristics. As such, there is, indeed, no reason to have events on ethereumProvider at all.

@VictorTaelin
Copy link

So, in short, I'm 99% in favor of your proposal (1% is that additional .send removal). I'll implement it on Ethereum-RPC, and I'll also implement a wrapper with that builds the longer spec (i.e., on("eth_subscription",...), on("bzz_subscription",...), on("connect",...) and on("end",...)) in terms of the simpler one, to test the hypothesis that it is possible without performance losses.

I'll be waiting to see what everyone else thinks.

@kumavis
Copy link
Member

kumavis commented May 7, 2017

here is what i think most stacks will look like

  • high level userspace library ( e.g. framework integrations, fancy subscription mgmt )
  • eth api ( rpc methods exposed as functions, e.g. eth.getBalance(address) )
  • json rpc request handler ( request and response are paired by .send, notifications are emitted as event )
  • connection duplex stream ( requests go in, responses and notifications come out )
  • transport ( websockets, http, ipc, carrier pigeon )

web3.js has been more of the (high level userspace library) variety, with a lot of API surface and a lot of change. This meant cool features (like web3.Contract) but breaking changes. When standard APIs do this they break the web. So web3.js should continue to grow and experiment, and the standard should be at some lower level (I think we all agree so far).

The goal here is to determine what the API standard for Ethereum Browser Environments should be, and thus the actual transport between the app using this API and the logic that process those requests is out of scope. That narrows our layer options to:

  • eth api
  • request handler
  • connection duplex stream

providing something at the eth api level would be more akin to existing browser APIs, but it requires you to know what methods the rpc endpoint provides. Perhaps some sort of handshake could be done here on connection, like the introduction of a "web3_availableMethods" rpc method.

the current web3 provider behaves like the request handler layer. this layer is fairly generic and provider some utility over the raw connection duplex stream. It groups response with their requests, and provides a way to listen for server-pushed notifications. At this level we're at the level of the official json rpc spec, no ethereum semantics or custom functionality. We could add a little more custom functionality on top that grouped server-sent notifications with their original subscriptions, but i would advocate that we handle that at a higher level

The connection duplex stream level is certainly the purest. Data goes in, data comes out. Sort of an unusual api for a browser standard, but very unixy.

So what do we do?

If our goal is to establish an standard Ethereum Browser API, then the eth api seems to make the most sense. (e.g. something like eth-query ) But the eth rpc method space is still in flux, lots of new things are being introduced and deprecated. Makes it a difficult choice.

The request handler layer (essentially the original proposal of this thread) would be a stable standard but certainly an unorthdox one.

Which of those two im leaning towards changes day to day.
Do you agree with my identification of the layers of abstraction and their pros and cons?

@danfinlay
Copy link

I agree that it's unusual just how pure this new proposed standard is. It's almost an over-reaction to criticisms of the web3 library's size. No other browser standard wraps multiple other methods, from callbacks to events, into a single function.

This method would make the API almost completely useless to people not importing external client libraries, which is a new direction for a web API.

That said, it definitely achieves the goal of "minimal surface area" for user-space libraries, and gives developers a lot of room for future iteration, and keeps page load time minimal.

Maybe it's worth considering: Which minimal API is more useful to developers?

While an availableMethods call could load methods from a remote process, this forces all other feature detection (which is often done at load time) to be done after an async resource load, which could actually result in slower page loading than just injecting functions for each capability in the first place.

So while I can't really complain with the current proposal as a very low-level access method, I think at least slightly longer term I will be in favor of some kind of global object that has methods for each current feature.

@danfinlay
Copy link

I think @tcoulter's concern is also valid, part of this spec is to make it easy for people to develop clients against it.

Since that layer is easier to implement, and the other layer can be built on top of it, maybe we really can just make the pure-function API first, standardize it, and then start experimenting with other APIs to inject as well.

@VictorTaelin
Copy link

Thanks for the analysis, @kumavis. I agree with your categories. In an ideal world, where the RPC API wasn't supposed to ever change, the best choice would be to standardize the eth api layer, as you put it. But, as you said, it isn't as stable and new RPC methods will be added.

This is confusing me, though. We're assuming the request handler layer (what we're proposing here) is somehow more stable and flexible than eth api. Why, though? There are two types of changes. Non-backwards compatible changes (e.g., removing eth_getFilterChanges) would break both DApps using ethereumProvider.send("eth_getFIlterChanges",...) and DApps using ethApi.getFilterChanges(). Backwards compatible changes (e.g., adding eth_something) are possible on both: on ethApi, add a new method and inject that instead; on ethereumProvider, just call the new method. So, on both cases, they're equivalent. One isn't more flexible than the other, the request handler is still exposing the same api, just indirectly. Am I missing something?

To be honest, at this point, I'm deeply confused with all the options and mindsets here. If I was alone in doing that I'd, before anything, focus on designing an RPC API that wouldn't need to change. I'd then just expose the API to the browser (i.e., something like kumavi's eth-query) and tell DApps they can rely on it. And that's it. Seemed so simple to me.

Now I feel like I'm being more destructive than productive on this conversation. I don't want to interfere anymore. I'm happy with most of those solutions. In the end, none of those details is a major issue. But losing time is. Let's not have our own blockchain size debate. I'd say, let's go with Fabian's last proposal. He has good reasons to think it is good, and, perfect or not, it is clearly sufficient. I just hope we can find some consensus soon.

@kumavis
Copy link
Member

kumavis commented May 8, 2017

@MaiaVictor you are entirely correct about breaking changes with method removal ( and method behavior change ). One difference though is that if a new rpc method is introduced, it would be accessible via the json-rpc interface but not the eth API.

Perhaps the happy medium is something very close to what we have now:
eth api (simple web3 / eth-query)
with the json rpc request handler exposed ( web3 provider )

Let's not have our own blockchain size debate.

🤣 yes I agree -- though its damn hard to change things once people start using them so lets try to get it right this time

@frozeman
Copy link
Author

frozeman commented May 8, 2017

Ok now read through all of that.

So first of a few informations, because i think not everybody caught that yet.
There will be the new Pub/Sub API coming -> https://github.com/ethereum/go-ethereum/wiki/RPC-PUB-SUB (and in fact its already one year! implemented, and i guess parity too), which is thought to replace eth_newFilter and eth_getFilterChanges.

The reason why is to standardise notification streams, which in the new API we already have:

  • logs
  • newHeads
  • pendingTransactions

And we could add a lot more.

The old filter was not flexible to add more and unclear to understand, we all just got used to.

The current way means, duplex streams are the new default, e.g. WS and IPC
If we can convince the nodes to add polling support we can add HTTP back that list, but i think this will take a bit of time.

So given that fact we have now push, and this needs to be part of the standard a on('notifications|data|eth_subscription', .. is necessary, to receive PUSH notifications.

I also really like @MaiaVictor idea to strip away the RPC layer, as its not really necessary to know how methods on nodes are called, but just that you can call them.

I also like the last approach i posted, eg. ethereumProvider.send(methodName, params, callback) as if new methods get added we don't need to change the provider, but high level libs like w3.js can start using it.
We don't want to update mist or metamask's provider all the time, because there is a new RPC method.

Therefore i really like the last approach. Its simple clean and allows for flexibility.

Concerning the connect and end events:

Those are necessary, because subscriptions are based on sockets, e.g. if your IPC/WS socket disconnects your node will cancel the subscription. So dapps and high level libs like w3.js need to be informed to be able to regain subscriptions, or fetch missing information.
Currently we only have eth_getLogs as a way to retrieve past logs, but we might add a generic way to retrieve missed logs, when we add polling to subscriptions. All of these improvements would be possible with this new provider. So it is backwards compatible and should be safe for the future.

At the same time it allows dapp developers to even use it directly, which is far more complicated when we would require them to build JSONRPC objects, or even give them a raw duplex and they need to match responses themselves.

With this solution is think we have a great middle ground.

If you guys have better solutions, i would ask you to post example APIs, like i do. so we can agree father, because don't forget "people don't read"! ;) So the easier to digest, the better.
so for readability i post the proposed solution again:

// payload: is a valid jsonrpc 2.0 object
// callback: a error first callback
ethereumProvider.send(methodName, params, callback)
> undefined

// CALLBACK return
> err, result
// returns subscription notification,
ethereumProvider.on(subscriptionType, handler)

subscriptionType = 'eth_subscription' or 'shh_subscription', 'data'? `notification`?

// HANDLER return
> {
       "subscription":"0x2acffa11d68280c2954daeb77cb849d9",
       "result": {...}
    }
// react on connection start
ethereumProvider.on('connect', handler)

// HANDLER return
null
// react on connection end
ethereumProvider.on('end', handler)

// HANDLER return
Error Object with message (timeout, error, drop, etc)

@danfinlay
Copy link

Yeah I'm basically fine with that.

The bonus method that @kumavis mentioned that I'd support is if the RPCs also had an availableMethods endpoint, that could maybe return something like ethjs-schema returns.

But that could be its own proposal. In the meanwhile this is a nice low level provider api.

@danfinlay
Copy link

Sounds good, let's plan a time.

@danfinlay
Copy link

Hey @frozeman, we had a conversation with @wanderer about this proposal, and our long-term goals of enabling connecting to multiple shards/VMs over the main API, and we came up with a single very simple change to this proposal that would enable those goals.

Introducing MetaProvider (name up for discussion)

MetaProvider is one additional object, and this is the object that owns the connect method, and returns a normal provider. Using it would be like this:

// .connect() could default to the main Ethereum network.
var ethereumProvider = metaProvider.connect(`blockchain://${genesisHash}/block/${laterHash}`)
ethereumProvider.on(‘connect’, () => {
  ethereumProvider.send(payload, handleSendResult)
})
ethereumProvider.on('error', (reason) => {
  // reason.message could be "The requested blockchain is not supported by this client.")
})

@frozeman
Copy link
Author

frozeman commented Jun 21, 2017

Sounds interesting, but mist only allows for one blockchain to run in the background, due to the nature of local nodes, and it would be too resource intensive to run multiple testbetworks as well.

How would we keep cross platform compatibility? if some dapps have to check for metaProvider and other for ethereumProvider? don't you think the start logic should be that big (which is that this provider wants to simplify)

I also choose the name "ethereumProvider" to allow for future additions of other providers.
This should be the soley access to the ethereum world of tools (swarm, whisper, ethereum).

Maybe better than would be a:

ethereumProvider.connect(`blockchain://${genesisHash}/block/${laterHash}`, function(err, worked))

While its connected by default to the network of the browser, dapps can request to change it.
If its not changeable then the callback returns simply an error.

the "connect" and "end" events, are mainly for cases when the user switches the network in the browser (outside of the dapp), to inform the dapps that connections changed.

And to check for the network, dapps or libs have to provide a function, not the provider (see: https://github.com/ethereum/web3.js/blob/1.0/packages/web3-eth/src/getNetworkType.js#L27)


Scratch the above ^

EDIT: maybe another important point is maybe we should just create a custom eth_changeBlockchain method, which will not work on nodes, but only in some browsers.
This way we don't need to change the provider, to add this functionality.

The reason is that this provider: This provider is not only to connect to an ethereum node, but gives access to the WHOLE ethereum ecosystem, like swarm, whisper and ethereum (shh_, bzz_, eth_).
While your connect event proposal is ethereum network specific.

Whisper and swarm don't care about the blockchain they are on, they work differently. I don't think the ethereum network should get a special position on the provider.

By simply using a custom RPC method (eth_changeBlockchain), we allow switching, with error or success reporting without introducing any custom connect event.

@kumavis @wanderer @FlySwatter @tcoulter
Lets please find an agreement, so we can implement that. We seriously need that to move on with Mist.

@frozeman
Copy link
Author

frozeman commented Jun 21, 2017

Also very important to me is that dapps wont need to call any connect event. The connection is already available by default when the dapps starts, so that they can immediately call any function like:

ethereumProvider.send('eth_accounts', [], function)

@frozeman
Copy link
Author

I updated the title post API

@frozeman
Copy link
Author

frozeman commented Jun 21, 2017

Another change could be we name it ethereum, rather than ethereumProvider and attach properties like

ethereum.shh
> true
ethereum.bzz
> "http://localhost:8500"
ethereum.eth
> true
ethereum.personal
> false
ethereum.noYetKnown
> undefined

ethereum.send
> function
ethereum.on
> function
...
// all other event listener functions like `once` etc should also be there

**EDIT** As swarm works mainly through HTTP, we can expose the HTTP endpoint on its property. If there is no `ethereum` or `ethereum.bzz` then users can connect to the gateway at `http://swarm-gateways.net`

@danfinlay
Copy link

One of our hopes is that eventually we could have multiple providers in the same window, like multiple shard clients. Maybe the connect method could return a fresh provider for the requested chain, or error?

@tcoulter
Copy link

👍 for the rename.

@frozeman
Copy link
Author

frozeman commented Jun 22, 2017

@FlySwatter The problem is that this will be hard to solve in Mist and status.im which actually run nodes.
On the other hand we want that the connection is immediately available so that dapps can startup and request and don't need a custom startup logic to select the network.

You also don't load a website and tell the browser to connect to the internet or some ethernet. If so you would do that in some custom logic, which is what a custom rpc function like eth_connectToBlockchain would allow.

@frozeman
Copy link
Author

I edited the comment, above to reflect handling of swarm endpoints. As they don't go through the provider, we need to add the url there.

I will talk to @zelig how spam if your local node is prevented.

@kaikun213
Copy link

Hello all,

Good discussion. I would agree with the two proposed things:

  1. standardize an EthereumProvider
  2. standardize a minimal set of API methods needed

However it is important to differ between both and discuss them separately.

To the first point you suggest an ethereumProvider.on(subscriptionType, handler) with subscriptionType being e.g. eth_subscription, ssh_subscription or any other API. Where would we define the method to subscribe to? If we comply with the Ethereum PubSub you mentioned we would pass another parameter ethereumProvider.on(subscriptionType, handler, params). Params would consist of the method name and the possible parameters such the same way as passed to send.
E.g. params = ['newHeads', []] or ['eth_getBalance', [address, blockNumber]]

Regarding the second point, in eth_subscription should be only the standardized minimal supported method set. Same as e.g. an eth module at the moment provides a specific set of methods. All other "feature" methods should be decoupled in other modules to keep it relatively simple for the dApp-dev.

@frozeman
Copy link
Author

frozeman commented Jul 13, 2017

Ok maybe i didn't explain fully.
Subscriptions are not created when calling ethereum.on('notification', cb)
You still need to create your subscription before using ethereum.send('eth_subscribe', ['pendingTransactions', {some options}]). In the event callback of the "on" you get all subscriptions coming through. You need to match them yourself with the subscription ID you got from ethereum.send('eth_subscribe'...
This will be done automatically in web3.js 1.0. But dapps devs and other libs could build that themselves.

This way the subscription don't differ from other function calls, except that there is a way to receive notifications from the node.

To unsubscribe call ethereum.send('eth_unsubscribe', ['subId'])

@kaikun213
Copy link

kaikun213 commented Jul 15, 2017

Ok. Which advantage do we gain by splitting up the process?
Except giving the user a very simple way to have the same callback-wrapper function for all subscriptions.
Wouldn't it be even simpler and more accurate to have only ethereum.subscribe(subscriptionType, handler, params),returning a promise of the subscription ID and ethereum.unsubscribe(subscriptionId). Very simple and generic enough to fit for every subscription/method. (params = [ 'rpc', [params]])

@frozeman
Copy link
Author

thats what it does, but you need the .on(...) to get the notifications.
I am not sure about a promise return. A callback for this simple 3 functions is totally fine. this is a low level api anyway.

@frozeman
Copy link
Author

frozeman commented Feb 21, 2018

@kumavis @tcoulter @danfinlay @MaiaVictor @kaikun213 As i just now wanted to finally add the ethereum provider to Mist, i realised that our new design doesn't support batch requests, which especially for remote node connections, like Infura etc are very useful, if one fires a lot of requests.

Im thinking along this lines:

// callback: a error first callback
ethereum.sendBatch(requests, callback)
> undefined

requests = [{method: 'eth_something', parameters: [1, 2]}, {...}, {...}]

// CALLBACK return
> err, result

result = [{result: result1}, {error: Error('some error')}, {result: result3}, ....]

Matching on the client (library) side happens here through the array indexes. The EthereumProvider class has to make sure that the results are returned in the right order.

What you guys think?

@tcoulter
Copy link

Initial reactions are 👍.

I was thinking we could alternatively (or also?) overwrite ethereum.send() and detect and array as the first parameter, and if so send a batch request. @frozeman and I chatted privately about it and it might aggravate TypeScript users. I'd be for it, but it's definitely personal preference.

Otherwise looks fine from here!

@benjamincburns
Copy link

benjamincburns commented Feb 21, 2018

@tcoulter pinged us Trufflers to weigh in. For those who don't know me, I maintain Ganache, and have done so since October 2017. As such I'm missing some of the legacy of Ethereum development, but I've definitely been around the block w.r.t these sort of discussions.

From my perspective, I'm not sure I fully understand the goal of the provider. Taking the provider interface required by web3.js, it doesn't expose anything specific to Ethereum's RPC API, so transport encapsulation is really the only responsibility which fits its current incarnation. However, it exposes nothing useful about the transport itself. Most proposals on this thread seem to follow suit, and much of the argument seems to be around whether or not to expose those transport-level details. My question is, if this thing isn't exposing details specific to Ethereum's RPC API, and if it exists solely to encapsulate the transport layer, then where else would transport-level details go?

If I get a vote, it's on the branch of proposals which expose common methods for transport control which is mostly agnostic to the transport mechanism in question, like connect, close, etc, plus events to those effects. I'd rather code a dapp against a transport encapsulation which exposes these controls/events than have to worry about how each implementation encapsulates this otherwise common logic.

I know this isn't meant to be defining the web3.js library, but since that's presently the most canonical example, I offer it up as illustration. Today the web3-providers-http module doesn't expose a close method (that I can find, would use this to end KeepAlive connections), and IIRC, the web3-provider-ws module exposes one behind a "private" _connection object.

On the topic of whether or not exposing an EventEmitter interface should be required, I think it's very likely that we'll continue to expand upon the limited existing push mechanisms (i.e. Pub/Sub), so I think it's reasonable to include this in the baseline spec, as well. Personally I'd prefer it if the provider abstracted away just a little bit more (e.g. on('newHeads') instead of send({ ... method: 'eth_subscribe', params: ['newHeads'] ... })), but that's not super important. What is important is that we make the push mechanism a first-class part of the provider spec.

@frozeman
Copy link
Author

frozeman commented Feb 22, 2018

Hi,

a lot of text :) not sure if i got all of it.
This specific provider is not meant to replace http, ws or IPC providers, but it should be the browser(like) native connector point of dapps. If a dapp wants to connect to remote nodes, or over other ways it still should use their own provider.

We could surely think of even making those providers as abstract as this one.

The main idea is that web3.js gets removed as a first class citizen, and to give a simple and flexible enough interface for dapps/libraries to work within an ethereum compatible browsers.

The other advantage of the ethereum object is, that we can add functionality, like booleans for ethereum.bzz === true to test for compatibles, in the future.

Concerning the subscription its more like:
ethereum.send('eth_subscribe', ['newHeads']), callback).
Here the callback return will be the subscription object with the subscription ID, and you then check on ethereum.on('notification', handler) for the incoming subscription, and match the IDs.

So i would say subscriptions, are already a first class citizen according to this spec.

I am exposing the connection events, as this could be relevant to the DAPP, to warn, or stop functionality. But wouldn't expose the control over the connection itself. Currently in my implementation, if it disconnects, and you send again, it tries to reconnect automatically.

Ideally Mist, or MetaMask will hide the connection complexity away, and show warnings to users themselves.

@frozeman
Copy link
Author

frozeman commented Feb 22, 2018

ps. i updated the comment to include errors in the results.

The main discussion i would like to have is more about the ethereum.sendBatch() agree on that and the whole API, and start adding it to ethereum browsers like

@frozeman
Copy link
Author

frozeman commented Feb 22, 2018

Concerning the sendBatch error handling. Currently i implemented it that all errors are in the result array, event if its a connectivity error. This would make it easier for dapp developers to handle those, as they dont have to check for callback(error, result) error AND result.map((result) => result.error ), but just iterate over the result, but then we could make this a callback return ONLY the result array, and not the error object.
Here is the provider implementation in action:
screen shot 2018-02-22 at 12 48 05
screen shot 2018-02-22 at 12 50 10

And here the batch error handling of connection errors:
screen shot 2018-02-22 at 13 01 51
OR:
screen shot 2018-02-22 at 13 03 21

@ricmoo
Copy link

ricmoo commented Feb 22, 2018

Hey @frozeman

Still catching up on this thread, and will probably have to read it a few times, but love the overall idea.

Quick background on how ethers.js handles connections. First of all, complete separation of a Provider and Signer; a Signer can "somehow" sign things and a Provider can "somehow" talk to the blockchain. Currently there is full support for providers that can use Etherscan, INFURA, JSON-RPC, an existing Web3 Provider, and then meta Providers (like fallback and round-robin).

The Web3Provider, which wraps a Web3-style provider and exposes it as an Ethers Provider, no calls are batched.

For the Web3 Bridge which allows a standard Ethers Provider and Signer to be used inside a Web3 instance I return batch requests in the same way Web3 Providers do to be compatible. The Web3 Bridge is used on projects like Ethers Wallet (iOS) and ethers.io where I need to create an injected Web3 Object for customers that code against the Web3 API, but the backend is an Ethers Provider.

I still need to read through the whole document, but is the idea of EthereumProvider to be what instances of Web3.providers.HTTPProvder (for example) would be?

I was wondering the other day whether we even need an explicit sendBatch. Batching is something the provider could decide to do or not do. Deferring a fetch to whatever backend using setTimeout(sendAllRequests, 0) for example would automatically batch all requests made in the current event loop (and some sensible cap can be placed limiting batch sizes). It makes the API simpler, the developer does not need to think about batching (which often goes awry), and implementors of Providers do not support batching (e.g. Etherscan) don't need to emulate it with multiple calls that then get stapled together to look like a batch result. Things that support batching just get better performance even if the developer didn't intend for it.

Just an idea; here is the discussion so far. Kind of outdated, but this thread made me think of it, so maybe it makes sense to prototype with the Web3Provider.

@ligi
Copy link
Member

ligi commented Mar 15, 2018

Really like the idea - just as a note from a mobile side: it would be awesome if EthereumProvider will (as far as I see continue) to consist only of methods. No variables. No nested classes. Then we can use WebView.addJavascriptInterface to add the provider and not have to use hackish workarounds ;-)

@frozeman
Copy link
Author

I talked with @ligi and i think its better to not add the ethereum.sendBatch(), as it is protocol depended and assumes to much of how the nodes are called.
Mist or MetaMask can introduce its own batching logic to increase performance, developers shouldn't care about that.
Currently batching is not a good option as it returns based on the slowest response anyway.
web3.js will still keep batching if a developer connects directly to a remote node, but the ethereum provider should rather hide that logic from dapps and libraries.

Im going to implement the ethereum provider as described in the specs above in Mist and encourage @danfinlay @kumavis @ricmoo to do the same for their ethereum browsers.

@danfinlay
Copy link

This sounds good to me, I look forward to catching up to your API!

@ryanio
Copy link

ryanio commented Jul 6, 2018

Hey everyone,

Thanks to Fabian's ideas and everyone else's discussions and contributions in this thread, Marc and I on the Mist team have designed an EIP for the Ethereum Provider API called EIP 1193.

Document: https://github.com/ethereum/EIPs/blob/master/EIPS/eip-1193.md
Discussion: https://ethereum-magicians.org/t/eip-1193-ethereum-provider/640

We would like to invite everyone to take a look and continue any discussion, feedback, improvements, etc. in the ethereum-magicians thread to help finalize the proposal. We look forward to your input!

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

No branches or pull requests

11 participants