Skip to content

Commit

Permalink
Merge #6499: backport: merge bitcoin#22689, bitcoin#23381, bitcoin#22674
Browse files Browse the repository at this point in the history
, bitcoin#23804, bitcoin#24310, bitcoin#24537, bitcoin#24582, bitcoin#24404, bitcoin#25013, bitcoin#25029, bitcoin#25254, bitcoin#25388 (mempool backports)

578be36 merge bitcoin#25388: move policy constants to policy (Kittywhiskers Van Gogh)
aca2c0d merge bitcoin#25254: Move minRelayTxFee to policy/settings (Kittywhiskers Van Gogh)
1d82dd8 merge bitcoin#25029: Move fee estimation RPCs to separate file (Kittywhiskers Van Gogh)
ad94a30 merge bitcoin#25013: Remove cs_main from verifymessage, move msg utils to new file (Kittywhiskers Van Gogh)
8e42b12 merge bitcoin#24404: Remove confusing P1008R1 violation in ATMPArgs (Kittywhiskers Van Gogh)
006370d merge bitcoin#24582: Move txoutproof RPCs to txoutproof.cpp (Kittywhiskers Van Gogh)
dfa4772 merge bitcoin#24537: Split mempool RPCs from blockchain.cpp (Kittywhiskers Van Gogh)
537d3b3 merge bitcoin#24310: docs / fixups from RBF and packages (Kittywhiskers Van Gogh)
8239d5f merge bitcoin#23804: followups for de-duplication of packages (Kittywhiskers Van Gogh)
71b2623 merge bitcoin#22674: mempool validation and submission for packages of 1 child + parents (Kittywhiskers Van Gogh)
8392d23 merge bitcoin#23381: refactoring for package submission (Kittywhiskers Van Gogh)
d3c0059 merge bitcoin#22689: deprecate top-level fee fields in getmempool RPCs (Kittywhiskers Van Gogh)

Pull request description:

  ## Additional Information

  * [bitcoin#23694](bitcoin#23694) (backported in [dash#6256](#6256)) was backported before [bitcoin#22689](bitcoin#22689) and therefore, the formatting changes in the former were not incorporated in the backport. This has been resolved by accommodating them in the latter.

  * The fields deprecated in [bitcoin#22689](bitcoin#22689) were originally deprecated in [bitcoin#12240](bitcoin#12240) but not gated behind runtime arguments, the backport does so.

  * This pull request includes multiple backports that include code specific to RBF (BIP 125) and SegWit, which have been removed due to Dash Core not supporting them, comments and documentation (like `doc/policy/packages.md`) have been amended to reflect this accordingly.

  * When backporting [bitcoin#22674](bitcoin#22674), a new constructor was introduced for submitting `MEMPOOL_ENTRY` `ResultType`, upstream this can be done trivially as it uses function overloading ([source](https://github.com/bitcoin/bitcoin/pull/22674/files#diff-d3c243938494b10666b44404a27af7d84b44a72b85a27431e0c89e181462ca6eR198-R204)) but as Dash Core doesn't accept
  `replaced_txns`, there is no opportunity to differentiate arguments.
    * This was resolved by accepting `ResultType` as an argument but always passing `MEMPOOL_ENTRY` as the argument ([source](71b2623#diff-d3c243938494b10666b44404a27af7d84b44a72b85a27431e0c89e181462ca6eR232-R242))

  * Unit tests like `package_witness_swap_tests` (introduced in [bitcoin#23804](bitcoin#23804)) have been omitted due to functionality tested not being implemented in Dash Core. Subsequent backports that include modifications to these tests have mirrored similar omissions.

  ## Breaking Changes

  * The top-level fee fields `fee`, `modifiedfee`, `ancestorfees` and `descendantfees` returned by RPCs `getmempoolentry`,`getrawmempool(verbose=true)`, `getmempoolancestors(verbose=true)` and `getmempooldescendants(verbose=true)` are deprecated and will be removed in the next major version (use `-deprecated=fees` if needed in this version). The same fee fields can be accessed through the `fees` object in the result.

    **WARNING: deprecated fields `ancestorfees` and `descendantfees` are denominated in sats, whereas all fields in the `fees` object are denominated in DASH.**

  ## Checklist

  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_

ACKs for top commit:
  UdjinM6:
    utACK 578be36
  PastaPastaPasta:
    utACK 578be36

Tree-SHA512: 38a62ec826f91c349269373463dd47f662cdc14dbb3c82cb5c56e779f22a2623fdce055018f15bcf6b5da029312dbcce0925545650c0eae8e3f5177812b68321
  • Loading branch information
PastaPastaPasta committed Dec 30, 2024
2 parents 6310321 + 578be36 commit a5787c9
Show file tree
Hide file tree
Showing 43 changed files with 2,055 additions and 1,151 deletions.
1 change: 1 addition & 0 deletions doc/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ The Dash Core repo's [root README](/README.md) contains relevant information on
- [Reduce Memory](reduce-memory.md)
- [Reduce Traffic](reduce-traffic.md)
- [Tor Support](tor.md)
- [Transaction Relay Policy](policy/README.md)
- [ZMQ](zmq.md)

License
Expand Down
10 changes: 10 additions & 0 deletions doc/policy/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
# Transaction Relay Policy

Policy is a set of validation rules, in addition to consensus, enforced for unconfirmed
transactions.

This documentation is not an exhaustive list of all policy rules.

- [Packages](packages.md)


62 changes: 62 additions & 0 deletions doc/policy/packages.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
# Package Mempool Accept

## Definitions

A **package** is an ordered list of transactions, representable by a connected Directed Acyclic
Graph (a directed edge exists between a transaction that spends the output of another transaction).

For every transaction `t` in a **topologically sorted** package, if any of its parents are present
in the package, they appear somewhere in the list before `t`.

A **child-with-unconfirmed-parents** package is a topologically sorted package that consists of
exactly one child and all of its unconfirmed parents (no other transactions may be present).
The last transaction in the package is the child, and its package can be canonically defined based
on the current state: each of its inputs must be available in the UTXO set as of the current chain
tip or some preceding transaction in the package.

## Package Mempool Acceptance Rules

The following rules are enforced for all packages:

* Packages cannot exceed `MAX_PACKAGE_COUNT=25` count and `MAX_PACKAGE_SIZE=101KvB` total size
(#20833)

- *Rationale*: This is already enforced as mempool ancestor/descendant limits. If
transactions in a package are all related, exceeding this limit would mean that the package
can either be split up or it wouldn't pass individual mempool policy.

- Note that, if these mempool limits change, package limits should be reconsidered. Users may
also configure their mempool limits differently.

* Packages must be topologically sorted. (#20833)

* Packages cannot have conflicting transactions, i.e. no two transactions in a package can spend
the same inputs. Packages cannot have duplicate transactions. (#20833)

* No transaction in a package can conflict with a mempool transaction.

* When packages are evaluated against ancestor/descendant limits, the union of all transactions'
descendants and ancestors is considered. (#21800)

- *Rationale*: This is essentially a "worst case" heuristic intended for packages that are
heavily connected, i.e. some transaction in the package is the ancestor or descendant of all
the other transactions.

The following rules are only enforced for packages to be submitted to the mempool (not enforced for
test accepts):

* Packages must be child-with-unconfirmed-parents packages. This also means packages must contain at
least 2 transactions. (#22674)

* Transactions in the package that have the same txid as another transaction already in the mempool
will be removed from the package prior to submission ("deduplication").

- *Rationale*: Node operators are free to set their mempool policies however they please, nodes
may receive transactions in different orders, and malicious counterparties may try to take
advantage of policy differences to pin or delay propagation of transactions. As such, it's
possible for some package transaction(s) to already be in the mempool, and there is no need to
repeat validation for those transactions or double-count them in fees.

- *Rationale*: We want to prevent potential censorship vectors. We should not reject entire
packages because we already have one of the transactions. Also, if an attacker first broadcasts
a competing package, the honest package should still be considered for acceptance.
11 changes: 11 additions & 0 deletions doc/release-notes-6499.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
Updated RPCs
------------

- The top-level fee fields `fee`, `modifiedfee`, `ancestorfees` and `descendantfees`
returned by RPCs `getmempoolentry`,`getrawmempool(verbose=true)`,
`getmempoolancestors(verbose=true)` and `getmempooldescendants(verbose=true)`
are deprecated and will be removed in the next major version (use
`-deprecated=fees` if needed in this version). The same fee fields can be accessed
through the `fees` object in the result. WARNING: deprecated
fields `ancestorfees` and `descendantfees` are denominated in sats, whereas all
fields in the `fees` object are denominated in DASH.
5 changes: 5 additions & 0 deletions src/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,7 @@ BITCOIN_CORE_H = \
rpc/blockchain.h \
rpc/client.h \
rpc/index_util.h \
rpc/mempool.h \
rpc/mining.h \
rpc/protocol.h \
rpc/rawtransaction_util.h \
Expand Down Expand Up @@ -528,8 +529,10 @@ libbitcoin_server_a_SOURCES = \
rpc/blockchain.cpp \
rpc/coinjoin.cpp \
rpc/evo.cpp \
rpc/fees.cpp \
rpc/index_util.cpp \
rpc/masternode.cpp \
rpc/mempool.cpp \
rpc/governance.cpp \
rpc/mining.cpp \
rpc/misc.cpp \
Expand All @@ -538,6 +541,8 @@ libbitcoin_server_a_SOURCES = \
rpc/rawtransaction.cpp \
rpc/server.cpp \
rpc/server_util.cpp \
rpc/signmessage.cpp \
rpc/txoutproof.cpp \
script/sigcache.cpp \
shutdown.cpp \
spork.cpp \
Expand Down
1 change: 1 addition & 0 deletions src/Makefile.test.include
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ BITCOIN_TESTS =\
test/torcontrol_tests.cpp \
test/transaction_tests.cpp \
test/txindex_tests.cpp \
test/txpackage_tests.cpp \
test/txreconciliation_tests.cpp \
test/txvalidation_tests.cpp \
test/txvalidationcache_tests.cpp \
Expand Down
2 changes: 1 addition & 1 deletion src/bench/rpc_mempool.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <bench/bench.h>
#include <rpc/blockchain.h>
#include <rpc/mempool.h>
#include <txmempool.h>

#include <univalue.h>
Expand Down
1 change: 1 addition & 0 deletions src/net_processing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
#include <node/blockstorage.h>
#include <node/txreconciliation.h>
#include <policy/policy.h>
#include <policy/settings.h>
#include <primitives/block.h>
#include <primitives/transaction.h>
#include <random.h>
Expand Down
2 changes: 1 addition & 1 deletion src/policy/feerate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <consensus/amount.h>
#include <policy/feerate.h>

#include <tinyformat.h>

CFeeRate::CFeeRate(const CAmount& nFeePaid, uint32_t num_bytes)
Expand Down
3 changes: 3 additions & 0 deletions src/policy/feerate.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
#include <consensus/amount.h>
#include <serialize.h>


#include <cstdint>
#include <string>
#include <type_traits>

const std::string CURRENCY_UNIT = "DASH"; // One formatted unit
const std::string CURRENCY_ATOM = "duff"; // One indivisible minimum value unit
Expand Down
18 changes: 18 additions & 0 deletions src/policy/fees.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,30 @@
#include <policy/fees.h>

#include <clientversion.h>
#include <consensus/amount.h>
#include <fs.h>
#include <logging.h>
#include <policy/feerate.h>
#include <primitives/transaction.h>
#include <random.h>
#include <serialize.h>
#include <streams.h>
#include <sync.h>
#include <tinyformat.h>
#include <txmempool.h>
#include <uint256.h>
#include <util/serfloat.h>
#include <util/system.h>
#include <util/time.h>

#include <algorithm>
#include <cassert>
#include <cmath>
#include <cstddef>
#include <cstdint>
#include <exception>
#include <stdexcept>
#include <utility>

static const char* FEE_ESTIMATES_FILENAME = "fee_estimates.dat";

Expand Down
6 changes: 3 additions & 3 deletions src/policy/fees.h
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,20 @@

#include <consensus/amount.h>
#include <policy/feerate.h>
#include <uint256.h>
#include <random.h>
#include <sync.h>
#include <threadsafety.h>
#include <uint256.h>

#include <array>
#include <map>
#include <memory>
#include <set>
#include <string>
#include <vector>

class CAutoFile;
class CFeeRate;
class CTxMemPoolEntry;
class CTxMemPool;
class TxConfirmStats;

/* Identifier for each of the 3 different TxConfirmStats which will track
Expand Down
23 changes: 22 additions & 1 deletion src/policy/packages.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,16 @@
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.

#include <consensus/validation.h>
#include <policy/packages.h>
#include <policy/policy.h>
#include <primitives/transaction.h>
#include <uint256.h>
#include <util/hasher.h>

#include <algorithm>
#include <cassert>
#include <iterator>
#include <memory>
#include <numeric>
#include <unordered_set>

Expand Down Expand Up @@ -60,3 +64,20 @@ bool CheckPackage(const Package& txns, PackageValidationState& state)
}
return true;
}

bool IsChildWithParents(const Package& package)
{
assert(std::all_of(package.cbegin(), package.cend(), [](const auto& tx){return tx != nullptr;}));
if (package.size() < 2) return false;

// The package is expected to be sorted, so the last transaction is the child.
const auto& child = package.back();
std::unordered_set<uint256, SaltedTxidHasher> input_txids;
std::transform(child->vin.cbegin(), child->vin.cend(),
std::inserter(input_txids, input_txids.end()),
[](const auto& input) { return input.prevout.hash; });

// Every transaction must be a parent of the last transaction in the package.
return std::all_of(package.cbegin(), package.cend() - 1,
[&input_txids](const auto& ptx) { return input_txids.count(ptx->GetHash()) > 0; });
}
18 changes: 18 additions & 0 deletions src/policy/packages.h
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@
#ifndef BITCOIN_POLICY_PACKAGES_H
#define BITCOIN_POLICY_PACKAGES_H

#include <consensus/consensus.h>
#include <consensus/validation.h>
#include <policy/policy.h>
#include <primitives/transaction.h>

#include <cstdint>
#include <vector>

/** Default maximum number of transactions in a package. */
Expand All @@ -17,6 +19,15 @@ static constexpr uint32_t MAX_PACKAGE_COUNT{25};
static constexpr uint32_t MAX_PACKAGE_SIZE{101};
static_assert(MAX_PACKAGE_SIZE * 1000 >= MAX_STANDARD_TX_SIZE);

// If a package is submitted, it must be within the mempool's ancestor/descendant limits. Since a
// submitted package must be child-with-unconfirmed-parents (all of the transactions are an ancestor
// of the child), package limits are ultimately bounded by mempool package limits. Ensure that the
// defaults reflect this constraint.
static_assert(DEFAULT_DESCENDANT_LIMIT >= MAX_PACKAGE_COUNT);
static_assert(DEFAULT_ANCESTOR_LIMIT >= MAX_PACKAGE_COUNT);
static_assert(DEFAULT_ANCESTOR_SIZE_LIMIT >= MAX_PACKAGE_SIZE);
static_assert(DEFAULT_DESCENDANT_SIZE_LIMIT >= MAX_PACKAGE_SIZE);

/** A "reason" why a package was invalid. It may be that one or more of the included
* transactions is invalid or the package itself violates our rules.
* We don't distinguish between consensus and policy violations right now.
Expand All @@ -25,6 +36,7 @@ enum class PackageValidationResult {
PCKG_RESULT_UNSET = 0, //!< Initial value. The package has not yet been rejected.
PCKG_POLICY, //!< The package itself is invalid (e.g. too many transactions).
PCKG_TX, //!< At least one tx is invalid.
PCKG_MEMPOOL_ERROR, //!< Mempool logic error.
};

/** A package is an ordered list of transactions. The transactions cannot conflict with (spend the
Expand All @@ -41,4 +53,10 @@ class PackageValidationState : public ValidationState<PackageValidationResult> {
*/
bool CheckPackage(const Package& txns, PackageValidationState& state);

/** Context-free check that a package is exactly one child and its parents; not all parents need to
* be present, but the package must not contain any transactions that are not the child's parents.
* It is expected to be sorted, which means the last transaction must be the child.
*/
bool IsChildWithParents(const Package& package);

#endif // BITCOIN_POLICY_PACKAGES_H
16 changes: 14 additions & 2 deletions src/policy/policy.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,20 @@
#include <policy/policy.h>

#include <coins.h>
#include <policy/settings.h>

#include <consensus/amount.h>
#include <consensus/consensus.h>
#include <consensus/validation.h>
#include <policy/feerate.h>
#include <primitives/transaction.h>
#include <script/interpreter.h>
#include <script/script.h>
#include <script/standard.h>
#include <serialize.h>
#include <span.h>

#include <algorithm>
#include <cstddef>
#include <vector>

CAmount GetDustThreshold(const CTxOut& txout, const CFeeRate& dustRelayFeeIn)
{
Expand Down
Loading

0 comments on commit a5787c9

Please sign in to comment.