In this commit, we modify the fee estimation to use vsize as a base
rather than size. A recent commit landed to track the fee rate using
vsize in the mempool, and also correct some incorrect unit math. This is
a follow up to that commit to ensure that fee estimation is uniform
throughout.
This commit changes the value of bytesPerKb to 1000 from 1024.
This is done to ensure consistency between the fee estimator
and the mempool, where the feeRate is set to
fee * 1000 / serializedSize
new txs that it observes. The block manager alerts the fee estimator
of new and orphaned blocks.
Check for invalid state and recreate FeeEstimator if necessary.
Rollback takes a block hash rather than a BlockStamp.
Increase rounds in TestEstimateFeeRollback to test dropping txs that have been in the mempool too long.
This removes the standardness check to reject transactions with a lock
time greater than a maxint32 because the old bitcoind nodes which it was
designed to protect against are no longer valid for other reasons and
thus there are no longer any of them on the network to worry about.
This replaces the ErrDoubleSpend and ErrMissingTx error codes with a
single error code named ErrMissingTxOut and updates the relevant errors
and expected test results accordingly.
Once upon a time, the code relied on a transaction index, so it was able
to definitively differentiate between a transaction output that
legitimately did not exist and one that had already been spent.
However, since the code now uses a pruned utxoset, it is no longer
possible to reliably differentiate since once all outputs of a
transaction are spent, it is removed from the utxoset completely.
Consequently, a missing transaction could be either because the
transaction never existed or because it is fully spent.
This commit implements the new “weight” metric introduced as part of
the segwit soft-fork. Post-fork activation, rather than limiting the
size of blocks and transactions based purely on serialized size, a new
metric “weight” will instead be used as a way to more accurately
reflect the costs of a tx/block on the system. With blocks constrained
by weight, the maximum block-size increases to ~4MB.
The github markdown interpreter has been changed such that it no longer
allows spaces in between the brackets and parenthesis of links and now
requires a newline in between anchors and other formatting. This
updates all of the markdown files accordingly.
While here, it also corrects a couple of inconsistencies in some of the
README.md files.
The current max orphan transaction size causes problems with dependent
transaction relay due to its artificially small size in relation to the
max standard transaction size.
Consequently, this modifies the orphan transaction policy by increasing
the max size of each orphan to the same value allowed for standard
non-orphan transactions and reducing the default max allowed number of
orphans to 100.
From a memory usage standpoint, the worst case max mem usage prior to
this change was 5MB plus structure and tracking overhead (1000 max
orphans * 5KB max each). With this, that is raised to 10MB (100 max
orphans * 100KB max each) in the worst case.
It is important to note that the values were originally implemented as a
naive means to control the size of the orphan pool before several of the
recent enhancements which more aggressively remove orphans from the
orphan pool were added, so they needed to be evaluated again.
For a very long time prior to recent changes, the orphan pool would
quickly reach the max allowed worst-case usage and effectively stay
there forever whereas with more recent changes, the actual run-time
orphan pool usage is usually much smaller.
Finally, as another point in favor of this change, as the network has
evolved, nodes have generally become better about orphan management and
as such missing ancestors will typically either be broadcast or mined
fairly quickly resulting in fewer overall orphans.
This allows a caller-provided tag to be associated with orphan
transactions. This is useful since the caller can use the tag for
purposes such as keeping track of which peers orphans were first seen
from.
Also, since a parameter is required now anyways, it associates the peer
ID with processed transactions from remote peers.
This modifies the NewMsgTx function to accept the transaction version as
a parameter and updates all callers.
The reason for this change is so the transaction version can be bumped
in wire without breaking existing tests and to provide the caller with
the flexibility to create the specific transaction version they desire.
This move the export for MinHighPriority from the mempool package to the
mining package. This should have been done when the priority
calculation code was moved to the mining package.
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 moves the priority-related code from the mempool package to the
mining package and also exports a new constant named UnminedHeight which
takes the place of the old unexported mempoolHeight.
Even though the mempool makes use of the priority code to make decisions
about what it will accept, priority really has to do with mining since
it influences which transactions will end up into a block. This change
also has the side effect of being a step towards enabling separation of
the mining code into its own package which, as previously mentioned,
needs access to the priority calculation code as well.
Finally, the mempoolHeight variable was poorly named since what it
really represents is a transaction that has not been mined into a block
yet. Renaming the variable to more accurately reflect its purpose makes
it clear that it belongs in the mining package which also needs the
definition now as well since the priority calculation code relies on it.
This will also benefit an outstanding PR which needs access to the same
value.
This implements orphan expiration in the mempool such that any orphans
that have not had their ancestors materialize within 15 minutes of their
initial arrival time will be evicted which in turn will remove any other
orphans that attempted to redeem it.
In order to perform the evictions with reasonable efficiency, an
opportunistic scan interval of 5 minutes is used. That is to say that
there is not a hard deadline on the scan interval and instead it runs
when a new orphan is added to the pool if enough time has passed.
The following is an example of running this code against the main
network for around 30 minutes:
23:05:34 2016-10-24 [DBG] TXMP: Expired 3 orphans (remaining: 254)
23:10:38 2016-10-24 [DBG] TXMP: Expired 112 orphans (remaining: 231)
23:15:43 2016-10-24 [DBG] TXMP: Expired 95 orphans (remaining: 206)
23:20:44 2016-10-24 [DBG] TXMP: Expired 90 orphans (remaining: 191)
23:25:51 2016-10-24 [DBG] TXMP: Expired 71 orphans (remaining: 191)
23:30:55 2016-10-24 [DBG] TXMP: Expired 70 orphans (remaining: 105)
23:36:19 2016-10-24 [DBG] TXMP: Expired 55 orphans (remaining: 107)
As can be seen from the above, without orphan expiration on this data
set, the orphan pool would have grown an additional 496 entries.
This modifies the way orphan removal and processing is done to more
aggressively remove orphans that can no longer be valid due to other
transactions being added or removed from the primary transaction pool.
The net effect of these changes is that orphan pool will typically be
much smaller which greatly improves its effectiveness. Previously, it
would typically quickly reach the max allowed worst-case usage and
effectively stay there forever.
The following is a summary of the changes:
- Modify the map that tracks which orphans redeem a given transaction to
instead track by the specific outpoints that are redeemed
- Modify the various orphan removal and processing functions to accept
the full transaction rather than just its hash
- Introduce a new flag on removeOrphans which specifies whether or not
to remove the transactions that redeem the orphan being removed as
well which is necessary since only some paths require it
- Add a new function named removeOrphanDoubleSpends that is invoked
whenever a transaction is added to the main pool and thus the outputs
they spent become concrete spends
- Introduce a new flag on maybeAcceptTransaction which specifies whether
or not duplicate orphans should be rejected since only some paths
require it
- Modify processOrphans as follows:
- Make use of the modified map
- Use newly available flags and logic work more strictly work with tx
chains
- Recursively remove any orphans that also redeem any outputs redeemed
by the accepted transactions
- Several new tests to ensure proper functionality
- Removing an orphan that doesn't exist is removed both when there is
another orphan that redeems it and when there is not
- Removing orphans works properly with orphan chains per the new
remove redeemers flag
- Removal of multi-input orphans that double spend an output when a
concrete redeemer enters the transaction pool
This introduces a new pool membership test function to the mempool
testing infrastructure and refactors the tests to make use of it.
It is useful since it is common logic that is not only needed in the
existing tests, but will be needed by most mempool-related tests.
This optimizes the way in which the mempool oprhan map is limited in the
same way the server block manager maps were previously optimized.
Previously the code would read a cryptographically random value large
enough to construct a hash, find the first entry larger than that value,
and evict it.
That approach is quite inefficient and could easily become a
bottleneck when processing transactions due to the need to read from a
source such as /dev/urandom and all of the subsequent hash comparisons.
Luckily, strong cryptographic randomness is not needed here. The primary
intent of limiting the maps is to control memory usage with a secondary
concern of making it difficult for adversaries to force eviction of
specific entries.
Consequently, this changes the code to make use of the pseudorandom
iteration order of Go's maps along with the preimage resistance of the
hashing function to provide the desired functionality. It has
previously been discussed that the specific pseudorandom iteration order
is not guaranteed by the Go spec even though in practice that is how it
is implemented. This is not a concern however because even if the
specific compiler doesn't implement that, the preimage resistance of the
hashing function alone is enough.
The following is a before and after comparison of the function for both
speed and memory allocations:
benchmark old ns/op new ns/op delta
----------------------------------------------------------------
BenchmarkLimitNumOrphans 3727 243 -93.48%
benchmark old allocs new allocs delta
-----------------------------------------------------------------
BenchmarkLimitNumOrphans 4 0 -100.00%
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).
This coincides with the mempool only, policy change which enforces
transaction finality according to the median-time-past rather than
blockheader timestamps. The behavior is pre-cursor to full blown BIP
113 consensus deployment, and subsequent activation.
As a result, the TimeSource field in the mempoolConfig is no longer
needed so it has been removed. Additionally, checkTransactionStandard has been
modified to instead take a time.Time as the mempool is no longer explicitly
dependant on a Chain instance.
This commit adds an additional closure function to the mempool’s config
which computes the median time past from the point of view of the best
node in the chain. The mempool test harness has also been updated to allow
setting a mock median time past for testing purposes.
In addition to increasing the testability of the mempool, this commit
should also speed up transaction and block validation for BIP 113 as
the MTP no longer needs to be re-calculated each time from scratch.
This changes the transaction input standardness checks as follows:
- Allow any script in a pay-to-script-hash transaction to be relayed and
mined so long as it has no more than 15 signature operations
- Remove the obsolete checks which naively calculated the number of
expected inputs in favor of the more accurate ScriptVerifyCleanStack
and ScriptVerifySigPushOnly functionality of the script engine that was
added after these checks.
This adds a basic test harness infrastructure for the mempool package
which aims to make writing tests for it much easier.
The harness provides functionality for creating and signing transactions
as well as a fake chain that provides utxos for use in generating valid
transactions and allows an arbitrary chain height to be set. In order
to simplify transaction creation, a single signing key and payment
address is used throughout and a convenience type for spendable outputs
is provided.
The harness is initialized with a spendable coinbase output by default
and the fake chain height set to the maturity height needed to ensure
the provided output is in fact spendable as well as a policy that is
suitable for testing.
Since tests are in the same package and each harness provides a unique
pool and fake chain instance, the tests can safely reach into the pool
policy, or any other state, and change it for a given harness without
affecting the others.
In order to be able to make use of the existing blockchain.Viewpoint
type, a Clone method has been to the UtxoEntry type which allows the
fake chain instance to keep a single view with the actual available
unspent utxos while the mempool ends up fetching a subset of the view
with the specifically requested entries cloned.
To demo the harness, this also contains a couple of tests which make use
of it:
- TestSimpleOrphanChain -- Ensures an entire chain of orphans is
properly accepted and connects up when the missing parent transaction
is added
- TestOrphanRejects -- Ensure orphans are actually rejected when the
flag on ProcessTransactions is set to reject them
This modifies the config for the new mempool package such that it takes
a callback function to obtain the best chain height instead of requiring
a fully initialized blockchain.BlockChain instance.
This will make it much easier to test the mempool since the tests will
be able to provide their own height function to test various
functionality without having create and manipulate full blocks and chain
instances.
This does the minimum work necessary to refactor the mempool code into
its own package. The idea is that separating this code into its own
package will greatly improve its testability, allow independent
benchmarking and profiling, and open up some interesting opportunities
for future development related to the memory pool.
There are likely some areas related to policy 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:
- Create the new package
- Move several files into the new package:
- mempool.go -> mempool/mempool.go
- mempoolerror.go -> mempool/error.go
- policy.go -> mempool/policy.go
- policy_test.go -> mempool/policy_test.go
- Update mempool logging to use the new mempool package logger
- Rename mempoolPolicy to Policy (so it's now mempool.Policy)
- Rename mempoolConfig to Config (so it's now mempool.Config)
- Rename mempoolTxDesc to TxDesc (so it's now mempool.TxDesc)
- Rename txMemPool to TxPool (so it's now mempool.TxPool)
- Move defaultBlockPrioritySize to the new package and export it
- Export DefaultMinRelayTxFee from the mempool package
- Export the CalcPriority function from the mempool package
- Introduce a new RawMempoolVerbose function on the TxPool and update
the RPC server to use it
- Update all references to the mempool to use the package.
- Add a skeleton README.md
