aboutsummaryrefslogtreecommitdiff
path: root/src/policy
AgeCommit message (Collapse)Author
2024-08-12policy/feerate.h: avoid constraint self-dependencyMatt Whitlock
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/format:48, from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/bits/chrono_io.h:39, from /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/chrono:3362, from ./util/time.h:9, from ./primitives/block.h:12, from ./blockencodings.h:8, from blockencodings.cpp:5: /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits: In substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]': /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1140:25: required by substitution of 'template<class _Tp, class ... _Args> using std::__is_constructible_impl = std::__bool_constant<__is_constructible(_Tp, _Args ...)> [with _Tp = CFeeRate; _Args = {std::optional<CFeeRate>&}]' 1140 | = __bool_constant<__is_constructible(_Tp, _Args...)>; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:1145:12: required from 'struct std::is_constructible<CFeeRate, std::optional<CFeeRate>&>' 1145 | struct is_constructible | ^~~~~~~~~~~~~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:178:35: required by substitution of 'template<class ... _Bn> std::__detail::__first_t<std::integral_constant<bool, false>, typename std::enable_if<(!(bool)(_Bn::value)), void>::type ...> std::__detail::__or_fn(int) [with _Bn = {std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate>}]' 178 | __enable_if_t<!bool(_Bn::value)>...>; | ^~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/type_traits:196:41: required from 'struct std::__or_<std::is_constructible<CFeeRate, std::optional<CFeeRate>&>, std::is_convertible<std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, std::optional<CFeeRate> >, std::is_convertible<std::optional<CFeeRate>, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate>&>, std::is_convertible<const std::optional<CFeeRate>&, CFeeRate>, std::is_constructible<CFeeRate, const std::optional<CFeeRate> >, std::is_convertible<const std::optional<CFeeRate>, CFeeRate> >' 196 | : decltype(__detail::__or_fn<_Bn...>(0)) | ~~~~~~~~~~~~~~~~~~~~~~~~~^~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:824:45: required from 'constexpr const bool std::optional<CFeeRate>::__construct_from_contained_value<CFeeRate, CFeeRate>' 824 | = !__converts_from_optional<_Tp, _From>::value; | ^~~~~ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:7: required by substitution of 'template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<CFeeRate>::optional(const std::optional<_Tp>&) [with _Up = CFeeRate]' 884 | && __construct_from_contained_value<_Up> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ./validation.h:164:41: required from here 164 | return MempoolAcceptResult(state); | ^ /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:886:2: required by the constraints of 'template<class _Tp> template<class _Up> requires !(is_same_v<std::optional<_Tp>, typename std::remove_cvref<_It2>::type>) && (is_constructible_v<_Tp, const _Up&>) && (__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type>) constexpr std::optional<_Tp>::optional(const std::optional<_From>&)' /usr/lib/gcc/x86_64-pc-linux-gnu/15/include/g++-v15/optional:884:14: error: satisfaction of atomic constraint '__construct_from_contained_value<_Up, typename std::remove_cv< <template-parameter-1-1> >::type> [with _Tp = _Tp; _Up = _Up]' depends on itself 884 | && __construct_from_contained_value<_Up> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
2024-07-30policy: stop 3rd party wtxid malleability of anchor spendGreg Sanders
2024-07-08tidy: modernize-use-equals-defaultMarcoFalke
2024-07-02use version=3 instead of v3 in debug stringsglozow
Make it more clear to the user what we mean by v3.
2024-07-02scripted-diff: change names from V3 to TRUCglozow
-BEGIN VERIFY SCRIPT- sed -i 's/SingleV3Checks/SingleTRUCChecks/g' $(git grep -l 'SingleV3Checks') sed -i 's/PackageV3Checks/PackageTRUCChecks/g' $(git grep -l 'PackageV3Checks') sed -i 's/PV3C/PTRUCC/g' src/policy/truc_policy.h sed -i 's/V3_MAX_VSIZE/TRUC_MAX_VSIZE/g' $(git grep -l 'V3_MAX_VSIZE') sed -i 's/V3_CHILD_MAX_VSIZE/TRUC_CHILD_MAX_VSIZE/g' $(git grep -l 'V3_CHILD_MAX_VSIZE') sed -i 's/V3_DESCENDANT_LIMIT/TRUC_DESCENDANT_LIMIT/g' $(git grep -l 'V3_DESCENDANT_LIMIT') sed -i 's/V3_ANCESTOR_LIMIT/TRUC_ANCESTOR_LIMIT/g' $(git grep -l 'V3_ANCESTOR_LIMIT') sed -i 's/CheckMempoolV3Invariants/CheckMempoolTRUCInvariants/g' $(git grep -l 'CheckMempoolV3Invariants') -END VERIFY SCRIPT-
2024-07-02[doc] replace mentions of v3 with TRUCglozow
Keep mentions of v3 in debug strings to help people who might not know that TRUC is applied when version=3. Also keep variable names in tests, as it is less verbose to keep v3 and v2.
2024-06-18rename policy/v3_policy.* to policy/truc_policy.*glozow
2024-06-17Merge bitcoin/bitcoin#28984: Cluster size 2 package rbfAva Chow
94ed4fbf8e1a396c650b5134d396d6c0be35ce10 Add release note for size 2 package rbf (Greg Sanders) afd52d8e63ed323a159ea49fd1f10542abeacb97 doc: update package RBF comment (Greg Sanders) 6e3c4394cfadf32c06c8c4732d136ca10c316721 mempool: Improve logging of replaced transactions (Greg Sanders) d3466e4cc5051c314873dd14ec8f7a88494c0780 CheckPackageMempoolAcceptResult: Check package rbf invariants (Greg Sanders) 316d7b63c97144ba3e21201315c784852210f8ff Fuzz: pass mempool to CheckPackageMempoolAcceptResult (Greg Sanders) 4d15bcf448eb3c4451b63e8f78cc61f3f9f9b639 [test] package rbf (glozow) dc21f61c72e5a97d974ca2c5cb70b8328f4fab2a [policy] package rbf (Suhas Daftuar) 5da396781589177d4ceb3b4b59c9f309a5e4d029 PackageV3Checks: Relax assumptions (Greg Sanders) Pull request description: Allows any 2 transaction package with no in-mempool ancestors to do package RBF when directly conflicting with other mempool clusters of size two or less. Proposed validation steps: 1) If the transaction package is of size 1, legacy rbf rules apply. 2) Otherwise the transaction package consists of a (parent, child) pair with no other in-mempool ancestors (or descendants, obviously), so it is also going to create a cluster of size 2. If larger, fail. 3) The package rbf may not evict more than 100 transactions from the mempool(bip125 rule 5) 4) The package is a single chunk 5) Every directly conflicted mempool transaction is connected to at most 1 other in-mempool transaction (ie the cluster size of the conflict is at most 2). 6) Diagram check: We ensure that the replacement is strictly superior, improving the mempool 7) The total fee of the package, minus the total fee of what is being evicted, is at least the minrelayfee * size of the package (equivalent to bip125 rule 3 and 4) Post-cluster mempool this will likely be expanded to general package rbf, but this is what we can safely support today. ACKs for top commit: achow101: ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 glozow: reACK 94ed4fbf8e via range-diff ismaelsadeeq: re-ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 theStack: Code-review ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 murchandamus: utACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 Tree-SHA512: 9bd383e695964f362f147482bbf73b1e77c4d792bda2e91d7f30d74b3540a09146a5528baf86854a113005581e8c75f04737302517b7d5124296bd7a151e3992
2024-06-10PackageV3Checks: Relax assumptionsGreg Sanders
Relax assumptions about in-mempool children of in-mempool parents. With package RBF, we will allow a package of size 2 with conflicts on its parent and reconsider the parent if its fee is insufficient on its own. Consider: TxA (in mempool) <- TxB (in mempool) TxA (in mempool) <- TxB' (in package, conflicts with TxB) <- TxC (in package) If TxB' fails to RBF TxB due to insufficient feerate, the package TxB' + TxC will be considered. PackageV3Checks called on TxB' will see an in-mempool parent TxA, and see the in-mempool child TxB. We cannot assume there is no in-mempool sibling, rather detect it and fail normally. Prior to package RBF, this would have failed on the first conflict in package.
2024-06-07refactor: Rename CTransaction::nVersion to versionAva Chow
In order to ensure that the change of nVersion to a uint32_t in the previous commit has no effect, rename nVersion to version in this commit so that reviewers can easily spot if a spot was missed or if there is a check somewhere whose semantics have changed.
2024-06-02[policy] make v3 transactions standardglozow
Note that, as CURRENT_VERSION = 2, the wallet will not make transactions with nVersion=3 yet.
2024-05-31[refactor] use TRUC_VERSION in place of 3glozow
2024-05-23Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to ↵Ava Chow
10kvB 154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow) a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow) d578e2e3540e085942001350ff3aeb047bdac973 [policy] explicitly require non-v3 for CPFP carve out (glozow) Pull request description: Opening for discussion / conceptual review. We like the idea of a smaller maximum transaction size because: - It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction) - They are easier to bin-pack in block template production - They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space. History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510): - 2010-09-13 In https://github.com/bitcoin/bitcoin/commit/3df62878c3cece15a8921fbbdee7859ee9368768 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction() - 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning). This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult. ~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number. ACKs for top commit: sdaftuar: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 achow101: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 instagibbs: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 t-bast: ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11 murchandamus: crACK 154b2b2296edccb5ed24e829798dacb6195edc11 Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
2024-05-21[policy] restrict all v3 transactions to 10kvBglozow
2024-05-03Merge bitcoin/bitcoin#30024: doc: replace remaining "520" magic nums with ↵Ava Chow
MAX_SCRIPT_ELEMENT_SIZE ffc674595cb19b2fdc5705b355bdd3e7f724b860 Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZE (Jon Atack) Pull request description: Noticed these while reviewing BIPs yesterday. It would be clearer and more future-proof to refer to their constant name. ACKs for top commit: instagibbs: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 sipa: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 achow101: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860 glozow: ACK ffc674595cb19b2fdc5705b355bdd3e7f724b860, agree it's clearer for these comments to refer to the greppable name of the limit rather than the number Tree-SHA512: 462afc1c64543877ac58cb3acdb01d42c6d08abfb362802f29f3482d75401a2a8adadbc2facd222a9a9fefcaab6854865ea400f50ad60bec17831d29f7798afe
2024-05-02Replace remaining "520" magic numbers with MAX_SCRIPT_ELEMENT_SIZEJon Atack
2024-05-01[txpackages] use std::lexicographical_compare instead of sorting hex stringsglozow
No behavior change, but getting the hex string is more expensive than necessary.
2024-04-30Merge bitcoin/bitcoin#28970: p2p: opportunistically accept 1-parent-1-child ↵Ava Chow
packages e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f [functional test] opportunistic 1p1c package submission (glozow) 87c5c524d63c833cf490c7f2f73d72695ad480df [p2p] opportunistically accept 1-parent-1-child packages (glozow) 6c51e1d7d021ed6523107a6db87a865aaa8fc4c9 [p2p] add separate rejections cache for reconsiderable txns (glozow) 410ebd6efaf20fe4715c9b825103b74db69f35ac [fuzz] break out parent functions and add GetChildrenFrom* coverage (glozow) d095316c1c23e9460dfbd9fdbaf292063adcd080 [unit test] TxOrphanage::GetChildrenFrom* (glozow) 2f51cd680fb4323f1c792dae37d4c4e0e0e35804 [txorphanage] add method to get all orphans spending a tx (glozow) 092c978a42e8f4a02291b994713505ba8aac8b28 [txpackages] add canonical way to get hash of package (glozow) c3c1e15831c463df7968b028a77e787da7e6256d [doc] restore comment about why we check if ptx HasWitness before caching rejected txid (glozow) 6f4da19cc3b1b7cd23cb4be95a6bb9acb79eb3bf guard against MempoolAcceptResult::m_replaced_transactions (glozow) Pull request description: This enables 1p1c packages to propagate in the "happy case" (i.e. not reliable if there are adversaries) and contains a lot of package relay-related code. See https://github.com/bitcoin/bitcoin/issues/27463 for overall package relay tracking. Rationale: This is "non-robust 1-parent-1-child package relay" which is immediately useful. - Relaying 1-parent-1-child CPFP when mempool min feerate is high would be a subset of all package relay use cases, but a pretty significant improvement over what we have today, where such transactions don't propagate at all. [1] - Today, a miner can run this with a normal/small maxmempool to get revenue from 1p1c CPFP'd transactions without losing out on the ones with parents below mempool minimum feerate. - The majority of this code is useful for building more featureful/robust package relay e.g. see the code in #27742. The first 2 commits are followups from #29619: - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1523094034 - https://github.com/bitcoin/bitcoin/pull/29619#discussion_r1519819257 Q: What makes this short of a more full package relay feature? (1) it only supports packages in which 1 of the parents needs to be CPFP'd by the child. That includes 1-parent-1-child packages and situations in which the other parents already pay for themselves (and are thus in mempool already when the package is submitted). More general package relay is a future improvement that requires more engineering in mempool and validation - see #27463. (2) We rely on having kept the child in orphanage, and don't make any attempt to protect it while we wait to receive the parent. If we are experiencing a lot of orphanage churn (e.g. an adversary is purposefully sending us a lot of transactions with missing inputs), we will fail to submit packages. This limitation has been around for 12+ years, see #27742 which adds a token bucket scheme for protecting package-related orphans at a limited rate per peer. (3) Our orphan-handling logic is somewhat opportunistic; we don't make much effort to resolve an orphan beyond asking the child's sender for the parents. This means we may miss packages if the first sender fails to give us the parent (intentionally or unintentionally). To make this more robust, we need receiver-side logic to retry orphan resolution with multiple peers. This is also an existing problem which has a proposed solution in #28031. [1]: see this writeup and its links https://github.com/bitcoin/bips/blob/02ec218c7857ef60914e9a3d383b68caf987f70b/bip-0331.mediawiki#propagate-high-feerate-transactions ACKs for top commit: sr-gi: tACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f instagibbs: reACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f theStack: Code-review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f :package: dergoegge: light Code review ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f achow101: ACK e518a8bf8abf3d7b83c9013f56d0dca18ae04d6f Tree-SHA512: 632579fbe7160cb763bbec6d82ca0dab484d5dbbc7aea90c187c0b9833b8d7c1e5d13b8587379edd3a3b4a02a5a1809020369e9cd09a4ebaf729921f65c15943
2024-04-26[txpackages] add canonical way to get hash of packageglozow
2024-04-22Avoid explicitly computing diagram; compare based on chunksPieter Wuille
2024-03-26ImprovesFeerateDiagram: Spelling fix and removal of unused diagram vectorsGreg Sanders
2024-03-26Merge bitcoin/bitcoin#29242: Mempool util: Add RBF diagram checks for single ↵glozow
chunks against clusters of size 2 72959867784098137a50c34f86deca8235eef4f8 Unit tests for CalculateFeerateDiagramsForRBF (Greg Sanders) b767e6bd47cb0fb8f7aea3fb10c597e59a35bf74 test: unit test for ImprovesFeerateDiagram (Greg Sanders) 7e89b659e1ddd0c04fa2bddba9706b5d1a1daec3 Add fuzz test for FeeFrac (Greg Sanders) 4d6528a3d6bf3821c216c68f99170e2faab5d63c fuzz: fuzz diagram creation and comparison (Greg Sanders) e9c5aeb11d641b8cae373452339760809625021d test: Add tests for CompareFeerateDiagram and CheckConflictTopology (Greg Sanders) 588a98dccc5dbb6e331f28d83a4a10a13d70eb31 fuzz: Add fuzz target for ImprovesFeerateDiagram (Greg Sanders) 2079b80854e2595f6f696e7c13a56c7f2a7da9f4 Implement ImprovesFeerateDiagram (Greg Sanders) 66d966dcfaad3638f84654e710f403cb0a0a2ac7 Add FeeFrac unit tests (Greg Sanders) ce8e22542ed0b4fa5794d3203207146418d59473 Add FeeFrac utils (Greg Sanders) Pull request description: This is a smaller piece of https://github.com/bitcoin/bitcoin/pull/28984 broken off for easier review. Up to date explanation of diagram checks are here: https://delvingbitcoin.org/t/mempool-incentive-compatibility/553 This infrastructure has two near term applications prior to cluster mempool: 1) Limited Package RBF(https://github.com/bitcoin/bitcoin/pull/28984): We want to allow package RBF only when we know it improves the mempool. This narrowly scoped functionality allows use with v3-like topologies, and will be expanded at some point post-cluster mempool when diagram checks can be done efficiently against bounded cluster sizes. 2) Replacement for single tx RBF(in a cluster size of up to two) against conflicts of up to cluster size two. `ImprovesFeerateDiagram` interface will have to change for this use-case, which is a future direction to solve certain pins and improve mempool incentive compatibility: https://delvingbitcoin.org/t/ephemeral-anchors-and-mev/383#diagram-checks-fix-this-3 And longer-term, this would be the proposed way we would compute incentive compatibility for all conflicts, post-cluster mempool. ACKs for top commit: sipa: utACK 72959867784098137a50c34f86deca8235eef4f8 glozow: code review ACK 72959867784098137a50c34f86deca8235eef4f8 murchandamus: utACK 72959867784098137a50c34f86deca8235eef4f8 ismaelsadeeq: Re-ACK https://github.com/bitcoin/bitcoin/commit/72959867784098137a50c34f86deca8235eef4f8 willcl-ark: crACK 72959867784098137a50c34f86deca8235eef4f8 sdaftuar: ACK 72959867784098137a50c34f86deca8235eef4f8 Tree-SHA512: 79593e5a087801c06f06cc8b73aa3e7b96ab938d3b90f5d229c4e4bfca887a77b447605c49aa5eb7ddcead85706c534ac5eb6146ae2396af678f4beaaa5bea8e
2024-03-18Implement ImprovesFeerateDiagramGreg Sanders
This new function takes the populated sets of direct and all conflicts computed in the current mempool, assuming the replacements are a single chunk, and computes a diagram check. The diagram check only works against cluster sizes of 2 or less, and fails if it encounters a different topology. Co-authored-by: Suhas Daftuar <sdaftuar@chaincode.com>
2024-03-01[policy] sibling eviction for v3 transactionsglozow
2024-02-21[refactor] return pair from SingleV3Checksglozow
2024-02-12[doc] fix docs and comments from v3glozow
2024-02-09Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinningAva Chow
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow) e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow) 27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow) 9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow) 158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See #27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see #27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR #25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df5c700e6940c712028303761d91ae15847 achow101: ACK 29029df5c700e6940c712028303761d91ae15847 instagibbs: ACK 29029df5c700e6940c712028303761d91ae15847 modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
2024-02-08[policy] add v3 policy rulesglozow
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
2024-01-31Don't use scientific notation in log messagesKristaps Kaupe
2024-01-16[refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txidglozow
It's preferable to use type-safe transaction identifiers to avoid confusing txid and wtxid. The next commit will add a reference to this set; we use this opportunity to change it to Txid ahead of time instead of adding new uses of uint256.
2024-01-03Merge bitcoin/bitcoin#29013: test: doc: follow-up #28368Gloria Zhao
b1318dcc56a0181783ee7ddbd388ae878a0efc52 test: change `m_submitted_in_package` input to fuzz data provider boolean (ismaelsadeeq) 5615e16b705d74bf6ebb7c39523844f97a41cb6f tx fees: update `m_from_disconnected_block` to `m_mempool_limit_bypassed` (ismaelsadeeq) fcd429664818f14cace580513e7e6159335b5416 doc: fix typo and update incorrect comment (ismaelsadeeq) 562664d26374331d291b97e2e2f7fca1f0fd467b test: wait for fee estimator to catch up before estimating fees (ismaelsadeeq) Pull request description: This is a simple PR that does two things 1. Fixes #29000 by waiting for the fee estimator to catch up after `removeForBlock` calls before calling `estimateFee` in the `BlockPolicyEstimates` unit test. 2. Addressed some outstanding review comments from #28368 - Updated `NewMempoolTransactionInfo::m_from_disconnected_block` to `NewMempoolTransactionInfo::m_mempool_limit_bypassed` which now correctly indicates what the boolean does. - Changed input of `processTransaction`'s tx_info `m_submitted_in_package` input from false to fuzz data provider boolean. - Fixed some typos, and update incorrect comment ACKs for top commit: martinus: re-ACK b1318dcc56a0181783ee7ddbd388ae878a0efc52 glozow: utACK b1318dcc56a0181783ee7ddbd388ae878a0efc52 Tree-SHA512: 45268729bc044da4748fe004524e0df696d2ec92c5bd053db9aad6e15675f3838429b2a7b9061a6b694be4dc319d1782a876b44df506ddd439d62ad07252d0e1
2024-01-02tx fees: update `m_from_disconnected_block` to `m_mempool_limit_bypassed`ismaelsadeeq
The boolean indicates whether the transaction was added without enforcing mempool fee limits. m_mempool_limit_bypassed is the correct variable name. Also changes NewMempoolTransactionInfo booleans descriptions to the format that is consistent with the codebase.
2023-12-09Add multiplication operator to CFeeRateMurch
2023-11-22tx fees, policy: CBlockPolicyEstimator update from `CValidationInterface` ↵ismaelsadeeq
notifications `CBlockPolicyEstimator` will implement `CValidationInterface` and subscribe to its notification to process transactions added and removed from the mempool. Re-delegate calculation of `validForFeeEstimation` from validation to fee estimator. Also clean up the validForFeeEstimation arg thats no longer needed in `CTxMempool`. Co-authored-by: Matt Corallo <git@bluematt.me>
2023-11-22tx fees, policy: update `CBlockPolicyEstimator::processBlock` parameterismaelsadeeq
Update `processBlock` parameter to reference to a vector of `RemovedMempoolTransactionInfo`.
2023-11-22tx fees, policy: cast with static_cast instead of C-Style castismaelsadeeq
2023-11-16Merge bitcoin/bitcoin#28605: Fix typosfanquake
43de4d3630274e1287179c86896ed4c2d8b9eff4 doc: fix typos (Sjors Provoost) Pull request description: This PR fixes typos found by lint-spelling.py using codespell 2.2.6. Our CI linter job uses codespell 2.2.5 and found fewer typos that I did locally. In any case it's happy now. ACKs for top commit: pablomartin4btc: re ACK 43de4d3630274e1287179c86896ed4c2d8b9eff4 Tree-SHA512: c032fe86cb49c924a468385653b31f309a9db68c478d70335bba3e65a1ff3826abe80284fe00a090ab5a509e1edbf17e476f6922fb15d055e50f1103dad2ccb0
2023-11-13Merge bitcoin/bitcoin#28076: util: Replace std::filesystem with util/fs.hfanquake
bbbbdb0cd57d75a06357d2811363d30a498f4499 ci: Add filesystem lint check (MarcoFalke) fada2f91108a56cc5c447bd6b6fac411e4d5cdca refactor: Replace <filesystem> with <util/fs.h> (MarcoFalke) Pull request description: Using `std::filesystem` is problematic: * There is a `fs` namespace wrapper for it. So having two ways to achieve the same is confusing. * Not using the `fs` wrapper is dangerous and buggy, because it disables known bugs by deleting problematic functions. Fix all issues by removing use of it and adding a linter to avoid using it again in the future. ACKs for top commit: TheCharlatan: ACK bbbbdb0cd57d75a06357d2811363d30a498f4499 fanquake: ACK bbbbdb0cd57d75a06357d2811363d30a498f4499 🦀 Tree-SHA512: 0e2d49742b08eb2635e6fce41485277cb9c40fe20b81017c391d3472a43787db1278a236825714ca1e41c9d2f59913865cfb0c649e3c8ab1fb598c849f80c660
2023-11-10[refactor] remove access to mapTx from policy/rbf.cppglozow
2023-11-07doc: fix typosSjors Provoost
As found by lint-spelling.py using codespell 2.2.6.
2023-11-03Merge bitcoin/bitcoin#28758: refactors for subpackage evaluationfanquake
b5a60abe8783852f5b31bc1e63b5836530410e65 MOVEONLY: CleanupTemporaryCoins into its own function (glozow) 10c0a8678cd28e7f0715e6cfa3e651903e4ad4aa [test util] CreateValidTransaction multi-in/out, configurable feerate, signal BIP125 (glozow) 6ff647a7e0d85040a6033047c5cf84f8f22b1c65 scripted-diff: rename CheckPackage to IsWellFormedPackage (glozow) da9aceba217bbded6909f06144eaa1e1a4ebcb69 [refactor] move package checks into helper functions (glozow) Pull request description: This is part of #27463. It splits off the more trivial changes from #26711 for ease of review, as requested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253. - Split package sanitization in policy/packages.h into helper functions - Add some tests for its quirks (https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1340521597) - Rename `CheckPackage` to `IsPackageWellFormed` - Improve the `CreateValidTransaction` unit test utility to: - Configure the target feerate and return the fee paid - Signal BIP125 on transactions to enable RBF tests - Allow the specification of multiple inputs and outputs - Move `CleanupTemporaryCoins` into its own function to be reused later without duplication ACKs for top commit: dergoegge: Code review ACK b5a60abe8783852f5b31bc1e63b5836530410e65 instagibbs: ACK b5a60abe8783852f5b31bc1e63b5836530410e65 Tree-SHA512: 39d67a5f0041e381f0d0f802a98ccffbff11e44daa3a49611189d6306b03f18613d5ff16c618898d490c97a216753e99e0db231ff14d327f92c17ae4d269cfec
2023-11-02Merge bitcoin/bitcoin#21161: Fee estimation: extend bucket ranges consistentlyglozow
a5e39d325da4eeb9273fb7c919fcbfbc721ed75d Fee estimation: extend bucket ranges consistently (Anthony Towns) Pull request description: When calculating a median fee for a confirmation target at a particular threshold, we analyse buckets in ranges rather than individually in case some buckets have very little data. This patch ensures the breaks between ranges are independent of the the confirmation target. Fixes #20725 ACKs for top commit: ismaelsadeeq: Code review ACK a5e39d325da4eeb9273fb7c919fcbfbc721ed75d glozow: btw what I meant by [this](https://github.com/bitcoin/bitcoin/pull/21161#pullrequestreview-1350258467) was ACK a5e39d325da4eeb9273fb7c919fcbfbc721ed75d jonatack: Initial ACK a5e39d325da4eeb9273fb7c919fcbfbc721ed75d Tree-SHA512: 0edf4e56717c4ab8d4ab0bc0f1d7ab36a13b99de12f689e55c9142c6b81691367ffd8df2e8260c5e14335310b1a51770c6c22995db31109976239befcb558ef8
2023-11-01scripted-diff: rename CheckPackage to IsWellFormedPackageglozow
-BEGIN VERIFY SCRIPT- sed -i 's/CheckPackage(/IsWellFormedPackage(/g' $(git grep -l CheckPackage) -END VERIFY SCRIPT-
2023-11-01[refactor] move package checks into helper functionsglozow
This allows IsSorted() and IsConsistent() to be used by themselves. IsSorted() with a precomputed set is used so that we don't create this set multiple times.
2023-10-05Merge bitcoin/bitcoin#27609: rpc: allow submitpackage to be called outside ↵Andrew Chow
of regtest 5b878be742dbfcd232d949d2df1fff4743aec3d8 [doc] add release note for submitpackage (glozow) 7a9bb2a2a59ba49f80519c8435229abec2432486 [rpc] allow submitpackage to be called outside of regtest (glozow) 5b9087a9a7da2602485e85e0b163dc3cbd2daf31 [rpc] require package to be a tree in submitpackage (glozow) e32ba1599c599e75b1da3393f71f633de860505f [txpackages] IsChildWithParentsTree() (glozow) b4f28cc345ef9c5261c4a8d743654a44784c7802 [doc] parent pay for child in aggregate CheckFeeRate (glozow) Pull request description: Permit (restricted topology) submitpackage RPC outside of regtest. Suggested in https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1510851570 This RPC should be safe but still experimental - interface may change, not all features (e.g. package RBF) are implemented, etc. If a miner wants to expose this to people, they can effectively use "package relay" before the p2p changes are implemented. However, please note **this is not package relay**; transactions submitted this way will not relay to other nodes if the feerates are below their mempool min fee. Users should put this behind some kind of rate limit or permissions. ACKs for top commit: instagibbs: ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8 achow101: ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8 dergoegge: Code review ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8 ajtowns: ACK 5b878be742dbfcd232d949d2df1fff4743aec3d8 ariard: Code Review ACK 5b878be742. Though didn’t manually test the PR. Tree-SHA512: 610365c0b2ffcccd55dedd1151879c82de1027e3319712bcb11d54f2467afaae4d05dca5f4b25f03354c80845fef538d3938b958174dda8b14c10670537a6524
2023-10-04[net processing] FeeFilterRounder doesn't own a FastRandomContextdergoegge
2023-10-02[txpackages] IsChildWithParentsTree()glozow
Many edge cases exist when parents in a child-with-parents package can spend each other. However, this pattern should also be uncommon in normal use cases.
2023-09-20Replace MAX_PACKAGE_SIZE with MAX_PACKAGE_WEIGHT to avoid vbyte confusionGreg Sanders
While allowing submitted packages to be slightly larger than what may be allowed in the mempool to allow simpler reasoning about contextual-less checks vs chain limits.
2023-09-14refactor: Replace <filesystem> with <util/fs.h>MarcoFalke
All code in this repo uses <util/fs.h>, except for a few lines. This is confusing and potentially dangerous, if the safe <util/fs.h> wrappers are not used.
2023-09-13Merge bitcoin/bitcoin#28251: validation: fix coins disappearing mid-package ↵fanquake
evaluation 32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 [test] mempool coins disappearing mid-package evaluation (glozow) a67f460c3fd1c7eb8070623666d887eefccff0d6 [refactor] split setup in mempool_limit test (glozow) d08696120e3647b4c2cd0ae8d6e57dea12418b7c [test framework] add ability to spend only confirmed utxos (glozow) 3ea71feb11c261f002ed918f91f3434fd8a23589 [validation] don't LimitMempoolSize in any subpackage submissions (glozow) d227b7234cd4cfd7c593ffcf8e2f24573d1ebea5 [validation] return correct result when already-in-mempool tx gets evicted (glozow) 9698b81828ff98820fa49c83ca364063233374c6 [refactor] back-fill results in AcceptPackage (glozow) 8ad7ad33929ee846a55a43c55732be0cb8973060 [validation] make PackageMempoolAcceptResult members mutable (glozow) 03b87c11ca0705e1d6147b90da33ce555f9f41c8 [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow) 3f01a3dab1c4ee37fd4093b6a0a3b622f53e231d [CCoinsViewMemPool] track non-base coins and allow Reset (glozow) 7d7f7a1189432b1b6245ba25df572229870567cb [policy] check for duplicate txids in package (glozow) Pull request description: While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore. Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out. Pointed out by instagibbs in https://github.com/bitcoin/bitcoin/commit/faeed687e5cde5e32750d93818dd1d4add837f24 on top of the v3 PR. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/28251/commits/32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90