Instead of having both server and blockManager be aware of the
txProcessed and blockProcessed channels, now the server passed them as
method arguments to blockProcessor.
This refactors the RPC server to accept and take ownership of already
configured listeners and refactors the logic to setup those listeners to
the server. This mirrors the logic used by the connection manager and
is desirable since it is another step closer to being able to split the
RPC server code out into a separate package and will make it much easier
to internally test since it allows creating mock listeners.
This decouples the RPC server from the internal btcd server to move
closer to being able to split it out into a separate package.
In order to accomplish this, it introduces an rpcserverConfig type and
several new interfaces, named rpcserverPeer, rpcserverConnManager, and
rpcserverBlockManager, which are necessary to break the direct
dependencies on the main server and block manager instances.
It also adds concrete implementations of the new interfaces and uses
them to configure the RPC server.
Ultimately, the RPC server should ideally be decoupled even more such
that all of the types in the configuration struct use interfaces instead
of the concrete types. Doing this would make the RPC server much easier
to internally test since it would allow creating lightweight stubs for
the various pieces.
Instead of having the block manager notify the RPC server about
accepted, connected, and disconnected blocks, the RPC server will
directly listen for notifications from the blockchain.
The objective is to remove the reference from blockManager to
server. Since the blockManager is responsible for keeping the mempool
in sync, it should have a direct reference to it.
This commit modifies the existing block selection logic to limit
preferentially by weight instead of serialized block size, and also to
adhere to the new sig-op cost limits which are weighted according to
the witness discount.
This commit modifies the logic within the block manager and service to
preferentially fetch transactions and blocks which include witness data
from fully upgraded peers.
Once the initial version handshake has completed, the server now tracks
which of the connected peers are witness enabled (they advertise
SFNodeWitness). From then on, if a peer is witness enabled, then btcd
will always request full witness data when fetching
transactions/blocks.
Version 0.15.0 of Bitcoin Core will include a new RPC command that will
allow us to obtain the amount of time (in seconds) that the server has
been running.
This modifies the blockNode and BestState structs in the blockchain
package to store hashes directly instead of pointers to them and updates
callers to deal with the API change in the exported BestState struct.
In general, the preferred approach for hashes moving forward is to store
hash values in complex data structures, particularly those that will be
used for cache entries, and accept pointers to hashes in arguments to
functions.
Some of the reasoning behind making this change is:
- It is generally preferred to avoid storing pointers to data in cache
objects since doing so can easily lead to storing interior pointers
into other structs that then can't be GC'd
- Keeping the hash values directly in the block node provides better
cache locality
This modifies the block node structure to include a couple of extra
fields needed to be able to reconstruct the block header from a node,
and exposes a new function from chain to fetch the block headers which
takes advantage of the new functionality to reconstruct the headers from
memory when possible. Finally, it updates both the p2p and RPC servers
to make use of the new function.
This is useful since many of the block header fields need to be kept in
order to form the block index anyways and storing the extra fields means
the database does not have to be consulted when headers are requested if
the associated node is still in memory.
The following timings show representative performance gains as measured
from one system:
new: Time to fetch 100000 headers: 59ms
old: Time to fetch 100000 headers: 4783ms
A DNS lookup was being attempted on onion addresses causing
connections to fail. This has been fixed by introducing type
onionAddr (which implements a net.Addr interface) and passing
it to btcdDial.
Also, the following onion related fixes have been made:
* getaddednodeinfo - updated to handle onion addrs.
* TorLookupIP - fixed err being shadowed.
* newServer - rename tcpAddr to netAddr
* addrStringToNetAddr - skip if host is already an IP addr.
* addrStringToNetAddr - err if tor is disabled
* getaddednodeinfo - check if host is already an IP addr.
addrmgr.GetAddress() had a parameter `class string` originally intended
to support looking up addresses according to some type of filter such as
IPv4, IPv6, and only those which support specific wire.ServiceFlags
(full nodes, nodes that support bloom filters, nodes that support
segwit, etc). But currently the parameter is unused and also has an
inappropriate type `string`.
If it would ever be used, it's easy to add back and should then get an
appropriate type such as something that allows bitflags to be set so
that the caller could request combinations such as peers that support
IPv6, are full nodes, and support bloom filters.
This corrects an issue introduced by commit
e8f63bc295 where a failure to lookup a
hostname could lead to a panic in certain circumstances. An error is
now returned in that case as expected.
This commit modifies the `ConnManager` to use the `net.Add` interface
through the package instead of a plain string to represent and
manipulate addresses. This change makes the package much more general as
users of the package can possibly utilize custom implementations of the
`net.Addr` interface to establish connections.
More precisely, the `ConnReq` struct has been modified to use a net.Addr
instance explicitly, and the `DialFunc` type has also been modified to
take a `net.Addr` directly. This latter change gives functions that
adhere to the `DialFunc` type more flexibility as to exactly how the
connection is established.
Additionally, the `connmgr.Config.GetNewAddress` configuration option
now directly returns a `net.Addr. This change allows the `connmgr` to be
decoupled from all DNS queries which allows callers to preferentially
select more secure methods like performing DNS lookups over a Tor proxy.
This modifies the signatures of all serverPeer callbacks that are
provided as peer.Listeners to use _ for the first parameter name which
ensures the passed peer can't be used within the function and updates
all references to the server peer.
This helps ensure any overridden methods that might be defined on a
serverPeer will be invoked where directly calling methods on the passed
peer would not.
Also, while here, add a comment to the OnFeeFilter function.
This modifies the connection manager to provide support for accepting
inbound connections on a caller-provided set of listeners and notify the
caller via a callback.
This is only the minimum work necessary to get inbound support into the
connection manager. The intention for future commits is to move more
connection-related logic such as limiting the maximum number of overall
connections and banned peer tracking into the connection manager.
This removes the type definitions for the callback functions in favor of
declaring them directly in the Config struct. This is more consistent
with the rest of the code base and is preferred since it means callers
reviewing the documentation don't have to follow another level of
indirection to figure out the signature.
This removes any remaining orphan transactions that were sent by a peer
when it disconnects since it is extremely unlikely that the missing
parents will ever materialize from elsewhere.
This does the minimum work necessary to refactor the CPU miner code into
its own package. The idea is that separating this code into its own
package will improve its testability and ultimately be useful to other
parts of the codebase such as the various tests which currently
effectively have their own stripped-down versions of this code.
The API will certainly need some additional cleanup and changes to make
it more usable outside of the specific circumstances it was originally
designed to support (namely the generate RPC), however it is better to
do that in future commits in order to keep the changeset as small as
possible during this refactor.
Overview of the major changes:
- Create the new package
- Move cpuminer.go -> cpuminer/cpuminer.go
- Update mining logging to use the new cpuminer package logger
- Rename cpuminerConfig to Config (so it's now cpuminer.Config)
- Rename newCPUMiner to New (so it's now cpuminer.New)
- Update all references to the cpuminer to use the package
- Add a skeleton README.md
This corrects a few issues introduced with the connection manager where
the server was not notifying the connection manager when a connection
request is available again.
The cases resolved are:
- Unable to initialize a server peer instance in response to the connection
- Failure to associate the connection with the server peer instance
- Disconnection of a non-persistent outbound peer
It also changes the log message to a debug in the former case because
it's not something that should be shown to the user as an error given
it's not due to anything the user has misconfigured nor is it even
unexpected if an invalid address is provided.
This does the minimum work necessary to refactor the block template
generation code into the mining package. The idea is that separating
this code into the mining package will greatly improve its testability,
allow independent benchmarking and profiling, and open up some
interesting opportunities for future development related to mining.
There are some areas related to policy and other configuration that
could be further refactored, however it is better to do that in future
commits in order to keep the changeset as small as possible during this
refactor.
Overview of the major changes:
- Move mining.go -> mining/mining.go
- Move mining_test.go -> mining/mining_test.go
- Add logger to mining package
- Update the MINR subsystem to use the new mining package logger
- Export CoinbaseFlags from the mining package
- BlkTmplGenerator is now mining.BlkTmplGenerator
- Update all references to the mining code to use the package
This modifies the block template generate for the mining code such that
it takes chain instance and params instead of requiring a fully
initialized blockManager instance.
Also, in preparation for being able to more easily separate the code, it
exposes and makes use of two new functions:
- BestSnapshot which returns the state snapshot from the underlying
chain instance
- TxSource which returns the underlying transaction source
This is a step towards being able to separate the mining code into its
own package. No functional change.
This introduces a cpuminerConfig type which contains the necessary
information to break the direct dependency on the main server instance.
This change is a step towards being able to separate the cpu miner into
its own subpackage. No functional change.
This commit adds a new option to the mempool’s policy configuration
which determines which transaction versions should be accepted as
standard.
The default version set by the policy within the server is 2; this
allows accepting transactions which have version 2 enabled in order to
utilize the new sequence locks feature.
This introduces a new type named BlkTmplGenerator which encapsulates the
various state needed to generate block templates.
This is useful since it means code that needs to generate block
templates can simply accept the generator rather than needing access to
all of the additional state which in turn will ultimately make it easier
to split the mining code into its own package.
This renames the mempool.Config.RelayNonStd option to AcceptNonStd which
more accurately describes its behavior since the mempool was refactored
into a separate package.
The reasoning for this change is that the mempool is not responsible for
relaying transactions (nor should it be). Its job is to maintain a pool
of unmined transactions that are validated according to consensus and
policy configuration options which are then used to provide a source of
transactions that need to be mined.
Instead, it is the server that is responsible for relaying transactions.
While it is true that the current server code currently only relays txns
that were accepted to the mempool, this does not necessarily have to
be the case. It would be entirely possible (and perhaps even a good
idea as something do in the future), to separate the relay policy from
the mempool acceptance policy (and thus indirectly the mining policy).
