search

spacemeshos / go-spacemesh

Go Implementation of the Spacemesh protocol full node. 💾⏰💪
https://spacemesh.io
MIT License
767 stars 214 forks source link

Epic: API Refactoring #1764

Closed avive closed 3 years ago

avive commented 4 years ago

For background on motivation and design, see https://github.com/spacemeshos/SMIPS/issues/21 and https://github.com/spacemeshos/api.

Phases

Phase 1: E2E proof of concept (#2041)

implement and test single Echo endpoint in NodeService (totally independent of existing grpc server)

Phase 2: CLI support (#2043)

add CLI flags and allow new NodeService to be switched on

Phase 3: grpc-gateway proof of concept (#2053)

implement and test single Echo endpoint in NodeService using grpc-gateway server (totally independent of existing grpc-gateway server)

Phase 4: multiplexing grpc (#2054)

implement a second new GRPC service and multiplex it onto the same server/port as NodeService

Phase 5: multiplexing grpc-gateway (#2055)

do the same for the grpc-gateway server

Phase 6: NodeService (#2056)

fully implement this service, excluding stream endpoints

Phase 7: second grpc service (#2059)

implement a second service, MeshService, and test

Phase 8: MeshService streams (#2061)

Phase 9: finish implementing grpc services (#2071)

Phase 10: docs (#2083)

document everything!

Phase 11: cleanup (#2131)

remove/deprecate old API, grpc service, grpc-gateway service, and all protobuf build code. Note: most of this work was already done in #2036 and can be carried over from that branch.

Phase 12: missing stuff

fill in the endpoints/data elements that weren't previously implemented because of functionality missing in go-spacemesh

Phase 13: optimization

replace naive implementations of data queries with optimized implementations - may require adding DB indices in some cases

Misc

noamnelke commented 4 years ago

This can be done with no code changes to the node if the SMApp uses the REST endpoints instead of gRPC.

The node currently exposes both a gRPC interface and an HTTP/JSON interface for all endpoints. If the SMApp starts using the HTTP interface, we can use a public API gateway that only exposes a subset of endpoints and redirects requests from those endpoints to a set of backing nodes using round-robin (or similar load balancing mechanism).

I think this makes a better architecture than nodes maintaining a persistent gRPC session with remote, user-controlled wallets.

We're currently in the process of doing the same transition for the same reason for the PoET service: #1716

avive commented 4 years ago

yes, this makes sense architecturally.

avive commented 4 years ago

@ilans - this is what I mentioned in our talk about the API

avive commented 4 years ago

Consider adding pagination to all methods which return a list of data items. @IlyaVi

avive commented 4 years ago

I believe we should refactor the API into categories / facades in the node level instead of just having a flat list of methods. This can be as simple as adding the facade name after v1 in methods rest urls. e.g. /v1/mesh/method1...

Additional things to consider:

  1. Adding pub/sub support for server-side streaming data - at least for GRPC over http2. Some GRPC clients support server-side streaming of events. Dashboard, explorer and wallet can use this over GRPC.

  2. Add flags to disable / enable only some API categories/facades - e.g. add a flag not to open node management features via the API while still providing other API categories.

@lrettig

avive commented 4 years ago

Inventory of API clients:

  1. App / Wallet.
  2. Dashboard backend.
  3. Explorer backend.
  4. Backup agent (backup testnet data in a restorable way).
  5. Marketing programs reports. e.g. rewards program top 100.
avive commented 4 years ago

@lrettig @YaronWittenstein - We need to specify the API category for smart wallets. e.g. A method to read data from an app instance.

lrettig commented 4 years ago

Adding pub/sub support for server-side streaming data - at least for GRPC over http2. Some GRPC clients support server-side streaming of events. Dashboard, explorer and wallet can use this over GRPC.

I wasn't aware that you can do streaming over GRPC. That's exciting to hear. I'll look into it. I think streaming is important but I'm not married to websockets.

Add flags to disable / enable only some API categories/facades - e.g. add a flag not to open node management features via the API while still providing other API categories.

Agree. For comparison, this is how geth works:

--rpcapi API's offered over the HTTP-RPC interface (default: eth,net,web3)
--wsapi API's offered over the WS-RPC interface (default: eth,net,web3)
--ipcapi API's offered over the IPC-RPC interface (default: admin,debug,eth,miner,net,personal,shh,txpool,web3)
lrettig commented 4 years ago

@lrettig @YaronWittenstein - We need to specify the API category for smart wallets. e.g. A method to read data from an app instance.

I don't think the smart wallet, or any dapp, needs to use a special API. It's just transactions, right? Or in the case of statically reading data, it's like a "quasi" transaction. Basically we need to start thinking about what a Spacemesh web3.js would look like.

YaronWittenstein commented 4 years ago

@lrettig @YaronWittenstein - We need to specify the API category for smart wallets. e.g. A method to read data from an app instance.

I don't think the smart wallet, or any dapp, needs to use a special API. It's just transactions, right? Or in the case of statically reading data, it's like a "quasi" transaction. Basically we need to start thinking about what a Spacemesh web3.js would look like.

I'll soon start working on App's Storage ABI. The consequent tasks should be exposing gRPC / HTTP interface over the Full-Node. (which will propagate requests to the upcoming go-svm client)

avive commented 4 years ago

@lrettig @YaronWittenstein - We need to specify the API category for smart wallets. e.g. A method to read data from an app instance.

I don't think the smart wallet, or any dapp, needs to use a special API. It's just transactions, right? Or in the case of statically reading data, it's like a "quasi" transaction. Basically we need to start thinking about what a Spacemesh web3.js would look like.

yes - this epic is what is the sm.js looks like...

regarding smart wallet - the part in the api which is not txs that we need to specify is the method(s) to read wallet storage data. For example, to populate a smart wallet ui in the app...

lrettig commented 4 years ago

From Discord:

For now i see that mem pool overflow affect some GRPC requests. v1/accounttxs for example, because mem pool > than 4 MB

image

lrettig commented 4 years ago

regarding smart wallet - the part in the api which is not txs that we need to specify is the method(s) to read wallet storage data. For example, to populate a smart wallet ui in the app...

You just write a "constant" method in the smart contract and call it like this:

https://web3js.readthedocs.io/en/v1.2.6/web3-eth-contract.html#methods-mymethod-call

avive commented 4 years ago

I don't think this is yaron's current design for read-only data in svm... I think he wants to have an explicit api for this use case... @YaronWittenstein

YaronWittenstein commented 4 years ago

@avive @lrettig

The current App Storage infrastructure has no notion of permissions. My plan was to add such constraints on top - when we'll get to implementing the SMESH language.

lrettig commented 4 years ago

Agree with Yaron. You can't have permissions on state data. Every node needs to be able to read all of the state to validate transactions - unless you want to talk about doing something super complicated using zero knowledge proofs, and we're obviously not going that route anytime soon!

Can totally add that on top using encryption of course :)

lrettig commented 3 years ago

Everything is done except some documentation and some optimization. There are issues to track all of these and we'll follow up on them separately.