Update devp2p API methods

[acolytec3]

We should upgrade devp2ps' API to match changes done in the other packages

• Change _common to common on any/all classes exposed by Devp2p (notably RLPx)
• Change any classes that extend the EventEmitter class to have an internal events property instead so the main class API isn't cluttered up with EventEmitter properties

[holgerd77]

One note: this is very well breaking though, so this should be done relatively imminently after RC1 so that we can get it into the final releases.

[holgerd77]

Hi Amir,
so this is the respective issue. 🙂

So to expand a bit on the initial description: the _common to common change (rename) follows the example from #2857 and should be pretty straight forward I guess.

Update: I am now looking a bit into this. For devp2p we have actually no external accesses of _common at all. So I would think it makes sense to switch here and keep this as _common for now, no necessity to artificially expose when not needed.

I would in turn add the following to the issue: in devp2p we have all these _ properties, like e.g. _privateKey: Uint8Array. Also following #2857 these should be made protected (so somewhat the same as private, but protected gives more flexibility on subclassing and the like).

This can very likely not be done in just one go but will likely need some one-by-one approach (or 2-4 at once or so) and then re-running tests (or better: CI with client) re-occuringly, otherwise one runs the risk to "guard" too many properties at once and have things breaking on various fronts since properties are accessed anyhow.

If properties are only accessed by tests please do not make public but rather access by any-cast or something (there was this alternative approach doing e.g. rlpx['_privateKey'] which circumvents the protected keyword and keeps the typing, so maybe even better).

For the EventEmitter thing (might make sense to do in a separte PR now with the above task expanded):

So there is also an example in #2857 as well on this.

For devp2p there are seven classes which at the moment inherit from EventEmitter directly, so e.g. export class RLPx extends EventEmitter.

These classes should instead get an additional (public) property events which instantiates an EventEmitter and the events should be listened to from this property instead of the class object itself.

So rlpx.on('event', ...) changes to rlpx.events.on('event', ...).

Likely also good to do a one-by-one approach here, there is a lot of event-listening in the client and there things might break otherwise. Or minimally client tests should be run every now and then I guess.