This allows us to use a shared pointer while at the same time not having
the problem that a peer de-registration hits an already deleted
PrivacySegment.
Last year we started wrapping the Peer object in a shared pointer,
which is used now in the managers.
This continues the idea by making the PrivacySegment use smart
pointers too and the P2PNetInterface is changed to do the same
for downstream applications.
As we moved most of the creation of a BufferPool to be via the
Streaming::pool() method, which uses a thread-local, it makes sense
to start cleaning up the design and make it more modern C++.
The above mentioned method would return a reference and you'd see
loads of places use `auto &pool =` which is less than ideal.
As the number of places where we actually instantiate a BufferPool
goes down, the usage of some sort of smart pointer makes more sense.
This now makes all APIs use BufferPool be wrapped in a shared_ptr.
Every peer that we connect to should get the bloom filter set and it
should also get the 'mempool' call sent once which will make that peer
respond with all matches of the bloom filters which are in the mempool.
The tricky part is that we should have the latest bloom filter set on
each of those peers before the mempool is sent, since they work
together.
Also if we are behind, the mempool should not be sent. (It can cause
problems in the wallet if we receive unconfirmed transactions before
mined transactions)
So, when a peer starts downloading merkle blocks, the bloom filter of all
the other peers becomes invalid the moment a match is found by an actual
wallet.
This is Ok on one peer because the merkleblock automatically updates the
filter on match on the server side, but obviously not on the other peers
we have for that wallet.
The approach we follow is that as soon as a sync-run is done on a single
peer (we do a main and also a backup sync), we tell the wallet to re-
build its bloom filter for _all_ peers and if the sync that peer did
leads us to be at the chain-tip, we also send each peer the mempool
command.
When the SyncChainAction was written, various interactions we do
today in the ConnectionManager did not happen yet. Features in
Peer didn't exist yet.
This updates the SyncChainAction to take those items into account
and be more responsive and conclue we are 'up to date' faster,
while also leaving behind a better state.
Tuesdays idea of adding some code into the SyncSPVAction didn't feel
right.
A second look made clear that bloom filter updates make much more sense
to go hand in hand with sending a mempool message. Especially since they
depend on each other on the server side.
To-rehash:
the wallet may decide at any time that a new bloom filter is needed. It
then uses the superclass (code in p2plib) PrivacySegment, to build that
filter. As part of that we get a lock object which, when going out of
scope, makes the peers that are subscribed to the privacySegment send
out the filter.
This separation of concerns means that the subclass wallet in the app
doens't know about peers or messages, only its superclass PrivacySegment
does.
What we did in this change is make the PrivacySegment class decide to
combine a bloom update with a mempool call. Typicall only once per
connection.
This means I can remove hacks in the SyncSPVAction which forced the
sending of the mempool message separately.
We connect to the "first" priority stated segments first, in order to
allow the UX to be made much better since it may take a bit of time to
find peers.
The uint256 and CKeyID classes are the same baseclass with template
differences only, which makes them fragile to use for overloading.
As such rename the convenience overload slightly.
this adds a listener interface and a way to emit the callbacks on
changing of the filter.
This also adds a mutex since we expect the Peer to use the filter which
will likely live in its own thread.
This makes the class thread safe and re-entrant.
Notice that we use a recursive mutex to allow various usecase on
altering the bloom filter.
The most involved one is a complete replacement which calls clear and
then various calls to add() style methods.
Second is a single 'add' which can be done without the clear first.
The second needs an explicit lock in the add() methods, which would
deadlock in the first usecase if I didn't pick the recursive mutex.
Instead of forwarding one transaction at a time as the peer sends them
to us, bundle them in a group of transactions known to be merkle-checked
and all belonging together in one block.
Since the peer has no obligation (and with CTOR even less) to send the
transactions in natural order, we should get them per block so we know
all transactions forwarded have parents.
We reuse the NetworkManager lower level code in order to connect
to the Bitcoin P2P network.
This implements the basics for anyone wanting to be a player on
this network.
tomFlowee
TomZ