p2p: Some improvements to PeerPoolSubscriber

[gsalgado]

• Renamed to PeerSubscriber because that's what it really is
• New @AbstractMethod max_queue_size, to force each subscriber to define their own
• Peer now keeps a list of actual PeerSubscribers instead of just their msg queues
• PeerSubscriber provides APIs to add msgs to queue and check queue
  size, abstracting underlying implementation
• If an attempt is made to add a msg but the queue is full, we log a
  warning and drop the msg (Closes: Unhandled QueueFull exception #1032)

This should also help debug #1052 and decide how to fix it

[pipermerriam]

I've been a fan of the following pattern.

class Thing:
    @abstractmethod
    def get_value(self):
        ...

    @property
    def value(self):
        return self.get_value()

It allows for slightly less verbose subclassing and still gives all the niceties of having property accessors. I believe this is the pattern we're using for properties on our Transaction classes.

[carver]

slightly less verbose subclassing

Meaning the subclass doesn't have to add the @property decorator?

I've been a fan of the following pattern.

I'm not fan. I found it sort of confusing when reading the Transaction subclasses, because we define get_intrinsic_gas in the subclass, but then callers would just use intrinsic_gas. Paranoid me feels the need to go back and make sure that the superclass defines it to be exactly the same, and make sure that one of the three (caller, subclass, superclass) doesn't just have a bug. If they are exactly the same, then why have both options?

[pipermerriam]

That is fair. I'd consider those concerns reason enough to abandon the pattern.