aboutsummaryrefslogtreecommitdiff
path: root/src/validation.h
AgeCommit message (Collapse)Author
2021-12-01Merge bitcoin/bitcoin#22677: cut the validation <-> txmempool circular ↵MarcoFalke
dependency 2/2 a64078e38563ef3ac5e5ec20c07569441c87eeee Break validation <-> txmempool circular dependency (glozow) 64e4963c635ec3a73a5fa3f32f6ec08e70609f60 [mempool] always assert coin spent (glozow) bb9078ed51159fa162484f16993313ed6cf980e3 [refactor] put finality and maturity checking into a lambda (glozow) bedf246f1e2497a3641093c6e8fa11fb34dddac4 [mempool] only update lockpoints for non-removed entries (glozow) 1b3a11e126b258fba975ed7c452221608f2c5472 MOVEONLY: TestLockPointValidity to txmempool (glozow) Pull request description: Remove 2 circular dependencies: validation - txmempool and validation - policy/rbf - txmempool Validation should depend on txmempool (e.g. `CChainstateManager` has a mempool and we often need to know what's in our mempool to validate transactions), but txmempool is a data structure that shouldn't really need to know about chain state. - Changes `removeForReorg()` to be parameterized by a callable that returns true/false (i.e. whether the transaction should be removed due to being now immature or nonfinal) instead of a `CChainState`. The mempool really shouldn't need to know about coinbase maturity or lockpoints, it just needs to know which entries to remove. ACKs for top commit: laanwj: Code review ACK a64078e38563ef3ac5e5ec20c07569441c87eeee mjdietzx: reACK a64078e38563ef3ac5e5ec20c07569441c87eeee theStack: re-ACK a64078e38563ef3ac5e5ec20c07569441c87eeee Tree-SHA512: f75995200569c09dfb8ddc09729da66ddb32167ff1e8a7e72f105ec062d2d6a9a390e6b4a2a115e7ad8ad3525f891ee1503f3cd2bed11773abcaf7c3230b1136
2021-12-01Remove AcceptToMemoryPoolWithTimelsilva01
2021-11-30MOVEONLY: TestLockPointValidity to txmempoolglozow
2021-11-10Merge bitcoin/bitcoin#23173: Add `ChainstateManager::ProcessTransaction`MarcoFalke
0fdb619aaf1d62598263361a6082d182be1af792 [validation] Always call mempool.check() after processing a new transaction (John Newbery) 2c64270bbe523ef87e7225c351464e7c716f0b3e [refactor] Don't call AcceptToMemoryPool() from outside validation.cpp (John Newbery) 92a3aeecf6a82e9cbc9fda11022b0548efd24d05 [validation] Add CChainState::ProcessTransaction() (John Newbery) 36167faea92c97ddea7403280a5074073c8e5f90 [logging/documentation] Remove reference to AcceptToMemoryPool from error string (John Newbery) 4c24142b1ec121623f81ba644d77341bc1bd88dd [validation] Remove comment about AcceptToMemoryPool() (John Newbery) 5759fd12b8d5937e9187fa33489a95b1d8e6d1e5 [test] Don't set bypass_limits to true in txvalidation_tests.cpp (John Newbery) 497c9e29640858bb3beb20089c2d4f9e133c7e42 [test] Don't set bypass_limits to true in txvalidationcache_tests.cpp (John Newbery) Pull request description: Similarly to how #18698 added `ProcessNewBlock()` and `ProcessNewBlockHeaders()` methods to the `ChainstateManager` class, this PR adds a new `ProcessTransaction()` method. Code outside validation no longer calls `AcceptToMemoryPool()` directly, but calls through the higher-level `ProcessTransaction()` method. Advantages: - The interface is simplified. Calling code no longer needs to know about the active chainstate or mempool object, since `AcceptToMemoryPool()` can only ever be called for the active chainstate, and that chainstate knows which mempool it's using. We can also remove the `bypass_limits` argument, since that can only be used internally in validation. - responsibility for calling `CTxMemPool::check()` is removed from the callers, and run automatically by `ChainstateManager` every time `ProcessTransaction()` is called. ACKs for top commit: lsilva01: tACK 0fdb619 on Ubuntu 20.04 theStack: Code-review ACK 0fdb619aaf1d62598263361a6082d182be1af792 ryanofsky: Code review ACK 0fdb619aaf1d62598263361a6082d182be1af792. Only changes since last review: splitting & joining commits, adding more explanations to commit messages, tweaking MEMPOOL_ERROR string, fixing up argument name comments. Tree-SHA512: 0b395c2e3ef242f0d41d47174b1646b0a73aeece38f1fe29349837e6fb832f4bf8d57e1a1eaed82a97c635cfd59015a7e07f824e0d7c00b2bee4144e80608172
2021-11-03[refactor] Don't call AcceptToMemoryPool() from outside validation.cppJohn Newbery
2021-11-03[validation] Add CChainState::ProcessTransaction()John Newbery
This just calls through to AcceptToMemoryPool() internally, and is currently unused. Also add a new transaction validation failure reason TX_NO_MEMPOOL to indicate that there is no mempool.
2021-10-28[validation/rpc] cache + use vsize calculated in PreChecksglozow
This is not only cleaner but also helps make sure we are always using the virtual size measure that includes the sigop weight heuristic (which is the vsize the mempool would return).
2021-10-25Merge bitcoin/bitcoin#23157: txmempool -/-> validation 1/2: improve ↵MarcoFalke
performance of check() and remove dependency on validation 082c5bf099c64e3d27abe9b68a71ce500b693e7e [refactor] pass coinsview and height to check() (glozow) ed6115f1eae0eb4669601106a9aaff078a2f3a74 [mempool] simplify some check() logic (glozow) 9e8d7ad5d9cc4b013826daead9cee09aad539401 [validation/mempool] use Spend/AddCoin instead of UpdateCoins (glozow) 09d18916afb0ecae90700d4befd9d5dc52767970 MOVEONLY: remove single-use helper func CheckInputsAndUpdateCoins (glozow) e8639ec26aaf4de3fae280963434bf1cf2017b6f [mempool] remove now-unnecessary code (glozow) 54c6f3c1da01090aee9691a2c2bee0984a054ce8 [mempool] speed up check() by using coins cache and iterating in topo order (glozow) 30e240f65e69c6dffcd033afc63895345bd51f53 [bench] Benchmark CTxMemPool::check() (glozow) cb1407196fba648aa75504e3ab3d46aa0181563a [refactor/bench] make mempool_stress bench reusable and parameterizable (glozow) Pull request description: Remove the txmempool <-> validation circular dependency by removing txmempool's dependency on validation. There are two functions in txmempool that need validation right now: `check()` and `removeForReorg()`. This PR removes the dependencies in `check()`. This PR also improves the performance of `CTxMemPool::check()` by walking through the entries exactly once, in ascending ancestorcount order, which guarantees that we see parents before children. ACKs for top commit: jnewbery: reACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e GeneFerneau: tACK [082c5bf](https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e) rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/23157/commits/082c5bf099c64e3d27abe9b68a71ce500b693e7e theStack: Code-review ACK 082c5bf099c64e3d27abe9b68a71ce500b693e7e Tree-SHA512: 40ac622af1627b5c3e6abb4f0f035d833265a8c5e8dc88faf5354875dfb5137f137825e54bbd2a2668ed37b145c5d02285f776402629f58596e51853a9a79d29
2021-10-12Fix outdated comments referring to ::ChainActive()Samuel Dobson
2021-10-04[validation/mempool] use Spend/AddCoin instead of UpdateCoinsglozow
UpdateCoins is an unnecessary dependency on validation. All we need to do is add and remove coins to check inputs. We don't need the extra logic for checking coinbases and handling TxUndos. Also remove the wrapper function in validation.h which constructs a throwaway TxUndo object before calling UpdateCoins because it is now unused.
2021-09-30[MOVEONLY] consensus: move amount.h into consensusfanquake
Move amount.h to consensus/amount.h. Renames, adds missing and removes uneeded includes.
2021-09-23Merge bitcoin/bitcoin#21526: validation: UpdateTip/CheckBlockIndex ↵W. J. van der Laan
assumeutxo support 673a5bd3377929a0a6a62eda8b560e47bc2cca0c test: validation: add unittest for UpdateTip behavior (James O'Beirne) 2705570109a2a90ecfd3f4180944498626fc2707 test: refactor: separate CreateBlock in TestChain100Setup (James O'Beirne) 298bf5d563cc740c6ae71750d86942e0278b22d6 test: refactor: declare NoMalleation const auto (James O'Beirne) 071200993f3a9412821ce5387851d659baf85327 move-only: unittest: add test/util/chainstate.h (James O'Beirne) 8f5710fd0ac5173b577e5d00708485170b321bcc validation: fix CheckBlockIndex for multiple chainstates (James O'Beirne) 5a807736dacfc3e6fa57231219336acf08be38fb validation: insert assumed-valid block index entries into candidates (James O'Beirne) 01a9b8fe719efab2c268dc738bc93cfbdf92edb7 validation: set BLOCK_ASSUMED_VALID during snapshot load (James O'Beirne) 42b2520db93fd9feb3df4101654391fa7d3e2140 chain: add BLOCK_ASSUMED_VALID for use with assumeutxo (James O'Beirne) b217020df78bc981d221fe04497c831120ef969f validation: change UpdateTip for multiple chainstates (James O'Beirne) 665072a36df2e4c88705fedd4ac7c955d7f6a488 doc: add comment for g_best_block (James O'Beirne) ac4051d891e2d5c8ac130da16b85b9d880b44720 refactor: remove unused assumeutxo methods (James O'Beirne) 9f6bb539359b98d5b39482ab8a28a68608f0c645 validation: add chainman ref to CChainState (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606) --- Modify UpdateTip and CheckBlockIndex for use with multiple chainstates. Includes a new unittest verifying `g_best_block` behavior (previously untested at the unit level) and various changes necessary for running and testing `ProcessNewBlock()`-like behavior on the background validation chainstate. This changeset introduces a new block index `nStatus` flag called `BLOCK_ASSUMED_VALID`, and it is applied to block index entries that are beneath the UTXO snapshot base block upon snapshot load. Once each block is validated (during async background validation), the flag is removed. This allows us to avoid (ab)using `BLOCK_VALID_*` flags for snapshot chain block entries, and preserves the original meaning of those flags. Note: this PR previously incorporated changes to `LoadBlockIndex()` and `RewindBlockIndex()` as noted in Russ' comments below, but once I generated the changes necessary to test the UpdateTip change, I decided to split this changes out into another PR due to the size of this one. ACKs for top commit: achow101: ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c jonatack: Code-review re-ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c reviewed diff, rebased to master/debug build/ran unit+functional tests naumenkogs: ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c fjahr: Code review ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c ariard: utACK 673a5bd3 ryanofsky: Code review ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c. Just linker fix and split commit changes mentioned https://github.com/bitcoin/bitcoin/pull/21526#issuecomment-921064563 since last review benthecarman: ACK 673a5bd3377929a0a6a62eda8b560e47bc2cca0c Tree-SHA512: 0a6dc23d041b27ed9fd0ee1f3e5971b92fb1d2df2fc9b655d5dc48594235321ab1798d06de2ec55482ac3966a9ed56de8d56e9e29cae75bbe8690bafc2dda383
2021-09-20refactor: Remove unused validation includesMarcoFalke
2021-09-16Merge bitcoin/bitcoin#22626: Remove txindex migration codeW. J. van der Laan
fa20f815a9cb438c5ab61e97a453612ddd8b21b5 Remove txindex migration code (MarcoFalke) fae878603345854527c211ebb7d1967f12c8bb9d doc: Fix validation typo (MarcoFalke) fab89006d656261770503e54fdd01ac9167bdd49 Add missing includes and forward declarations, remove unused ones (MarcoFalke) Pull request description: No supported version of Bitcoin Core used the legacy txindex, so all relevant nodes can be assumed to have upgraded. Thus, there is no need to keep this code any longer. As a temporary courtesy, provide a one-time warning on how to free the disk space used by the legacy txindex. Fixes #22615 ACKs for top commit: laanwj: Code review ACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5 hebasto: ACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5, tested on Linux Mint 20.2 (x86_64). Zero-1729: crACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5 theStack: Approach ACK fa20f815a9cb438c5ab61e97a453612ddd8b21b5 Tree-SHA512: 68aa32d064d1e3932e6e382816a4b5de417bd7e82861fea1ee50660e8c397f4efeb88ae4ed54a8ad1952c3563eb0b8449d7ccf883c353cc4d4dc7e15c53d78e8
2021-09-15validation: insert assumed-valid block index entries into candidatesJames O'Beirne
2021-09-15doc: add comment for g_best_blockJames O'Beirne
2021-09-03Add LIFETIMEBOUND to InitializeChainstateMarcoFalke
2021-08-29refactor: remove RecursiveMutex cs_nBlockSequenceIdSebastian Falbesoner
The RecursiveMutex cs_nBlockSequenceId is only used at one place in CChainState::ReceivedBlockTransactions() to atomically read-and-increment the nBlockSequenceId member. At this point, the cs_main lock is set, hence we can use a plain int for the member and mark it as guarded by cs_main.
2021-08-20Add missing includes and forward declarations, remove unused onesMarcoFalke
2021-08-05misc package validation doc improvementsglozow
2021-07-28Merge bitcoin/bitcoin#22528: refactor: move GetTransaction to ↵MarcoFalke
node/transaction.cpp f685a13bef0418663015ea6d8f448f075510c0ec doc: GetTransaction()/getrawtransaction follow-ups to #22383 (John Newbery) abc57e1f0882a1a2bb20474648419979af6e383d refactor: move `GetTransaction(...)` to node/transaction.cpp (Sebastian Falbesoner) Pull request description: ~This PR is based on #22383, which should be reviewed first~ (merged by now). In [yesterday's PR review club session to PR 22383](https://bitcoincore.reviews/22383), the idea of moving the function `GetTransaction(...)` from src/validation.cpp to src/node/transaction.cpp came up. With this, the circular dependency "index/txindex -> validation -> index/txindex" is removed (see change in `lint-circular-dependencies.sh`). Thanks to jnewbery for suggesting and to sipa for providing historical background. Relevant IRC log: ``` 17:52 <jnewbery> Was anyone surprised that GetTransaction() is in validation.cpp? It seems to me that node/transaction.cpp would be a more appropriate place for it. 17:53 <raj_> jnewbery, +1 17:53 <stickies-v> agreed! 17:54 <glozow> jnewbery ya 17:54 <jnewbery> seems weird that validation would call into txindex. I wonder if we remove this function, then validation would no longer need to #include txindex 17:54 <sipa> GetTransaction predates node/transaction.cpp, and even the generic index framework itself :) 17:55 <sipa> (before 0.8, validation itself used the txindex) 17:55 <jnewbery> (and GetTransaction() seems like a natural sibling to BroadcastTransaction(), which is already in node/transaction.cpp) 17:55 <jnewbery> sipa: right, this is not meant as a criticism of course. Just wondering if we can organize things a bit more rationally now that we have better separation between things. 17:55 <sipa> jnewbery: sure, just providing background 17:56 <sipa> seems very reasonable to move it elsewhere now ``` The commit should be trivial to review with `--color-moved`. ACKs for top commit: jnewbery: Code review ACK f685a13bef0418663015ea6d8f448f075510c0ec rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/22528/commits/f685a13bef0418663015ea6d8f448f075510c0ec mjdietzx: crACK f685a13bef0418663015ea6d8f448f075510c0ec LarryRuane: Code review, test ACK f685a13bef0418663015ea6d8f448f075510c0ec Tree-SHA512: 0e844a6ecb1be04c638b55bc4478c2949549a4fcae01c984eee078de74d176fb19d508fc09360a62ad130677bfa7daf703b67870800e55942838d7313246248c
2021-07-22Merge bitcoin/bitcoin#22383: rpc: Prefer to use txindex if available for ↵MarcoFalke
GetTransaction 78f4c8b98eada337346ffb206339c3ebae4ff43b prefer to use txindex if available for GetTransaction (Jameson Lopp) Pull request description: Fixes #22382 Motivation: prevent excessive disk reads if txindex is enabled. Worth noting that this could be argued to be less of a bug and more of an issue of undefined behavior. If a user calls GetTransaction with the wrong block hash, what should happen? ACKs for top commit: jonatack: ACK 78f4c8b98eada337346ffb206339c3ebae4ff43b theStack: Code review ACK 78f4c8b98eada337346ffb206339c3ebae4ff43b LarryRuane: tACK 78f4c8b98eada337346ffb206339c3ebae4ff43b luke-jr: utACK 78f4c8b98eada337346ffb206339c3ebae4ff43b jnewbery: utACK 78f4c8b98eada337346ffb206339c3ebae4ff43b rajarshimaitra: Code review ACK https://github.com/bitcoin/bitcoin/commit/78f4c8b98eada337346ffb206339c3ebae4ff43b lsilva01: Code Review ACK and Tested ACK https://github.com/bitcoin/bitcoin/pull/22383/commits/78f4c8b98eada337346ffb206339c3ebae4ff43b on Ubuntu 20.04 Tree-SHA512: af7db5b98cb2ae4897b28476b2fa243bf7e6f850750d9347062fe8013c5720986d1a3c808f80098e5289bd84b085de03c81a44e584dc28982f721c223651bfe0
2021-07-22refactor: move `GetTransaction(...)` to node/transaction.cppSebastian Falbesoner
can be reviewed with --color-moved
2021-07-16refactor: remove unused assumeutxo methodsJames O'Beirne
After feedback from Russ, I realized that there are some extraneous assumeutxo methods that are not necessary and probably just overly confusing. These include - `Validated*()` - `IsBackgroundIBD()` and they can be removed.
2021-07-16validation: add chainman ref to CChainStateJames O'Beirne
Add an upwards reference to chainstate instances to the owning ChainstateManager. This is necessary because there are a number of `this_chainstate == chainman.ActiveChainstate()` checks that will happen (as a result of assumeutxo) in functions that otherwise don't have an easily-accessible reference to the chainstate's ChainManager.
2021-07-15Move pblocktree global to BlockManagerMarcoFalke
2021-07-15Move LoadBlockIndexDB to BlockManagerMarcoFalke
2021-07-13refactor: move UpdateTip into CChainStateJames O'Beirne
Makes sense and saves on arguments. Co-authored-by: John Newbery <john@johnnewbery.com>
2021-07-13refactor: no mempool arg to GetCoinsCacheSizeStateJames O'Beirne
Unnecessary argument since we can make use of this->m_mempool Co-authored-by: John Newbery <john@johnnewbery.com>
2021-07-13refactor: move UpdateMempoolForReorg into CChainStateJames O'Beirne
Allows fewer arguments and simplification of call sites. Co-authored-by: John Newbery <john@johnnewbery.com>
2021-07-13validation: make CChainState::m_mempool optionalJames O'Beirne
Since we now have multiple chainstate objects, only one of them is active at any given time. An active chainstate has a mempool, but there's no point to others having one. This change will simplify proposed assumeutxo semantics. See the discussion here: https://github.com/bitcoin/bitcoin/pull/15606#pullrequestreview-692965905 Co-authored-by: Russell Yanofsky <russ@yanofsky.org>
2021-07-03prefer to use txindex if available for GetTransactionJameson Lopp
Fixes #22382
2021-06-30[move-only] Move ComputeBlockVersion from validation to versionbitsAnthony Towns
2021-06-30[refactor] Add versionbits deployments to deploymentstatus.hAnthony Towns
Adds support for versionbits deployments to DeploymentEnabled, DeploymentActiveAfter and DeploymentActiveAt. Also moves versionbitscache from validation to deploymentstatus.
2021-06-29[refactor] Add deploymentstatus.hAnthony Towns
Provides DeploymentEnabled, DeploymentActiveAt, and DeploymentActiveAfter helpers for checking the status of buried deployments. Can be overloaded so the same syntax works for non-buried deployments, allowing future soft forks to be changed from signalled to buried deployments without having to touch the implementation code. Replaces IsWitnessEnabled and IsScriptWitnessEnabled.
2021-06-13refactor: Remove chainparams arg from CChainState member functionsMarcoFalke
Passing this is confusing and redundant with the m_params member.
2021-06-13refactor: Remove ::Params() global from inside CChainState member functionsMarcoFalke
It is confusing and verbose to repeatedly access the global when a member variable can simply refer to it.
2021-06-10validation: Farewell, global Chainstate!Carl Dong
2021-06-10qt/test: Reset chainman in ~ChainstateManager insteadCarl Dong
There are some mutable, global state variables that are currently reset by UnloadBlockIndex such as pindexBestHeader which should be cleaned up whenever the ChainstateManager is unloaded/reset/destructed/etc. Not cleaning them up leads to bugs like a use-after-free that happens like so: 1. At the end of a test, ChainstateManager is destructed, which also destructs BlockManager, which calls BlockManager::Unload to free all CBlockIndexes in its BlockMap 2. Since pindexBestHeader is not cleaned up, it now points to an invalid location 3. Another test starts to init, and calls LoadGenesisBlock, which calls AddToBlockIndex, which compares the genesis block with an invalid location 4. Cute puppies perish by the hundreds Previously, for normal codepaths (e.g. bitcoind), we relied on the fact that our program will be unloaded by the operating system which effectively resets these variables. The one exception is in QT tests, where these variables had to be manually reset. Since now ChainstateManager is no longer a global, we can just put this logic in its destructor to make sure that callers are always correct. Over time, we should probably move these mutable global state variables into ChainstateManager or CChainState so it's easier to reason about their lifecycles.
2021-06-10Merge bitcoin/bitcoin#22084: package testmempoolaccept followupsfanquake
ee862d6efb4c3c01e55f0d5d7a82cce75323cf40 MOVEONLY: context-free package policies (glozow) 5cac95cd15da04b83afa1d31a43be9f5b30a1827 disallow_mempool_conflicts -> allow_bip125_replacement and check earlier (glozow) e8ecc621be6afd3252c0f8147e42c3b4918f7f46 [refactor] comment/naming improvements (glozow) 7d91442461776e2ef240d7885f768b624de341a7 [rpc] reserve space in txns (glozow) 6c5f19d9c4d267c54f4dbc4f9d65370ff1e0625b [package] static_assert max package size >= max tx size (glozow) Pull request description: various followups from #20833 ACKs for top commit: jnewbery: utACK ee862d6efb4c3c01e55f0d5d7a82cce75323cf40 ariard: Code Review ACK ee862d6 Tree-SHA512: 96ecb41f7bbced84d4253070f5274b7267607bfe4033e2bb0d2f55ec778cc41e811130b6321131e0418b5835894e510a4be8a0f822bc9d68d9224418359ac837
2021-06-03doc: Various validation doc fixupsMarcoFalke
* Rename RewindBlockIndex -> NeedsRedownload (follow-up to commit d831e711cab83c70bf2ded62fe33f484844e73dd) * Fix typos * Inline comments about faking chain data to avoid duplicating them
2021-06-02[refactor] comment/naming improvementsglozow
2021-05-31validation: change ProcessNewBlock() to take a CBlock referencefanquake
Update ProcessNewBlock arguments to newer style.
2021-05-27Merge bitcoin/bitcoin#20833: rpc/validation: enable packages through ↵W. J. van der Laan
testmempoolaccept 13650fe2e527bf0cf5d977bf5f3f1563b853ecdc [policy] detect unsorted packages (glozow) 9ef643e21b44f99f4bce54077788d0ad4d81f7cd [doc] add release note for package testmempoolaccept (glozow) c4259f4b7ee23ef6e0ec82c5d5b9dfa9cadd5bed [test] functional test for packages in RPCs (glozow) 9ede34a6f20378e86c5289ebd20dd394a5915123 [rpc] allow multiple txns in testmempoolaccept (glozow) ae8e6df709ff3d52b8e9918e09cacb64f83ae379 [policy] limit package sizes (glozow) c9e1a26d1f17c8b98632b7796ffa8f8788b5a83c [fuzz] add ProcessNewPackage call in tx_pool fuzzer (glozow) 363e3d916cc036488783bb4bdcfdd3665aecf711 [test] unit tests for ProcessNewPackage (glozow) cd9a11ac96c01e200d0086b2f011f4a614f5a705 [test] make submit optional in CreateValidMempoolTransaction (glozow) 2ef187941db439c5b3e529f08b6ab153ff061fc5 [validation] package validation for test accepts (glozow) 578148ded62828a9820398165c41670f4dbb523d [validation] explicit Success/Failure ctors for MempoolAcceptResult (glozow) b88d77aec5e7bef5305a668d15031351c0548b4d [policy] Define packages (glozow) 249f43f3cc52b0ffdf2c47aad95ba9d195f6a45e [refactor] add option to disable RBF (glozow) 897e348f5987eadd8559981a973c045c471b3ad8 [coins/mempool] extend CCoinsViewMemPool to track temporary coins (glozow) 42cf8b25df07c45562b7210e0e15c3fd5edb2c11 [validation] make CheckSequenceLocks context-free (glozow) Pull request description: This PR enables validation dry-runs of packages through the `testmempoolaccept` RPC. The expectation is that the results returned from `testmempoolaccept` are what you'd get from test-then-submitting each transaction individually, in that order (this means the package is expected to be sorted in topological order, for now at least). The validation is also atomic: in the case of failure, it immediately halts and may return "unfinished" `MempoolAcceptResult`s for transactions that weren't fully validated. The API for 1 transaction stays the same. **Motivation:** - This allows you to test validity for transaction chains (e.g. with multiple spending paths and where you don't want to broadcast yet); closes #18480. - It's also a first step towards package validation in a minimally invasive way. - The RPC commit happens to close #21074 by clarifying the "allowed" key. There are a few added restrictions on the packages, mostly to simplify the logic for areas that aren't critical to main package use cases: - No package can have conflicts, i.e. none of them can spend the same inputs, even if it would be a valid BIP125 replacement. - The package cannot conflict with the mempool, i.e. RBF is disabled. - The total count of the package cannot exceed 25 (the default descendant count limit), and total size cannot exceed 101KvB (the default descendant size limit). If you're looking for review comments and github isn't loading them, I have a gist compiling some topics of discussion [here](https://gist.github.com/glozow/c3acaf161c95bba491fce31585b2aaf7) ACKs for top commit: laanwj: Code review re-ACK 13650fe2e527bf0cf5d977bf5f3f1563b853ecdc jnewbery: Code review ACK 13650fe2e527bf0cf5d977bf5f3f1563b853ecdc ariard: ACK 13650fe Tree-SHA512: 8c5cbfa91a6c714e1c8710bb281d5ff1c5af36741872a7c5df6b24874d6272b4a09f816cb8a4c7de33ef8e1c2a2c252c0df5105b7802f70bc6ff821ed7cc1a2f
2021-05-24[validation] package validation for test acceptsglozow
Only allow test accepts for now. Use the CoinsViewTemporary to keep track of coins created by each transaction so that subsequent transactions can spend them. Uncache all coins since we only ever do test accepts (Note this is different from ATMP which doesn't uncache for valid test_accepts) to minimize impact on the coins cache. Require that the input txns have no conflicts and be ordered topologically. This commit isn't able to detect unsorted packages.
2021-05-20[validation] explicit Success/Failure ctors for MempoolAcceptResultglozow
Makes code more clear and prevents accidentally calling the wrong ctor.
2021-05-20[validation] make CheckSequenceLocks context-freeglozow
Allow CheckSequenceLocks to use heights and coins from any CoinsView and CBlockIndex provided. This means that CheckSequenceLocks() doesn't need to hold the mempool lock or cs_main. The caller is responsible for ensuring the CoinsView and CBlockIndex are consistent before passing them in. The typical usage is still to create a CCoinsViewMemPool from the mempool and grab the CBlockIndex from the chainstate tip.
2021-05-19Remove -feefilter optionamadeuszpawlik
Feefilter option is debug only and it isn't used in any tests, it's wasteful to check this option for every peer on every iteration of the message handler loop. refs #21545
2021-05-11refactor: Avoid magic value of all-zeros in assumeutxo base_blockhashMarcoFalke
Just use std::optional
2021-05-05Merge bitcoin/bitcoin#21727: refactor: Move more stuff to blockstorageW. J. van der Laan
fa09a9eac8d8ab65ce4064c35a9f21349a644982 style: Add { } to multi-line if (MarcoFalke) fadafab83379ff10d86ada179c6f9641d19464fe move-only: Move functions to blockstorage (MarcoFalke) fa7e64d58615fffea91cd64dc4a2790221ceff0a move-only: Move constants to blockstorage (MarcoFalke) fa247a327fc7c7cea6bc8f93637b8babd3015ffa refactor: Move block storage globals to blockstorage (MarcoFalke) fa81c30c6f1adac79517c958090db174eb6aeda2 refactor: Move pruning/reindex/importing globals to blockstorage (MarcoFalke) Pull request description: See #21575 ACKs for top commit: Sjors: ACK fa09a9eac8d8ab65ce4064c35a9f21349a644982 kiminuo: ACK fa09a9e laanwj: Code review ACK fa09a9eac8d8ab65ce4064c35a9f21349a644982 promag: Code review ACK fa09a9eac8d8ab65ce4064c35a9f21349a644982. Since last review Tree-SHA512: 2eb6962ff44da6b77f3058fc02ec66ab742e25ae8dcc8ec62b062896571910d43ca7c4bb16fb3ccb5e5245195b8dec6384b6c8d442fa97ca28d93bdff347d677
generated by cgit v1.2.3 (git 2.26.2) at 2025-01-27 18:08:08 +0000