ethereum / trinity

The Trinity client for the Ethereum network
https://trinity.ethereum.org
MIT License
476 stars 145 forks source link

Refactor current node logic into plugins #38

Closed cburgdorf closed 3 years ago

cburgdorf commented 6 years ago

What is wrong?

Recently, someone on gitter asked if Trinity supports spinning up custom nets without touching the default code base. So, far that doesn't seem to be the case (maybe through subtile hacks idk). There seems to be a great opportunity to break up our existing node logic (full / light, mainnet / ropsten) into more fine grained building blocks to enable plugins to deliver such functionality. E.g. it would be nice if someone could just write a plugin to e.g. support the Kovan net (just using Kovan as an example, I have zero knowlege about any potential blockers with that).

How can it be fixed

Lots of exploration.

pipermerriam commented 6 years ago

Supporting other networks like Kovan/Rinkeby as well as custom nets (aka local development chains) is somewhat orthogonal to converting sync/node logic to be a plugin.

However, I am very supportive of us trying to dogfood the plugin APIs heavily and converting syncing to use them strikes me as a good way to do this. cc @gsalgado on the concept of making the syncing code a plugin.

pipermerriam commented 6 years ago

link: https://github.com/ethereum/py-evm/issues/979

gsalgado commented 6 years ago

I haven't looked into how you guys implemented the plugin mechanism, but to me it sounds weird to have the sync implemented as a plugin. I think of plugins as providing extra functionality that can be added/removed as one sees fit, but chain sync is one of the core components (you'd never want to run a node without a syncer, right?) and I believe it wouldn't make sense to run a node with more than one syncer.

cburgdorf commented 6 years ago

I'm not sure if the syncing logic itself needs to be plugin. I think it's rather the node types which utilize the syncing logic that can become plugins. So, if someone wants to write a plugin that brings in a --kovan flag that may still bring in its own syncing logic (and maybe leverage some libs from trinity / p2p to do so) but it would probably be able to make use of existing infrastructure such as the PeerPool, RPCServer etc.

After all, how exactly this would work is still very much up in the air but I think we should aim for enabling such use cases through plugins (as opposed to people just hacking the code base to their needs)

pipermerriam commented 6 years ago

@gsalgado I don't think @cburgdorf and I are necessarily on the same page with this (which isn't necessarily a bad thing). If our two ideas don't sound congruent that may be because they are not.

@cburgdorf your idea about --kovan is probably orthogonal to whether syncing is done as a plugin or not.

First, converting sync to be a plugin is probably very low priority, at least until we get the plugin architecture better figured out.

Second, reasoning behind wanting it to be done via the plugin system (if it turns out to be reasonable to do so) is fueled by a few things:

Another way to look at this might be to consider sharding and the beacon chain. There will be more than a single chain to sync at some point here, and being able to do all of those in a modular way sounds nice, especially if it means the research team can develop the plugin for casper/sharding independent of the development of trinity.

cburgdorf commented 6 years ago

First, converting sync to be a plugin is probably very low priority, at least until we get the plugin architecture better figured out.

Right, that's basically sums up my thinking. I'm not opposed to the idea of having syncing itself be a plugin and I basically agree with everything you wrote. It's just that I think the syncing itself can remain in non-plugin land far longer than the node logic itself.

Let's quickly recap, how we currently support Ropsten (and focus on full nodes just to lower the cognitive load)

Basically, we write a RopstenChain and subclass FullNode to a RopstenFullNode which sets that chain.

https://github.com/ethereum/py-evm/blob/e2d1c7ad629ad7679efa278c34a0987f407dcff8/trinity/nodes/ropsten.py#L1-L13

Then, we simply use that node type as part of the command line parsing

https://github.com/ethereum/py-evm/blob/e2d1c7ad629ad7679efa278c34a0987f407dcff8/trinity/config.py#L232-L233

Breaking this up to be governed through the plugin mechanism is very cheap even with the current very basic plugin architecture.

So, my main first goal (and the reason I created this issue for) would be to basically just make full nodes and light nodes plugins which expect full or light chains as resources to be available to work with. The chains itself would be provided by plugins as well.

That kind of mechanism allows anyone to write a Kovan plugin that provides such chains and which hooks into the existing node mechanism. Of course, the Kovan plugin would still need to provide the chain(s) and depending on how different the underlying principles of these chains are it can reuse the existing Py-EVM/P2P libs to do so, but at least it becomes very simple to hook in another network without touching the core code base.

This is basically something that can be tackled in a very short term. I'm happy to send a PoC PR for that soon but there are a couple of things that I'd like to tackle before and one of it would be integration tests for Trinity because I don't want these efforts to gamble with the stability of the existing features.

pipermerriam commented 6 years ago

I'm happy to send a PoC PR for that soon

I think this would be the most illustrative way to evaluate this plan.