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

BeaconNode APIs #25

Closed
djrtwo opened this issue Apr 9, 2020 · 31 comments · Fixed by #73
Closed

BeaconNode APIs #25

djrtwo opened this issue Apr 9, 2020 · 31 comments · Fixed by #73

Comments

@djrtwo
Copy link
Contributor

djrtwo commented Apr 9, 2020

Thus far, this repo only consists of "validator" APIs. To aid in application developers, we aim to conform upon a set of user-level "beacon node" APIs.

Currently, Prysmatic has a number of user-level APIs defined in prysmaticlabs/ethereumapis with some decent traction with block explorers and other applications, and Lighthouse has a number of APIs that have also begun to be used by various applications (link?).

I propose that we make a list of the obvious items first (and especially any that have overlap between prysmatic and lighthouse). We can list them out, get a feel for the general categories, structure of arguments, etc and move from there. Explicit notes on how/why users are using them today will be helpful in better understanding the API in general.

After that, we can debate anything that seemed "non-obvious" or maybe client specific

@mcdee
Copy link
Contributor

mcdee commented Apr 9, 2020

The three general areas that I've found useful are chain info, node info and validator info. To provide some more information on each of these:

Chain info

These are properties of the chain and node, and should be inconsiderate of the node's connection to the network. Fields include:

  • Ethereum 1 deposit contract address
  • Various chain parameters:
    • Seconds per slot
    • Slots per epoch
    • Max seed lookahead
    • Min validator withdrawability delay
    • Persistent committee period
    • Min epochs to inactivity penalty
    • Eth1 follow distance
    • Far future epoch
    • Genesis fork version
    • Minimum deposit amount
    • Maximum effective balance
    • Effective balance increment
    • Ejection balance
    • BLS withdrawal prefix
    • (Possibly more here)
  • Current epoch
  • Previous fork version
  • Current fork version
  • Current fork epoch
  • Next fork version
  • Next fork epoch

Node info

These are reflections of the node's current state, and as such can depend on the node's data and connection to the network. Fields include:

  • Genesis time
  • Genesis validators root
  • Node identifier (definition would need to be firmed up; possibly network key, ENR)
  • Software version (could be structured, could be free-form text)
  • Listen multiaddr
  • Addresses of connected peers
  • State of the node's syncing process (is it not syncing, fast syncing, full syncing? Again would need definition)
  • Current epoch
  • Current slot
  • Current block root
  • Finalized epoch
  • Finalized slot
  • Finalized block root
  • Justified epoch
  • Justified slot
  • Justified block root
  • Previous justified epoch
  • Previous justified slot
  • Previous justified block root

Validator info

These are a simple way of obtaining data about a defined subset of validators. In Prysm this is a streaming gRPC call, which means that an interested can set up the list of validators they care about and receive real-time updates i.e. after an epoch transition:

  • Public key
  • Index
  • Epoch to which the data pertains
  • Status of the validator (deposited/pending/active/etc.)
  • Transition timestamp (the timestamp at which the validator enters its next state, so for example deposited->pending, exiting->exited; invalid if the state does not have a transition timestamp e.g. active)
  • Balance
  • Effective balance

@paulhauner
Copy link
Contributor

paulhauner commented Apr 9, 2020

Deleted post that I accidentally submitted

@paulhauner
Copy link
Contributor

paulhauner commented Apr 10, 2020

I'd suggest a more segregated approach than to @mcdee. My goals are to:

  • Avoid mixing constants (eth1 follow distance) with variables (current head), where appropriate.
  • Make clear distinctions between the current slot and the head slot.
  • Reuse existing data structures
  • Accommodate for the scenario where we're waiting for genesis from eth1 and some values are known and other are not. I think it would be ideal that if we're pre-genesis that an endpoint should either fully succeed or fully fail with some standard HTTP error, instead of just null-ing out some fields. I think this would help the API be more explicit and the implementation simpler.

This would be my suggestion to cover @mcdee's endpoints:

  • /spec (existing: Lighthouse)
    • Returns the configs struct. Clients should already have an instance of this struct from testing and it's also used by eth2-testnets to succinctly define a chain.
  • /clock (no existing examples) which returns:
    • current epoch
    • current slot
    • current unix time
    • slots per epoch
    • seconds per slot
    • previous fork version
    • current fork version
    • current fork epoch
    • next fork version
    • next fork epoch
  • /beacon/head (existing: Prysm, Lighthouse):
    • head slot (not current slot)
    • head epoch (not current epoch)
    • roots and slots for head block
    • head block state root
    • finalized/justified roots and epochs
  • /beacon/genesis endpoint (no existing examples):
    • genesis validators root
    • genesis time
    • genesis slot
  • /beacon/validator (existing Lighthouse, Prysm)
    • Epoch to which the data pertains
    • Everything in the Validator struct
    • The state.balances value
    • A "status" (deposited, pending, etc -- this needs clear definition)
  • /node/info
    • enr
    • listening multiaddrs
    • software version (TBD)
    • syncing status (TBD)
  • /network/peers (existing Lighthouse, Prysm)
    • list of connected peers

A rough consumer lifecycle might look like:

  • Get /spec and store the constants locally.
  • Use /genesis to see whether or not eth1 genesis has occurred yet. If it has, store the parameters.
  • Poll on /clock to check the current slot and perhaps figure out the time to the next slot.
  • Check /node/info to see if the node is synced.
  • Learn the finalized epoch with /beacon/head

Transition timestamp (the timestamp at which the validator enters its next state, so for example deposited->pending, exiting->exited; invalid if the state does not have a transition timestamp e.g. active)

This would be useful info, but we have to be very clear that these are estimates. For a lot of the validator transitions the exact times are unknowable.

Epoch to which the data pertains

I agree that this would be useful. However, it raises something else. I think that it would be useful to allow queries that specify an epoch to also specify a block or state root. This allows users to query about things that aren't in the canonical chain. I imagine this would be useful for block explorers and slashers.

@mcdee
Copy link
Contributor

mcdee commented Apr 10, 2020

@paulhauner sorry, to clarify: I was listing data I am using rather than suggesting an API structure.

Regarding your proposed API structure: it looks fine, although I'm in general more of a fan of APIs that target use cases, for example if I wanted to create a VoluntaryExit transaction, under your proposed endpoints I would need to call:

  • /spec to obtain the domain type for voluntary exit
  • /clock to obtain the current epoch and fork version
  • /beacon/genesis to obtain the genesis validators root
  • /beacon/validator to obtain the current state of the validator (don't want to exit if we aren't in a suitable state)
  • possibly /node/info to handle any quirks of the node to which I'm sending the transaction

That's quite a lot of work, and likely to be repeated for any non-trivial request. I'd rather look at this from a user's point of view and provide APIs that meet their requirements, rather than are constrained by internal data structures of the beacon chain.

Totally agree with the view of avoiding mixing constants with variables where possible.

[Transition timestamp]
I believe that most timestamps are well-known outside of the movement from Pending to Deposited is variable, due to timing of Ethereum 1 blocks. Stating that the relevant timestamps are best estimate, though, is a sensible thing to do.

[Epoch to which data pertains]
There are two points here. One is to provide the epoch to which data pertains, the other to ask for data at a given epoch.

The vast majority of requirements from client to node will be for real-time data. Explorers and the like will take the real-time data and put it in to a format useful for historical reporting. General users care about the state of the chain and their validators now, rather than last week. Adding an "at epoch x" style parameter to endpoints requires a lot of work, especially around edge cases. Is this something that is really necessary? (Note that I'm not against a node implementing this if they really want to, but making it a requirement for all nodes seems like overkill).

Instead, streaming APIs do tend to work well with this type of data. If we take a streaming /beacon/head as an example, at each epoch transition the node could spit out the current info as it sees it to everyone listening, along with the epoch to which the data pertains. This allows an explorer, for example, to be able to recreate its data from scratch just by starting up a new node and listening to the stream as it syncs.

Poll on /clock to check the current slot and perhaps figure out the time to the next slot.

Could I please ask here that we don't define an API that requires polling? Polling is always ugly, involves trade-offs that hurt either the server or the user, and usually end up with some form of limiter in place that confuses everybody. Streaming APIs for data that is fluid are easier to work with, as real-time as the node can manage, and don't result in wasted cycles (which may not matter individually but certainly will at scale).

@mpetrunic
Copy link
Collaborator

@paulhauner I would add/propose couple of changes

/clock (no existing examples) which returns...

I would propose this to return Server sent events stream with just current epoc, slot and unix time.

And have additional apis:

  • /beacon/fork and /beacon/fork/stream (sse event stream to listen on fork changes) to return fork related data

/beacon/head

This could also have /beacon/head/stream so clients can subscribe to head changes

@paulhauner
Copy link
Contributor

I was listing data I am using rather than suggesting an API structure.

Sorry, I realized afterwards that this might be the case.

although I'm in general more of a fan of APIs that target use cases

This seems like it approximates an opinionated vs unopinionated decision. I'd say that if we're going to choose opinionated then we need quite a firm list of use-cases and their priorities. I'm a little hesitant to go down the opinionated path since I don't feel I have a firm grasp on the uses cases of this application beyond staking. I expect to see a lot of emergent use cases and feel that it's a little early to become opinionated yet.

if I wanted to create a VoluntaryExit transaction

Yes, but then there's the counter argument that if I wanted to do some simple task then I would have downloaded a bunch of unnecessary information or that perhaps there's so many endpoints it becomes unclear which one to use. I think this type of discussion really needs to start from a list of prioritized use-cases as opposed to ad-hoc scenarios.

General users care about the state of the chain and their validators now, rather than last week.

Personally I've done quite a lot of work in extracting old eth1 data for analysis. I'd imagine the presence of libraries like ethereum-etl indicates to me that other people find this interesting/necessary too. I'm keen to build an API that considers those doing research and analysis as first-class users.

If you're giving historical information based on the state at your head, it doesn't seem particularly challenging to parameterize that over any state. We already do this and I believe Prysm does in some places too.

streaming APIs do tend to work well

would propose this to return Server sent events stream

I haven't worked with SSE on the server side before, so I need to do some research here. A quick poke around for Rust and Nim libraries gives the impression that support is thin.

Do you have some suggestions on how you wish to do streaming in HTTP @mcdee?

@mcdee
Copy link
Contributor

mcdee commented Apr 10, 2020

This seems like it approximates an opinionated vs unopinionated decision.

It's not so much opinionated, more that there are obvious clusters of info, for example:

  • current state of the chain
  • current state of the node
  • current state of a given validator
  • information needed to sign a transaction

I'm not suggesting we have lots of endpoints of the form /stuffformytransaction, but thinking about general use cases rather than specific ones can help to create a cleaner API for the end user.

Yes, but then there's the counter argument that if I wanted to do some simple task then I would have downloaded a bunch of unnecessary information

We already have that in your proposed API; the entire config structure is required to find out how many seconds per slot there are, for example. But I don't think that's a terrible thing: if data is static then sending a larger chunk seems like a fair trade-off as it will only be requested once per process run; if data is being pulled frequently then perhaps reducing the amount of data returned per call is sensible.

There's no easy way to consider if 1 request with 2KB of data is "better" than 2 requests each with 1KB of data. I'd be inclined to group data where appropriate and lightweight, and break out anything that required significant data access or computation in to their own endpoints, but that's just my view.

Personally I've done quite a lot of work in extracting old eth1 data for analysis...

Sure, but I was talking generally. How many people do an ETL of Ethereum data, compared to how many want to know their current balance? And again, I have no problem with the historical functionality being standardised, but not so much of a fan of it being a requirement.

Do you have some suggestions on how you wish to do streaming in HTTP

There are a few streaming options out there (web sockets, SSE, HTTP/2 server push) but 'm not thinking in terms of a particular protocol at current; I'd rather we sorted the API requirements and then picked an protocol from those that can meet them.

@paulhauner
Copy link
Contributor

the entire config structure is required to find out how many seconds per slot there are, for example.

I did include seconds per slot in the /clock endpoint, but I'll take this as a general point.

I'd be inclined to group data where appropriate and lightweight

Agreed

How many people do an ETL of Ethereum data, compared to how many want to know their current balance?

Good question, I don't think we really know what the main uses for phase 0 APIs are apart from staking and block explorers. However, I would say that knowing your balance change across some period would be interesting to many stakers and arguably a basic requirement. E.g., how much have I made in the past month? I don't mind making them optional, but I think past balance and participation metrics will be very useful for validator UX. Specialised endpoints is always an option, but I am partial to a generic solution.

I'd rather we sorted the API requirements and then picked an protocol from those that can meet them.

Sure. It seems to me your requirements stated so far can be fulfilled by:

  • Static endpoints
    • The consensus is already at REST/HTTP/JSON and it is suitable.
  • Streaming
    • No consensus yet.
    • SSE (server side events) have been suggested
    • Lighthouse already uses Websockets, but I'm not attached to them. SSE seems to be more attractive as a protocol but is not out-of-the-box for some teams. I usually find the input of @arnetheduck helpful when choosing technologies, I'd be keen to hear his input here.

Generally, I'm finding it hard to figure out what closes this issue so we can unblock #24 (#24 (comment)) and continue making progress. I assume static endpoints are a given, so perhaps we need to wait to see what people think about having streaming endpoints? Then, if we do all agree we need streaming, we can look at the requirements of those endpoints and choose a technology?

Contrary to my previous comment (#25 (comment)), perhaps doing detailed listing of endpoints is something that we want to avoid here and instead do that once we've got a protocol to express the endpoint in.

@rolfyone
Copy link
Collaborator

I think if there were concrete use cases, that'd be helpful in getting things moving fairly quickly. That way we could build out per use-case, without necessarily all use-cases being finalized, plus it gives a 'guide' to people that wish to consume things. It can also inform the decisions on what should be included / excluded in response objects.

To my mind there's also differing consumers, from validators that want to get the work done, to monitoring tools that want to get an overview. If we define these well, we can tailor endpoints to their needs, and not build out generalised endpoints until the specific use cases we have are met (or maybe never?)

The validator use cases seem to be relatively well encapsulated, as they

  • need to know what to do and when
  • need to be able to produce/attest to blocks in a way they won't get slashed
  • join / leave as a validator

Monitoring / block explorers are presumably relatively well defined in their use cases as well. Unfortunately I'm not really across the entire scope of everything (is anyone?), but maybe we need resources from each area to define a minimum set of use cases so that we could build out an opinionated spec...

The downside I guess is that we don't necessarily have everything anyone could ever need, but then maybe that also helps limit things that are built so everything that is actually built is required?

The upside is we don't need to be experts in all areas to understand what is needed and why, and similarly the use cases for people writing software to consume the endpoints will be available to have a high-level view of how the endpoints are put to use.

As an aside, TEKU is starting to publish the API here although at the moment its not automatically generated on pushing to master, so it will be slightly be hind.

@prestonvanloon
Copy link
Contributor

Sorry, I am late to this party. How can @prysmaticlabs be helpful here?

We have written out many use cases in this product requirements document. We found these requirements to be satisfactory in meeting most or all of the needs of eth2 block explorers which are already in (testnet) production.

Is anyone looking or has looked at the differences between Prysm's API and eth2.0-APIs?
I'd be interested to hear what is missing, lacking, confusing, inappropriate, infeasible, irrelevant, or specific to Prysm use cases. We tried to make this protocol agnostic to the underlying needs at Prylabs but it is possible that some things have leaked.

Prylabs team has many opinions on how this API should be designed which is reflected in our implementation of ethereumapis, but we're not sure how to best contribute to this conversation.

@mkalinin
Copy link

What if we have fine-grained up to a certain extent API allowing for various use cases build atop of it? If this would be the case then tooling satisfying real user needs could be build with zero involvement of client developers and be re-used with any client software supporting that API. This is aligned with the following rule of thumb: keep your API fine-grained, services coarse-grained.

Ethereum JSON-RPC showed that implementing use cases (like filters) as a part of a client is cumbersome and puts huge burden on client developers, especially, in those parts that have statefulness as a requirement. If we go down the use cases road then we will likely face the same problem. Some of the pain points have been previously shared in this thread.

One of two main responsibilities of the client is to keep beacon chain's head and provide access to the data it already has with no additional computations required. We probably want to be in line with single responsibility principle and avoid any unnecessary complication of already sophisticated beacon chain client software.

Of course, there is a drawback. Without additional tooling this approach makes beacon chain client almost useless for end user. It also could delay delivery date as this tooling will have to be created by some party. But this drawback seems to be a short term issue and shouldn't have any impact in the long term.

Currently, I am not much into the details of the API, this is merely a high level point of view to the problem of API design in our context. Feel free to skip these thoughts if you find them irrelevant.

@mcdee
Copy link
Contributor

mcdee commented Apr 14, 2020

The point about dumping internal data structures as API endpoints Vs. doing some work to present useful information to end users is an important one. The former puts significant burden on every entity that wants to use the data, whereas the latter places that burden on the beacon node client.

In general I prefer the latter, from both the producer and consumer side. Producers benefit because the information provided is what the user wants, and they aren't continually bombarded with questions about how to translate from the internal information to the external information, or why their guess at translation from internal to external information is not consistent with some other implementation. Consumers benefit because they can obtain the information they want without needing to understand the details of the underlying implementation. And the separation means that internal data structures can be altered if necessary without impacting on their consumers.

Obviously there are caveats to this approach. The translation work needs to be bounded and lightweight enough to not have an impact on the beacon node's primary responsibilities.

A good example of what I'm trying to say here is the validator info. Clients have some approximation of the validator info internally that maps to the Validator struct in the specs:

    pubkey: BLSPubkey
    withdrawal_credentials: Bytes32  # Commitment to pubkey for withdrawals
    effective_balance: Gwei  # Balance at stake
    slashed: boolean
    # Status epochs
    activation_eligibility_epoch: Epoch  # When criteria for activation were met
    activation_epoch: Epoch
    exit_epoch: Epoch
    withdrawable_epoch: Epoch  # When validator can withdraw funds

But this information is not a lot of use to answer some the questions most users want to know about their validator, like "I just submitted a deposit; when will my validator activate?" or "what is the balance of my validator?", so just providing the data structure and expecting the consumer to do the work is not, in my opinion, feasible.

@mkalinin
Copy link

@mcdee If consumers of that API would be end users with no other option then I would agree with you. Updating data structures impacting client developers, they will have to update and test the API, otherwise, if there is no test suite then it will likely annoy consumers with software bugs impacting on them as well.

My point is more about sharing responsibilities between clients and tools for end users that could handle complicated use cases. Of course, developers of those tools will have to track changes in client API and maintain its test suite as well, and so forth. But that would be their responsibility. If we don't have resources or according to other reasons we won't follow this approach then we have to keep this tooling inside of beacon client.

Probably, a compromise would be to start with basic use cases as you've mentioned and keep a lower-level API to allow for implementation of the others. I don't think that "What would be my revenue for this year if deposit pace and software failures stay at the same level?" is something that would fit the beacon client. User requests will highly likely go far beyond the responsibility of beacon clients.

@mcdee
Copy link
Contributor

mcdee commented Apr 14, 2020

@mkalinin On the complicated use cases, I agree that we don't want the API to provide that type of information (for all sorts of reasons). I would suggest that the example you gave wouldn't fall under my (admittedly hazy) idea of "lightweight".

But then I just said that consumers would like to know when their just-sent deposit is likely to be activated, which will also take a chunk of effort to work out. Which makes me think we really need to decide on the use cases that we do want to support (in addition to the low-level data, as you say) to be able to proceed here.

@arnetheduck
Copy link
Contributor

Considering that the beacon node is critical infrastructure, it would make sense to not burden it any more than is absolutely necessary, for several reasons: security surface area, performance constraints, complexity of maintaining cross-client compatibility, as @mkalinin points out.

Beacon nodes are not databases or general-purpose computation engines - that task is better left to after-market products that can cater to specific needs - to use an example, "what was my balance a month ago" is not something that the beacon node must calculate as part of its normal operation of selecting a head and maintaining a state, and therefore should really not be in the API, but rather the API should be able to provide data such that a third party may build a tool that can use the (much smaller) API supported by all client implementations.

We want multiple client implementations of the critical consensus parts, not reimplement SQL in every client.

Every API that diverges from being a task that the node has to perform to maintain its "normal" operation should therefore pass a much higher "utility" bar before being admitted into the standard, because inevitably this is where implementation differences will start to show - the data in the API, not being critical for normal operation, will eventually end up divergent and unreliable across implementations.

Thus, for the first release of the API, I'd go with minimal and complete - any computed and opinionated API also increase the risk of the core API not being complete because they also serve as crutches that the initial tooling will come to expect.

The other thing we could be looking at would be an extensibility mechanism where clients advertise what they support: a minimal core API, as well as extensions to that, where different clients freely can experiment with risky API that do additional calculations. Think libp2p, but for RPC: clients advertise certain capabilities and consumers can smartly choose which parts they use - this also provides an upgrade path and a documented way to extend the API in a straightforward way, beyond the minimal and complete core.

Regarding polling, the risks are race conditions - you can guess the next slot from /clock but you cannot be sure your guess matches the servers - there are two options here: letting the server decide (when to include a slashing) or an optimistic API ("include at block X - fail otherwise).

Regarding streaming, one thing that stands out as difficult is handling forks/reorgs correctly - when this happens, past data becomes invalid and we should design a data feed that takes this into account, ie clearly signals that a rewind has happened so that third-party tooling at least can be implemented correctly - polling API will generally have this problem as well.

over any state

generally, for node operation we need little beyond the latest finalized state. this ties in to there not yet being a way to synchronize state between clients, but synthesizing any state in a client may be an expensive operation - this is also why state sync is difficult: clients need to agree on which states to sync, and thus keep "stored" - 1-2 "blessed" states per weak subjectivity period have been suggested (ie the first state every 6 months could be a "blessed" starting point that acts as stand-in genesis and thus is transferrable) - as an example, a minimal and complete API here is to be able to transfer each block instead, from genesis.

splitting the protocol into several features like core/head and core/historical would solve this and allow clients to provide varying degrees of API power as well as targeted test suites - in fact, one can imagine that with a complete and canonical core API, the "middle layer" can be reused or implemented natively in each client - it's trivial for example to build a historical state adapter that runs in a separate process from the client if the block api is solid.

Ethereum JSON-RPC showed that implementing use cases (like filters) as a part of a client is cumbersome and puts huge burden on client developers,

+1

burden on the beacon node client.

specifically, it puts the burden on beacon node client developers, and having to negotiate the features between multiple implementations for every iteration, or seeing a fragmented community. A minimal and complete API does not have this issue because it already exposes all data available in a natural way.

server-side-events

Hm, I have no experience with these either, though I agree that on the surface they look attractive in that they enforce a "clean" and simple one-way stream - worth exploring at least as it looks like changing from SSE to websockets would be trivial because of the more constrained capabilities of SSE.

@djrtwo
Copy link
Contributor Author

djrtwo commented Apr 19, 2020

Proto and I have surveyed the existing client APIs and attempted to organize them into a minimal, representative set

Check it out here

Design goals (copied from doc):

  • Clear consistent grouping, preferably nested for better organization
  • Explicit path formatting, no paths without group, no groups with only something at root
  • Versioning in path prefix
  • Clarity about GET vs POST differences, especially were we changed from one to the other
  • Consistent query system. Currently params can be in query, path, or post. Too many ways
  • Fit REST well, arguments of Peter (Geth) are valid: we do not want expensive round-trips to the server with open sessions
  • Stream endpoints need to be reviewed, it is something Prysm specific, others seem fine without it
  • Review where websocket versions make sense
  • No aliases or weird unnecessary variants. Settle on one thing, and make it clear and usable
  • Split "real time" information that changes from "identified" information that will not change anymore. Make caching easy.
  • Be clear about expectation on what the endpoint is used for, and so why a client should support it
     

Open questions:

  • choice between plural and singular as default naming choice
  • the use of /full suffix. This is used to avoid GET methods for an endpoint that also ends up being a category. For example: /beacon/validators/full is used but could be replaced with /beacon/validators but at the cost of having /beacon/validators be a category (prefix) and an endpoint. This type of doubling up can make API parsers (e.g. swagger) have issues.
  • discuss granularity of endpoints (e.g the /network/host endpoints could arguably be condensed into a single endpoint with a more complex return value)
  • discuss methods to get information about validators.
    • There are currently 9 (11 depending on how you count) methods to get information about validators. Most are useful and currently provided as endpoints, but there is some redundancy. There is a tradeoff between server/user complexity. If we provide many ways to get different validator info, we reduce user complexity (don't have to process raw validator info as much) at the expense of more computation paths on the server and more complexity in the API.
    • from @arnetheduck: "Thus, for the first release of the API, I'd go with minimal and complete" -- /beacon/validators and /beacon/forkchoice are the biggest potential offenders right now. Both have methods that perform computation on server as an aid to users. One thing to keep in mind is that the beacon chain and state is much different than unbounded user-land in Ethereum today. I would guess that much of these types of issues in the eth1 are related to filtering/querying that unbounded state.

NOTE: Validator API proposal in separate page in spreadsheet. There is a solid start in this repo, and probably some points to debate. Prioritizing user and debug APIs in initial discussion

@mkalinin
Copy link

the use of /full suffix. This is used to avoid GET methods for an endpoint that also ends up being a category.

If we want to fit REST well then we likely want to follow its design best practices, a good description is given here. Namespace would be a widely used term for category or group. Usually namespace names are singular, they could be nested but to my experience having a hierarchy with more than 2 layers becomes cumbersome and could be a sign of bad design. If we have a /beacon/validator namespace then we must have something like /beacon/validator/validator_records or /beacon/validator/records as a resource identifier not a /beacon/validators/full.

Consistent query system. Currently params can be in query, path, or post. Too many ways

This also should follow REST ideology. Params in the path are usually resource identifiers, for instance, /beacon/validators/{validator_idx} endpoint returns validator by its index and validator_idx would be the identifier of the validator resource in this case. In the context of REST, query string is used to pass auxiliary information to the requests. For example, /beacon/validators/full should be /beacon/validators?resp={full|stripped} with resp=full or resp=stripped as default value. TBH, I don't know what POST params are, disregarding particular encoding, POST always has a payload containing the data required to create a new instance of the resource.

@djrtwo
Copy link
Contributor Author

djrtwo commented Apr 19, 2020

Thanks for the feedback @mkalinin

I did a pass on the spreadsheet, fitting things more into the standards you mentioned where possible. Most of the items that very cleanly map to the REST standards are outlined in black boxes. The rest of the items are more of "helpers" or don't map cleanly to types/records contained in the spec and more derivative or just sub-sections of a type.

A problem with mapping many of our datatypes to /records/{id} is that we have many ways to "ID" a datatypes. Validators are the most natural, they have an ID we very normally use, but other types are a bit more unclear. For example, do we identify blocks with slot (assuming it's a canonical chain query) or with hash_tree_root.

For blocks (and some other types) /records/{id} is not as natural of a mapping. I think that's why in many of the client APIs today, you see things like [/beacon/block | Get a BeaconBlock by slot or root], thus both potential IDs become query params.

Curious to hear your thoughts on all of the above

@mpetrunic
Copy link
Collaborator

For blocks (and some other types) /records/{id} is not as natural of a mapping. I think that's why in many of the client APIs today, you see things like [/beacon/block | Get a BeaconBlock by slot or root], thus both potential IDs become query params.

You could just name path param "blockId" and in description write that it can be either root or slot or "head". It's easy to distinguish what kind of id is by type of param.
/beacon/blocks/{blockId}:

  • /beacon/blocks/head
  • /beacon/blocks/123
  • /beacon/blocks/0xab2312... (or base64 if we decide to go that route)

@rolfyone
Copy link
Collaborator

I don't really like '/full' as a concept. I'd prefer we have pagination by default on lists likely to be large (/validators) and have a sensible default, but allow users to pass in large page sizes if required. Generally if the query parameters to search the list are adequate, people rarely actually want a 'full' large list like that.
Having query parameters like 'active' on the validators list is a good example of filtering in a meaningful way to get a reduced list. IMO it's more effective to have a parameter than a sub-route. I believe this would be considered 'best practice', which is already covered by @mkalinin.
I think there's use cases where the route could definitely include a slot id etc.

  • /beacon/state/{slot}
  • /beacon/committees/{committeeId}
  • /beacon/validators/{validatorId}
  • /beacon/block/{slotId} or /beacon/block/{blockRoot}

For me if you have an endpoint with query parameters, maybe 2 parameters possible and only 1 of those 2 parameters can be specified, and one must be specified (like /beacon/block in LH and TEKU) then you're actually looking at a path abstraction, not using query parameters. This enforces the usage, and makes the interface clearer to use.

The validators endpoint query parameters having different use-cases for active, pagination etc, show a good use-case for query parameters - where you want the first 100 validators, but only active to be returned. The parameters give you the flexibility to make that query happen, and get only the response you require.

@prestonvanloon
Copy link
Contributor

The current proposal covers Prysm's most important user stories. Here are a few points of feedback from me:

  • Prysm doesn't have a use case for returning eth1 deposits via eth2 API (yet). We haven't implemented this in any way, yet block explorers are able to link eth1 deposits to eth2 validators (see https://beacon.etherscan.io). This isn't to say that these use cases would have been better solved by a deposits API, but that we haven't received any user feedback to support adding any /beacon/deposit/*.
  • The three debug endpoints under /beacon/forkchoice are not used by block explorers today. beaconcha.in counts votes by processing all of the retrieved attestations. Etherscan probably does something similar. Without knowing the data structures returned here, I would argue that block explorers will not use these endpoints.
  • In my opinion, /network/host/* could be a single endpoint, but I would consider all of these endpoints to be internal / debug / optional.
  • I've added a /healthz endpoint which we find very useful for certain deployments. We would recommend to other clients to implement this as well as it can be a great indicator that the beacon node is ready to serve requests. I.e. the node is fully synced, has disk space, has peers, etc. The result of this endpoint is to return 200 for OK, and a non 2xx for not OK to serve traffic. Additionally, the client may return some helpful text to explain why the service is not ready.
  • The proposal doesn't outline this, but we should use versioning in the endpoint path. i.e. /eth/v1/beacon as we do in ethereumapis. I'd also recommend the /eth prefix as to namespace eth from other services if multiple API are being load balanced from the same root URL.
  • In my review of ethereumapis vs the proposal, we have found a few endpoints that we think are unused and are going to propose removing them from our implementation before promoting to v1beta. Namely, GetValidatorActiveSetChanges. We're going to collect some data on API usage to decide which APIs to promote to v1beta in the next few months. We'll be happy to make that information public as well.

Overall, I tentatively support this proposal for /beacon/* and /node/*. I would like to see the data structures, request parameters, and implementation information before committing a full buy-in from @prysmaticlabs.

Talk to you all tomorrow!

@paulhauner
Copy link
Contributor

paulhauner commented Apr 19, 2020

It's easy to distinguish what kind of id is by type of param

Duck-typing works until you have two different types with the same encoding. For example, how do we distinguish between slot/epoch or block_root/state_root?

For example, if I call /beacon/committees/123 am I passing it a slot or an epoch?

A problem with mapping many of our datatypes to /records/{id} is that we have many ways to "ID" a datatypes.

I think this is an important point. We're different from many applications in that we're not backed by a "traditional database" (e.g., SQL) where there's a clearly defined primary key for each of our endpoints.

For example, what is the primary key for a block in /beacon/blocks: slot or root? If you view the set of blocks as a single chain, then slot seems to be most user-friendly. However if you view it as a pool of blocks from many chains then slot becomes awkward.

If we assume there's no natural primary key for /beacon/blocks then I would say that supplying slot or root is applying a filter to the list of blocks. According to the previously linked best practices we should use query params for filtering:

"For filtering the dataset, we can pass various options through query params.
E.g GET /companies?category=banking&location=india would filter the companies list data with the company category of Banking and where the location is India."

If we did take this approach, we'd have to consider what /beacon/blocks?slot=123 returns. In my experience, many people expect zero or one blocks in the canonical chain. I could also see how someone would expect zero or many blocks from many chains with slot = 123. Perhaps using /beacon/blocks?canonical_slot=123 is more explicit.

Additionally, I think it would be prudent to disallow /beacon/blocks without a query filter since it's huge request and it's unclear if it should return a single chain or many.

I'd prefer we have pagination by default on lists likely to be large (/validators)

I agree that pagination will work on endpoints like /validators but it's going to quite difficult in places where there's no stable and sort-able ID like beacon/pool/attestations. Just a thought.

@rolfyone
Copy link
Collaborator

For example, if I call /beacon/committees/123 am I passing it a slot or an epoch?

This is a good concrete use case, i like it. More to the point, the '123' could reference a committee index. As a general rule, this fails because we dont have the information required for the call to work in the first place, we need another number. /beacon/committees/123/456. In this case, the outer would be the epoch/slot (i think it needs epoch?) so if only specifying /beacon/committees/123, then you're referring to the epoch. If you want only committee number 456, you'd use the second use-case of specifying the extra context.

If we assume there's no natural primary key for /beacon/blocks then I would say that supplying slot or root is applying a filter to the list of blocks. According to the previously linked best practices for a filter we should use query params:

👍

"For filtering the dataset, we can pass various options through query params. E.g GET /companies?category=banking&location=india would filter the companies list data with the company category of Banking and where the location is India."

If we did take this approach, we'd have to consider what /beacon/blocks?slot=123 returns. In my experience, many people expect zero or one blocks in the canonical chain. I could also see how someone would expect zero or many blocks from many chains with slot = 123. Perhaps using /beacon/blocks?canonical_slot=123 is more explicit.

👍

Additionally, I think it would be prudent to disallow /beacon/blocks without a query filter since it's huge request and it's unclear if it should return a single chain or many.

Absolutely, i think that's a good example of requiring further route context - just specifying /beacon/blocks would be bad. My preference would be a natural route first, but query parameters if that's the most logical choice. I would see using either block hash or slot id as a way to address this with a route.

I'd prefer we have pagination by default on lists likely to be large (/validators)

I agree that pagination will work on endpoints like /validators but it's going to quite difficult in places where there's no stable and sort-able ID like beacon/pool/attestations. Just a thought.

Attestations might not be so bad? they could potentially be accessed via epoch, or maybe slot even slot? I guess this comes into where its not a natural id, so should be more of a query parameter, rather than a route... Pagination can be ok with query parameters, but it does mean re-running the query constantly as a general rule, so the server value in using query parameters is diminished...

@mpetrunic
Copy link
Collaborator

mpetrunic commented Apr 20, 2020

Duck-typing works until you have two different types with the same encoding. For example, how do we distinguish between slot/epoch or block_root/state_root?
For example, if I call /beacon/committees/123 am I passing it a slot or an epoch?

This is a good concrete use case, i like it. More to the point, the '123' could reference a committee index. As a general rule, this fails because we dont have the information required for the call to work in the first place, we need another number. /beacon/committees/123/456. In this case, the outer would be the epoch/slot (i think it needs epoch?) so if only specifying /beacon/committees/123, then you're referring to the epoch. If you want only committee number 456, you'd use the second use-case of specifying the extra context.

That happens only if you design api poorly. In this case commities belongs to epoch resource, so endpoint would be:
/beacon/epochs/123/committees where you can filter further committess with query params or
/beacon/epochs/123/committees/{committeeId} to fetch single commitee

@paulhauner
Copy link
Contributor

That happens only if you design api poorly.

It seems to me that your solution is to prefix the value with a type, which make sense. But following from your example, what is 42 in /beacon/epochs/123/committees/42. Is that a slot or a committee index? It seems that we instead need /beacon/epochs/123/committees/committee_index/{committeeId}.

Thinking a little more about the /beacon/committees endpoint, here's a comparison between with and without query params:

User wants to request:

  • committees in the canonical chain, via epoch shorthand.
  • committees that aren't in the canonical chain.
  • all committees for an epoch.
  • all committees for a slot.
  • committee for an index and epoch.

Solution with query params:

/beacon/committees?state_root={root},canonical_epoch={epoch},slot={slot},committee_index={index}

Constraints:

  • Either state_root or canonical_epoch must be supplied.
  • All other fields are compatible and optional.

Without query params:

  • /beacon/epochs/123/committees
  • /beacon/epochs/123/committees/index/{committee_index}
  • /beacon/epochs/123/committees/slot/{slot}
  • /beacon/epochs/123/committees/slot/{slot}/index/{committee_index}
  • /beacon/state_root/0x.../committees
  • /beacon/state_root/0x.../committees/index/{committee_index}
  • /beacon/state_root/0x.../committees/slot/{slot}
  • /beacon/state_root/0x.../committees/slot/{slot}/index/{committee_index}

Now consider that in the future someone makes a strong case for including block_root as a query parameter. With query params we can add block_root to that first mutual-exclusion statement. Without query params we're adding four more endpoints.

Given that our data model isn't designed like a typical "customers/products/purchases" database, query params seem to be less opinionated, less verbose and more extensible.

@arnetheduck
Copy link
Contributor

The proposal doesn't outline this, but we should use versioning in the endpoint path

+1, along with a request to discover what API and extensions a node supports

@mcdee
Copy link
Contributor

mcdee commented Apr 20, 2020

Regarding streaming, one thing that stands out as difficult is handling forks/reorgs correctly - when this happens, past data becomes invalid and we should design a data feed that takes this into account, ie clearly signals that a rewind has happened so that third-party tooling at least can be implemented correctly - polling API will generally have this problem as well.

This can be handled far better in a streaming API than a polling one. A streaming API can just send an update, whereas a polling one would require the user to repeatedly look back at data (which, realistically, won't change the vast majority of the time).

If a full-blown streaming API is not something that people want to sign up for then perhaps a single endpoint that sends details of reorgs (and normal operations that act as triggers for subsequent client operations, such as "epoch transition computed") would be a compromise that would work for more people.

@mpetrunic
Copy link
Collaborator

It seems to me that your solution is to prefix the value with a type, which make sense. But following from your example, what is 42 in /beacon/epochs/123/committees/42. Is that a slot or a committee index? It seems that we instead need /beacon/epochs/123/committees/committee_index/{committeeId}.

That was not my idea. We should definitely document what kind of id type we support on each endpoint. That said, committee endpoint is a bit tricky as it doesn't have single identifier but:

  • /beacon/epochs/123/commitees with query params canonical={boolean},slot={slot},committee_index={index} makes sense but query params shouldn't be mandatory, they are just filters.

I don't think this discussion is productive, we should probably discuss this on per case basis on PR/issue with user requests/implementation/description.

@mkalinin
Copy link

I'd prefer we have pagination by default on lists likely to be large (/validators) and have a sensible default, but allow users to pass in large page sizes if required.

Totally agree!

I'd argue that we do have identifiers for at least all the chain objects that we're dealing with. In most of the cases they are the roots of those objects. For instance, to identify the validator in a REST model we would use /beacon/state/{state_root}/validators/{validator_idx}. The committees however doesn't fit this kind of identification. That's why REST often turns into RESTful which takes REST as a foundation but deviates from it in places where it becomes cumbersome.

We should probably re-consider sticking with REST in our case and look at e.g. GraphQL (however it has its own drawbacks, like using single endpoint and only POST requests). But let's try to give REST a chance anyway. We won't fully utilize REST as most of our cases are covered by GET and POST (to query and to create), rarely DELETE, and we would hardly ever use PUT/PATCH.

REST advises to represent every object as a resource rather than a set of unstructured data obtained with multiple requests. The resource must be uniquely identified by its URL. If it's not the case that we're addressing then we have to use search with query string that narrows down our result set. IMO, REST is good to access chain data in most of the cases we need cause chain data has well defined solid structure where each object could be uniquely identified by URL if the URL is built well. But if the URL uniquely identifying the object can't be built or we want to get a list of the objects as a result query string must be used instead of attempt to abuse URL. A good example of it are committees which are identified by the state (seed), the slot number and the committee index. Slots and states are orthogonal and can't be sanely put into the same URL.

I agree with @mpetrunic that for simplicity we could stem from strict REST and use different identifiers where it's appropriate. For instance, /beacon/states/{state_id}/validators/{validator_idx} could use state_root, "final", "latest" as a parameter identifying particular state that validators are gonna be obtained from. Having /beacon/validator/validators/{idx} endpoint is ambiguous cause it doesn't have a notion of state.

@rolfyone
Copy link
Collaborator

I'm not sure if this is the right place for this comment, but on the eth2-api spreadsheet, it seems that the /blocks/{blockId} endpoint doesn't have the root at the top of the response, just the SignedBeaconBlock. It'd greatly assist if the root is there, it's there in most of the existing APIs and i've been asked to add it to teku to make it easier to debug, so i think omitting it from the standard might become a problem.

Also most responses wrap everything with data, but theres a couple of validator endpoints that don't, and we should probably be consistent in that regard.

@mpetrunic
Copy link
Collaborator

mpetrunic commented May 20, 2020

@rolfyone Probaly better in #37 or in spresheets itself.

As for your question, there is:

  • /v1/beacon/blocks/{blockId}/root endpoint to get root of block where blockId can be some alias or slot

Also most responses wrap everything with data, but theres a couple of validator endpoints that don't, and we should probably be consistent in that regard.

I think we concluded that array responses should be wrapped inside data key as we will probably need some metadata as pagination etc while it's better for single resource endpoints to return resource without data wrapper so it's easier to add "ssz" response of resource

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

Successfully merging a pull request may close this issue.

8 participants