aboutsummaryrefslogtreecommitdiff
path: root/src/policy
AgeCommit message (Collapse)Author
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
2023-09-13[policy] check for duplicate txids in packageglozow
Duplicates of normal transactions would be found by looking for conflicting inputs, but this doesn't catch identical empty transactions. These wouldn't be valid but exiting early is good and AcceptPackage's result sanity checks assume non-duplicate transactions.
2023-08-23Merge bitcoin/bitcoin#26291: Update MANDATORY_SCRIPT_VERIFY_FLAGSAndrew Chow
1b09cc5959d4719ffad131b395f8185e9ab4b1a1 Make post-p2sh consensus rules mandatory for tx relay (Anthony Towns) 69c31bc748104407c596e84bcef893dc968fd758 doc, policy: Clarify comment on STANDARD_SCRIPT_VERIFY_FLAGS (Anthony Towns) Pull request description: The `MANDATORY_SCRIPT_VERIFY_FLAGS` constant was introduced in #3843 to distinguish between block consensus rules and relay standardness rules. However it was not actually used in the consensus code path: instead it only differentiates between the failure being reported as `TX_CONSENSUS` and `mandatory-script-verify-flag-failed` vs `TX_NOT_STANDARD` and `non-mandatory-script-verify-flag`. This updates the list of mandatory flags to include the post-p2sh soft forks that are enforced as consensus rules via `GetBlockScriptFlags()`. The effect of this change is that validation.cpp will report `TX_CONSENSUS` failures for txs that fail dersig/csv/cltv/nulldummy/witness/taproot checks, instead of `TX_NOT_STANDARD`, which in turn adds `Misbehaving(100)` via `MaybePunishNodeForTx` in `net_processing`. ACKs for top commit: Sjors: Code review ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1 darosior: ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1 achow101: ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1 theStack: Concept and code-review ACK 1b09cc5959d4719ffad131b395f8185e9ab4b1a1 Tree-SHA512: d3e5868e8cece478f2e934956ba0c231d8bb9c2daefd0df1f817774e292049902cfc1d0cd76dbd2e7722627a93eab2d7046ff678199aac70a2b01642e69349f1
2023-08-22Merge bitcoin/bitcoin#28157: test doc: tests `acceptstalefeeestimates` ↵glozow
option is only supported on regtest chain ee5a0369cc4305da7b3d26f37677de05ad797e51 test: ensure acceptstalefeeestimates is supported only on regtest chain (ismaelsadeeq) 22d5d4b2b2486feaef981e96f0321f020617f082 tx fees, policy: doc: update and delete unnecessary comment (ismaelsadeeq) Pull request description: This PR Follow up comments from [#27622](https://github.com/bitcoin/bitcoin/pull/27622) It test that the new `regtest-only` option `acceptstalefeeestimates` is not supported on [main, signet and test chains](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235218268), removes an unnecessary [comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1235204323), and update fee estimator `MAXFILEAGE` [description comment](https://github.com/bitcoin/bitcoin/pull/27622/files#r1233887314). ACKs for top commit: jonatack: ACK ee5a0369cc4305da7b3d26f37677de05ad797e51 glozow: utACK ee5a0369cc4305da7b3d26f37677de05ad797e51 Tree-SHA512: 4755f25b08db62f37614ea768272b12580ee0d481fb7fa339379901a6132c66828777c6747d3fe67490ceace3a6ff248bf13bdf65720f6e5ba8642eb762acd3c
2023-08-18Make post-p2sh consensus rules mandatory for tx relayAnthony Towns
2023-08-18doc, policy: Clarify comment on STANDARD_SCRIPT_VERIFY_FLAGSAnthony Towns
2023-08-17tx fees, policy: doc: update and delete unnecessary commentismaelsadeeq
2023-08-14Rename script/standard.{cpp/h} to script/solver.{cpp/h}Andrew Chow
Since script/standard only contains things that are used by the Solver and its callers, rename the files to script/solver.
2023-08-14MOVEONLY: Move datacarrier defaults to policy.hAndrew Chow
2023-08-14Move MANDATORY_SCRIPT_VERIFY_FLAGS from script/standard.h to policy/policy.hAnthony Towns
2023-06-26Merge bitcoin/bitcoin#27914: feerate: For GetFeePerK() return nSatoshisPerK ↵fanquake
instead of round trip through GetFee 11d650060aed25273d860baa4e03168a778832bb feerate: For GetFeePerK() return nSatoshisPerK instead of round trip through GetFee (Andrew Chow) Pull request description: Returning the sats/kvb does not need to round trip through GetFee(1000) since the feerate is already stored as sats/kvb. Fixes #27913, although this does bring up a larger question of how we should handle such large feerates in fuzzing. ACKs for top commit: furszy: Code ACK 11d65006 Tree-SHA512: bec1a0d4b572a0c810cf7eb4e97d729d67e96835c2d576a909f755b053a9707c2f1b3df9adb8f08a9c4d310cdbb8b1e1b42b9c004bd1ade02a07d8ce9e902138
2023-06-19feerate: For GetFeePerK() return nSatoshisPerK instead of round trip through ↵Andrew Chow
GetFee Returning the sats/kvb does not need to round trip through GetFee(1000) since the feerate is already stored as sats/kvb.
2023-06-14tx fees, policy: read stale fee estimates with a regtest-only optionismaelsadeeq
If -acceptstalefeeestimates option is passed stale fee estimates can now be read when operating in regtest environments. Additionally, this commit updates all declarations of the CBlockPolicyEstimator class to include a the second constructor variable.
2023-06-14tx fees, policy: do not read estimates of old fee_estimates.datismaelsadeeq
Old fee estimates could cause transactions to become stuck in the mempool. This commit prevents the node from using stale estimates from an old file.
2023-06-14tx fees, policy: periodically flush fee estimates to fee_estimates.datismaelsadeeq
This reduces chances of having old estimates in fee_estimates.dat.
2023-06-12Use `int32_t` type for most transaction size/weight valuesHennadii Stepanov
This change gets rid of a few casts and makes the following commit diff smaller.
2023-05-20refactor: Move system from util to common libraryTheCharlatan
Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md.
2023-04-19move-only: Extract common/args and common/config.cpp from util/systemTheCharlatan
This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.
2023-03-23refactor: Move fs.* to util/fs.*TheCharlatan
The fs.* files are already part of the libbitcoin_util library. With the introduction of the fs_helpers.* it makes sense to move fs.* into the util/ directory as well.
2023-01-31clang-tidy: Fix `modernize-use-default-member-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
2023-01-03Merge bitcoin/bitcoin#26289: Use util::Result in for calculating mempool ↵Andrew Chow
ancestors 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v) 5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v) f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v) 66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v) Pull request description: Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear. ## Unexpected behaviour This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs. In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit. ## Improvements ### Return value instead of out-parameters This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit. ### Observability There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](https://github.com/bitcoin/bitcoin/blob/69b10212ea5370606c7a5aa500a70c36b4cbb58f/src/node/miner.cpp#L399). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here. ACKs for top commit: achow101: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26289/commits/47c4b1f52ab8d95d7deef83050bad49d1e3e5990 glozow: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 furszy: light code review ACK 47c4b1f5 aureleoules: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
2022-12-24scripted-diff: Insert missed copyright headerHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py insert src/policy/fees_args.cpp -END VERIFY SCRIPT-
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-21Merge bitcoin/bitcoin#26265: POLICY: Relax MIN_STANDARD_TX_NONWITNESS_SIZE ↵Andrew Chow
to 65 non-witness bytes b2aa9e85289fc654106a890c35935e9c76c411fb Add release note for MIN_STANDARD_TX_NONWITNESS_SIZE relaxation (Greg Sanders) 8c5b3646b5afe8a61f5c66478d8e11f0d2ce5108 Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes (Greg Sanders) Pull request description: Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was proposed as a consensus change, and is the simpler of the two suggestions. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes Related mailing list discussions here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/020995.html And a couple years earlier: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-May/017883.html ACKs for top commit: achow101: reACK b2aa9e85289fc654106a890c35935e9c76c411fb glozow: reACK b2aa9e85289fc654106a890c35935e9c76c411fb pablomartin4btc: re-ACK https://github.com/bitcoin/bitcoin/commit/b2aa9e85289fc654106a890c35935e9c76c411fb jonatack: ACK b2aa9e85289fc654106a890c35935e9c76c411fb with some suggestions Tree-SHA512: c1ec1af9ddcf31b2272209a4f1ee0c5607399f8172e5a1dfd4604cf98bfb933810dd9369a5917ad122add003327c9fcf6ee26995de3aca41d5c42dba527991ad
2022-12-19Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytesGreg Sanders
Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was the route taken. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes
2022-12-13mempool: log/halt when CalculateMemPoolAncestors fails unexpectedlystickies-v
When CalculateMemPoolAncestors fails unexpectedly (e.g. it exceeds ancestor/descendant limits even though we expect no limits to be applied), add an error log entry for increased visibility. For debug builds, the application will even halt completely since this is not supposed to happen.
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-07 12:48:07 +0000