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

Feature/openrpc #3

Merged
merged 18 commits into from Mar 12, 2019

Conversation

@shanejonas
Copy link
Member

shanejonas commented Feb 27, 2019

resolves #4

I've made this proposal pretty generic.

The reasons for this are:

  • forces us to talk generally about the problems and advantages
  • to be able to make proposals to other projects easily
Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated

@shanejonas shanejonas requested a review from sorpaas Feb 27, 2019

Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated

@shanejonas shanejonas force-pushed the shanejonas:feature/openrpc branch from d295630 to 5ca2649 Feb 27, 2019

Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
@BelfordZ

This comment has been minimized.

Copy link
Member

BelfordZ commented Feb 27, 2019

giphy 2

Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
@meowsbits

This comment has been minimized.

Copy link
Member

meowsbits commented Feb 27, 2019

This seems simple and strong.

I'm now wondering if it might be still a little broad. idk, thinking on it.

in the meantime, I would suggest to add a little background on existing common API points or patterns (like Web3, eth_) to give some context to what a proposal to "add an(other) endpoint to all the APIs" kind of means.

Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
# Add OpenRPC Service Discovery

ECLIP: 2
Title: Add OpenRPC Service Discovery

This comment has been minimized.

@zmitton

zmitton Feb 27, 2019

...to multi-geth specifically? or to multigeth/geth/parity

This comment has been minimized.

@shanejonas

shanejonas Feb 27, 2019

Author Member

trying to keep it pretty generic so it could be proposed anywhere, even as say a BIP

This comment has been minimized.

@whilei

whilei Feb 27, 2019

Contributor

or for anything that has a JSONRPC server, eh?

This comment has been minimized.

@shanejonas

shanejonas Feb 28, 2019

Author Member

yea exactly

@phyro

This comment has been minimized.

Copy link

phyro commented Feb 27, 2019

Blockchains probably share a lot of common functionality e.g. getBlock(height), getTransaction(txHash) etc. It is possible to do the openrpc.json thing for each one separately and that's already a huge win, but if they all agree on the same Blockchain functions spec, it would make it easier to for example build a generalized chain explorer (actually just a part of it because you'd need to fill the gas thing). This could be shooting in the dark, but perhaps it could also make it simpler to add coins to the exchanges if they follow the same interface description? If this makes sense, then in this case ETC openrpc.json would extend the blockchain openrpc document which could be described in the metadata.
I'm not even sure this is related to this ECLIP, but it might make sense just to put 10 minutes into thinking about this if it's something there. It could be done afterwards as some kind of a proxy rpc mapping to the actual implementation though, but that's more messy.

@phyro phyro referenced this pull request Feb 27, 2019

Open

ECIP-1000: ECIP Process #58

@BelfordZ

This comment has been minimized.

Copy link
Member

BelfordZ commented Feb 27, 2019

@phyro should work that into a suggestion on this ECLIP

meowsbits and others added some commits Feb 27, 2019

fix: suggestion around extending across use cases
Co-Authored-By: shanejonas <jonas.shane@gmail.com>
fix(ECLIP): suggestion grammar on why do this
Co-Authored-By: shanejonas <jonas.shane@gmail.com>
fix(ECLIP2): simplify alternative
Co-Authored-By: shanejonas <jonas.shane@gmail.com>
@r8d8

This comment has been minimized.

Copy link

r8d8 commented Feb 28, 2019

This seems simple and strong.

I'm now wondering if it might be still a little broad. idk, thinking on it.

in the meantime, I would suggest to add a little background on existing common API points or patterns (like Web3, eth_) to give some context to what a proposal to "add an(other) endpoint to all the APIs" kind of means.

I thought OpenRPC made to be single tool for API creation, and aiming to remove all custom specs, docs, etc. for API in clients, or?

BelfordZ added a commit to etclabscore/ethereum.go-ethereum that referenced this pull request Mar 1, 2019

@meowsbits

This comment has been minimized.

Copy link
Member

meowsbits commented Mar 1, 2019

I just mean like a little more context in the document.

Like Add OpenRPC Service Discovery -> Add OpenRPC Service Discovery To JSONRPC Clients.

This is a proposal to add OpenRPC support by adding the method rpc.discover to the projects JSON-RPC API to enable automation and tooling.

-> This is a proposal to add OpenRPC support to existing and future Ethereum-familly RPC services by adding the method rpc.discover to these projects's JSON-RPC APIs, enabling automation and tooling.

edit suggestions suggested

Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated
Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated

BelfordZ referenced this pull request in BelfordZ/starIPs Mar 2, 2019

Merge pull request #3 from etclabscore/fix/typo-n-compose
fix: typo in CONTRIBUTING.md and compose command in BUILDING.md

BelfordZ referenced this pull request in BelfordZ/starIPs Mar 2, 2019

Merge pull request #3 from etclabscore/feature/add-doc-video
fix: add video guide for making small documentation changes
- Mock Server generated in many languages
- Tests generated in many languages
- Documentation Generation

This comment has been minimized.

@phyro

phyro Mar 3, 2019

Suggested change
### Possible benefits of standardized blockchain methods
Since Blockchains share a lot of common functionality, it would make sense for them to expose the same set of RPC methods that are common to all blockchains e.g. `getBlock(height)` or `getTransaction(txHash)`. The reuse of such blockchain methods could make certain tasks easier for services that need to integrate with multiple blockchains. Examples of services that could benefit from this include, but are not limited to:
- blockchain explorer for multiple chains
- exchanges listing new coins

This comment has been minimized.

@shanejonas

shanejonas Mar 12, 2019

Author Member

I think this is awesome. but maybe is another Improvement Proposal around BTC/ETC and other chains coming together on a good naming scheme for methods for these APIs?

Or I have other ideas around doing a meta-API t hat would allow this but its out of the scope of this ECLIP.

This comment has been minimized.

@phyro

phyro Mar 12, 2019

@shanejonas agree, this seems more like something on top of this proposal

Show resolved Hide resolved ECLIPs/ECLIP-2.md Outdated

shanejonas and others added some commits Mar 12, 2019

fix: update title in ECLIPs/ECLIP-2.md
Co-Authored-By: shanejonas <jonas.shane@gmail.com>
fix: update title in ECLIPs/ECLIP-2.md
Co-Authored-By: shanejonas <jonas.shane@gmail.com>
fix: update automation text around ECLIPs/ECLIP-2.md
Co-Authored-By: shanejonas <jonas.shane@gmail.com>
@stevanlohja
Copy link
Member

stevanlohja left a comment

LGTM

@stevanlohja stevanlohja merged commit 7688e69 into etclabscore:master Mar 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.