This implementation ensures that all addresses have an equal chance of
being included in the addr message. It also moves the pseudorandom number
generator seeding to package level so that it can be overridden for
testing if required.
When a persistent peer is disconnected (for example due to a
network timeout), a connection retry is issued. The logic for
doing so failed to remove the peer from the peerState, causing
dead peer connections to fill the peerState. Since connections
in the peerState are counted towards the maxPeers limit, this
would cause btcd to eventually stop retrying connection.
This commit fixes the issue by properly removing the peer from
the peerState.
The getaddr msg is usually replied to with an addr msg, but if
the other peer does not have any addresses to share it will not
reply at all (instead of replying with an addr msg with 0 addresses).
Therefore, the getaddr msg is not guaranteed a reply, so this commit
removes it from stall detection to avoid incorrectly kicking
such peers.
In case of an error during protocol negotiation in handleVersionMsg,
immediately break out and prevent further processing of OnVersion
listener which generally depends upon peer attributes like NA to be set
during the negotiation. Fixes#579.
The CPU miner relies on the mempool, so the mempool has to be created
before calling the function to create the CPU miner. When PR #568
introduced the mempool config struct, it moved the mempool creation
after the miner creation, which leads to the CPU miner crashing due to
trying to access a nil mempool.
This move the CPU miner creation after the mempool creation
appropriately.
This creates a skeleton mining package that simply contains a few of the
definitions used by the mining and mempool code.
This is a step towards decoupling the mining code from the internals of
btcd and ultimately will house all of the code related to creating block
templates and CPU mining.
The main reason a skeleton package is being created before the full
blown package is ready is to avoid blocking mempool separation which
relies on these type definitions.
This introduces the concept of a new interface named TxSource which aims
to generically provide a concurrent safe source of transactions to be
considered for inclusion in a new block. This is a step towards
decoupling the mining code from the internals of btcd. Ultimately the
intent is to create a separate mining package.
The new TxSource interface relies on a new struct named miningTxDesc,
which describes each entry in the transaction source. Once this code is
refactored into a separate mining package, the mining prefix can simply
be dropped leaving the type exported as mining.TxDesc.
To go along with this, the existing TxDesc type in the mempool has been
renamed to mempoolTxDesc and changed to embed the new miningTxDesc type.
This allows the mempool to efficiently implement the MiningTxDescs
method needed to satisfy the TxSource interface.
This approach effectively separates the direct reliance of the mining
code on the mempool and its data structures. Even though the memory
pool will still be the default concrete implementation of the interface,
making it an interface offers much more flexibility in terms of testing
and even provides the potential to allow more than one source (perhaps
multiple independent relay networks, for example).
Finally, the memory pool and all of the mining code has been updated to
implement and use the new interface.
This does three things:
- Splits the priority calculation logic from the TxDesc type
- Modifies the calcPriority function to perform the value age
calculation instead of accepting it as a parameter
- Changes the starting priority to be calculated when the transaction is
added to the pool
The first is useful as it is a step towards decoupling the mining code
from the internals of the memory pool. Also, the concept of priority is
related to mining policy, so it makes more sense to have the
calculations separate than being defined on the memory pool tx
descriptor.
The second change has been made because everywhere that uses the
calcPriority function first has to calculate the value age anyways and
by making it part of the function it can be avoided altogether in
certain circumstances thereby provided a bit of optimization.
The third change ensure the starting priority is safe for reentrancy
which will be important once the mempool is split into a separate
package.
This fixes an issue introduced during the peer refactor where persistent
peers that failed the initial connection are not retried as intended.
It also improves the retry logic as follows:
- Make the retry handler goroutine simply use a for loop instead of
launching a new goroutine for each backoff attempt. Even though
goroutines are fairly cheap to create, it is much more efficient to
simply loop
- Change the retry handler to accept a flag if it is the initial attempt
- Rather than dividing the const interval by 2 everywhere and passing
the retry duration in, just half the constant and set the initial
duration to it in the retry handler
Finally, include the address of the peer in the error message when a new
outbound peer can't be created.
This introduces the concept of a mining policy struct which is used to
control block template generation instead of directly accessing the
config struct. This is a step toward decoupling the mining code from
the internals of btcd. Ultimately the intent is to create a separate
mining package.
Now that the memory pool minimum fee calculation code is also
calculating a more precise value instead of rounding up to the nearest
kilobyte boundary, the comment in NewBlockTemplate regarding this
behavior is no longer accurate. Thus, this removes the comment.
Also, while here, change the calculation to use an int64 instead of
float since it matches the precision of the new calculation code used by
the memory pool and can also avoid the need for the slower floating
point math.
When the peer code was refactored, the lists of peers were converted to
maps however the code which runs when a peer disconnects still iterates
them like a slice. This is no longer necessary since they are maps
which means the peer can simply be looked up by its ID.
Also, the old code was comparing the map entry and the peer being
removed by their pointers which could lead to potentially not properly
finding the peer. This issue is also resolved by this change since it
looks up the peer by its ID.
This defines the peer stat fields directly in the Peer struct instead of
defining them in a separate struct and embedding them. It is slightly
more efficient this way and there is really no reason to have them
defined separately anyways since they are not passed around or otherwise
used independently.
Don't log "Established connection" message on new when
DisableConnectOnNew is set and no connection was actually established.
Do log that same message when Connect() successfully connects.
Fixes a rare hang during peer tests due to the same connection being
used for different outbound peers. Also fixed passing a chan to
QueueMessage(..) which was not being waited on, so was not being used.
500 tests with various transactions and scripts, verifying that
calcSignatureHash generates the expected hash in each case.
This requires changing SigHashType to uint32; that won't affect the
standard use-cases, but will make calcSignatureHash behave more like the
Core counterpart for non-standard SigHashType settings, like those in
some of these tests.
This is more efficient and prevents duplicate entries which can
lead to no-longer-orphans being attempted be added to the mempool
multiple times.
Bug found by me, debugged with @davecgh, and patch from @davecgh
This refactors the checkTransactionStandard function, along with related
constants, from the mempool to the policy.go file since it is strictly
related to policy.
In addition, it adds tests for the function which cover all code paths.
ProcessTransaction could have accepted a new transaction into mempool
but could have returned a reject message if a no-longer-orphan
transaction failed to be accepted. This would also skip any
additional no-longer-orphans, keeping them in the orphan pool.
Instead of returning an error incorrectly, log the error and skip
the no-longer-orphan transaction. This allows the rest of the
no-longer-orphans to be processed as well.
This modifies the IP parsing code to work with IPv6 zone ids. This is
needed since the net.ParseIP function does not allow zone ids even
though net.Listen does.
