Implement interfaces and abstract classes

[faustbrian]

At the moment we have classes that are named like ConnectionInterface, using an Interface suffix to indicate that they have implementation expectations.

Those classes are actually abstract classes and should be refactored to behave like one. There are a few exceptions which are or should actually be interfaces that are implemented by other classes instead of extending an abstract class.

[paroxysm]

@faustbrian @kristjank @vasild
I've hit a road-block with the ts typedefs PRs because going further would introduce circular dependencies. We have some awkward code where this happens.
i.e. core-blockchain references to core-transaction-pool & core-p2p in the methods transactionPool and p2p and some other places, but core-p2p & core-transaction-pool also reference the Blockchain type in core-blockchain.
core-test-utils has code that references core-blockchain types but it is itself a dependency of core-blockchain.
In order to proceed, there's TWO approaches we can implement.

1. We can re-visit and restructure these dependencies. i.e. Why does core-utils reference core-blockchain types when core-blockchain has core-utils as a dependency? We should move the code in core-utils that references core-blockchain into core-blockchain.
   i.e. core-blockchain has a devDep on core-p2p because of its tests and even references core-p2p's Peer class. Yet core-p2p also references core-blockchain types. So why is core-blockchain testing components in core-p2p?
   One way we can solve this dependency problem is leveraging core-event-emitter. Whenever a module, like core-p2p needs to invoke a method on core-blockchain, it would fire an event that core-blockchain is listening for. However, this only works if it's a one-way data exchange. The emitter can embed data in the event object, but the listeners cannot embed data to return back to the emitter.

2. We start writing interfaces for all the concrete/abstract classes we have in our modules and package in ONE new module, i.e. core-interface-types or core-interfaces. We'll use typescript's namespaces to define boundaries within this new module.
   This new module will then be a dependency to the existing modules that will be implementing those interfaces. i.e. add a export interface Blockchain {... } in core-interfaces module then later in core-blockchain it provides and registers a concrete implementation export class BlockchainImpl implements Blockchain { ...}. So that way, any class that needs the Blockchain type, will only need a dependency on core-interfaces module and only interact with the interface. The actual concrete implementation is only known to the module providing it and leveraging DI we get with core-container, we still get to expose the concrete implementation.

I'm for approach 2 because it has the least friction. The tests in the modules already reference, what would be,the interface names; when writing concrete implementations, we can adopt the naming pattern <InterfaceName>Impl i.e. class BlockchainImpl extends Blockchain or core-p2ps class MonitorImpl extends Monitor . In addition, changing the source of the interface types from the old individual modules to the new module is a simple find-n-replace command away.
Before I proceed further with the other ts types PRs, I need this roadblock solved(not entirely true, I can proceed with crypto typedefs 👍 )

[faustbrian]

The least intrusive one for now is approach 2 but there is a far bigger issue with how types are currently done, which is why I in your initial types PR said it is OK for now but will need changes later on.

With how types are currently introduced and used all packages have to know about each others existents and exactly know which package provides the concrete implementations. The whole point of how packages have been structured so far is that this doesn't have to be the case, they only need to have the core-container package as a dependency and then resolve everything from there without the need of having another 20 dependencies for the packages that provide the concrete implementations.

Packages are bound to the container so plugins don't have any use for each other anyways as the instances that are useful to them are created in the register method that is called by the container. Exceptions to that rule are obviously the database packages as the postgres package extends the database package which provides all the base functionality that is required.

These kinda structural issues need to be addressed first as we otherwise defeat the purpose of the plugin system, which is to easily swap out components without having to spend days replacing code in another 20 packages.

The general rule is that plugins should not require anything but core-container and helper packages like utilities.

[paroxysm]

True, with pure JS, only having a dependency on core-blockchain works but, with Typescript that cannot work because you'll have to import types from some module/package, if you want type-safety. Usually plugins depend on an already established set of types that serve as the base (like DLLs in C++/C#). While the components are structured in a plug-and-play nature, there are still clear-cut dependencies required to make the thing run. Right now, we have the dependencies as implicit(code fails at runtime, as opposed to compile-time when a certain module we depend on isn't registered). So regardless, you're still adding additional dependencies to benefit from type-safety, so the general-rule can only hold for new optional plugins, not core ones.

The alternative approach which you initially suggested @faustbrian , would be to create a "types" module alongside each module. While this is the de-facto way of supplying types for modules migrated to TS, the additional overhead of maintaining each one separately, creating new build/CI configs(ableit one-off), and having to go through NPM is sort of a pain in the ass. Ideally, we could leverage a build system that can be able to package up the .d.ts that TS generates for us as a dependency we can reference but i'm sure even that will run into problems(such as establishing a build order). And unfortunately, the approach I initially opted for had its limitations which I've just hit.

The single interfaces package with namespaces was my suggestion for overcoming the circular dependency issue and the NPM & additional configs overhead in one solution. While it does have its drawbacks,

1. Creating a single monolithic package to contain all the integrating types, any new plugin will have to introduce its concerns into this package as well.
2. Package has to be updated anytime we need to change the interface(which isn't too bad since they're in one location)
3. Still have to introduce new dependency other than core-blockchain

To address point 1 & 2, we set the precedent that, if a given module is not a core module(meaning, it's optional, or given that module is absent the code still runs fine) then don't introduce your types into the core-interfaces module. So ONLY core modules will have their types declared in core-interfaces, everything else will have to provide their own

[faustbrian]

I don't think there is that much more overhead for maintaining type packages as they should rarely change in the future and if they do it won't be anything major. Versioning and publishing them is also a negligible amount of work as that will be automated.

A plugin system rework is scheduled for 3.0 as I will be introducing a lot of breaking changes and resolve a lot of issues that were caused by how JavaScript works and enforce stricter contracts that need to be satisfied by plugins.

I am not home until the 1st of January but when I am back I will review a few things and look at what approach would be the simplest for now. If you want you can work on the interfaces solution and I will review it when I am back but no guarantee that it will get merged.

[paroxysm]

Fair enough, my concern w/ the types package is that you're essentially forcing all changes(whether permanent or temp) to go through npm. From a development standpoint, this degrades the experience as modules cannot see any local changes to the types w/o having to first publish them to npm.
However, if they end up being managed by lerna as a mono-repo package then that won't be an issue 👍

[faustbrian]

They would be managed in the same repository. I would restructure the repository to contain 3 folders that are managed by lerna as essentials, optional and types.

• essentials would be packages that are required to run a node like blockchain, pool, etc.
• optional would be packages like api, webhooks, etc.
• types would be packages that contain only the type defs

[paroxysm]

So, I thought about it some more, and even in their own types modules, you'd still have the circular dependency problem if your interfaces need to reference each other(method parameters or instance properties). If they're all in a single module, they can all see each other so it's all good, but if they're in different modules, the types modules would have to have dependencies on each other and you run into the same problem, introducing a circular dependency among the types modules.

The only way that would work is if the dependencies are realized(so app.resolvePlugin<ImportedType>) in the concrete implementation, not on the interface layer, i.e interface Blockchain { get transactionPool() : TransactionPool }. But that isn't a good approach because it restricts how you write your interfaces(to make sure you don't reference a type what would result in a circular dependency)