aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-09-15Return CAutoFile from BlockManager::Open*File()MarcoFalke
This is a refactor.
2023-09-15Make BufferedFile to be a CAutoFile wrapperMarcoFalke
This refactor allows to forward some calls to the underlying CAutoFile, instead of re-implementing the logic in the buffered file.
2023-09-15refactor: Drop unused fclose() from BufferedFileMarcoFalke
This was only explicitly used in the tests, where it can be replaced by wrapping the original raw file pointer into a CAutoFile on creation and then calling CAutoFile::fclose(). Also, it was used in LoadExternalBlockFile(), where it can also be replaced by the (implicit call to the) CAutoFile destructor after wrapping the original raw file pointer in a CAutoFile.
2023-09-15tests: fix incorrect assumption in v2transport_testPieter Wuille
2023-09-15Merge bitcoin/bitcoin#28476: ci: LLVM 17 for MSAN jobsfanquake
a241d6069cf0542acdd8ec6be63724da19f10720 ci: use LLVM 17.0.0 in MSAN jobs (fanquake) Pull request description: See https://libcxx.llvm.org/Hardening.html as well as https://discourse.llvm.org/t/rfc-removing-the-legacy-debug-mode-from-libc/71026. ACKs for top commit: MarcoFalke: review ACK a241d6069cf0542acdd8ec6be63724da19f10720 Tree-SHA512: c374dabf307fe762be0da96f63695a150f6018c1468fe9414fad23f74f5818bbf7a5a699e109084e31467482a900cfebf1d5835821e4da94aa310b2c9570749c
2023-09-15Merge bitcoin/bitcoin#28480: fuzz: Don't use afl++ deferred forkserver modefanquake
508d05f8a7b511dd53f543df8899813487eb03e5 [fuzz] Don't use afl++ deferred forkserver mode (dergoegge) Pull request description: Fixes #28469 This makes our afl++ harness essentially behave like libFuzzer, with the exception that the whole program does fully reset every 100000 iterations. 100000 is somewhat arbitrary and we could also go with `std::numeric_limits<unsigned in>::max()` but a smaller limit does allow for the occasional reset to counter act some amount of instability in the fuzzing loop (e.g. non-determinism, statefulness). It's a bit of a shame to do this just for the targets whose initial state can't be forked (e.g. threads) because other targets do benefit from not having to redo the state setup. An alternative would be https://github.com/bitcoin/bitcoin/issues/28469#issuecomment-1717526774: ``` If the goal is to be maximally performant, the fork would need to happen for each fuzz target specifically. I guess it can be achieved by wrapping __AFL_INIT(); into a helper function and then require all fuzz target initialize() to call it? ``` ACKs for top commit: MarcoFalke: lgtm ACK 508d05f8a7b511dd53f543df8899813487eb03e5 Tree-SHA512: d9fe94e2e3198795f8fb58f67eb383531a534bcd4ec75a1f0ae6ccb5531863dbc09800bb7d77536417745c4c8bc49a4f84dcc959918b27d4997a270eeacb0e7e
2023-09-15Merge bitcoin/bitcoin#28452: Do not use std::vector = {} to release memoryfanquake
3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e Do not use std::vector = {} to release memory (Pieter Wuille) Pull request description: It appears that invoking `v = {};` for an `std::vector<...> v` is equivalent to `v.clear()`, which does not release its allocated memory. There are a number of places in the codebase where it appears to be used for that purpose however (mostly written by me). Replace those with `std::vector<...>{}.swap(v);` (using a helper function `ClearShrink` in util/vector.h). To explain what is going on: `v = {...};` is equivalent in general to `v.operator=({...});`. For many types, the `{}` is converted to the type of `v`, and then assigned to `v` - which for `std::vector` would ordinarily have the effect of clearing its memory (constructing a new empty vector, and then move-assigning it to `v`). However, since `std::vector<T>` has an `operator=(std::initializer_list<T>)` defined, it has precedence (since no implicit conversion is needed), and with an empty list, that is equivalent to `clear()`. I did consider using `v = std::vector<T>{};` as replacement for `v = {};` instances where memory releasing is desired, but it appears that it does not actually work universally either. `V{}.swap(v);` does. ACKs for top commit: ajtowns: utACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e stickies-v: ACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e theStack: Code-review ACK 3fcd7fc7ff563bdc0e2bba66b4cbe72d898c876e Tree-SHA512: 6148558126ec3c8cfd6daee167ec1c67b360cf1dff2cbc132bd71768337cf9bc4dda3e5a9cf7da4f7457d2123288eeba77dd78f3a17fa2cfd9c6758262950cc5
2023-09-14Merge bitcoin/bitcoin#26152: Bump unconfirmed ancestor transactions to ↵Andrew Chow
target feerate f18f9ef4d31c70e2d71ab90a24511692821418c3 Amend bumpfee for inputs with overlapping ancestry (Murch) 2e35e944dab09eff30952233f8dfc0b12c4553d5 Bump unconfirmed parent txs to target feerate (Murch) 3e3e05241128f68cf12f73ee06ff997395643885 coinselection: Move GetSelectionWaste into SelectionResult (Andrew Chow) c57889da6650715f3e1153b6104bbdae15fcac90 [node] interface to get bump fees (glozow) c24851be945b2a633ee44ed3c8a501eee5580b62 Make MiniMinerMempoolEntry fields private (Murch) ac6030e4d8f7d578cd4a8593f41189efca548064 Remove unused imports (Murch) d2f90c31ef3b8dee5a3e0804ecc62fa1cfec7cd5 Fix calculation of ancestor set feerates in test (Murch) a1f7d986e0211e54e21a1d4a570e5f15294dca72 Match tx names to index in miniminer overlap test (Murch) Pull request description: Includes some commits to address follow-ups from #27021: https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1554675156 Reduces the effective value of unconfirmed UTXOs by the fees necessary to bump their ancestor transactions to the same feerate. While the individual UTXOs always account for their full ancestry before coin-selection, we can correct potential overestimates with a second pass where we establish the ancestry and bump fee for the whole input set collectively. Fixes #9645 Fixes #9864 Fixes #15553 ACKs for top commit: S3RK: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 ismaelsadeeq: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 achow101: ACK f18f9ef4d31c70e2d71ab90a24511692821418c3 brunoerg: crACK f18f9ef4d31c70e2d71ab90a24511692821418c3 t-bast: ACK https://github.com/bitcoin/bitcoin/pull/26152/commits/f18f9ef4d31c70e2d71ab90a24511692821418c3, I reviewed the latest changes and run e2e tests against eclair, everything looks good :+1: Tree-SHA512: b65180c4243b1f9d13c311ada7a1c9f2f055d530d6c533b78c2068b50b8c29ac1321e89e85675b15515760d4f1b653ebd9da77b37c7be52d9bc565a3538f0aa6
2023-09-14Remove MemPoolAccept::m_limits, only have local copies for carveoutsGreg Sanders
2023-09-14Merge bitcoin/bitcoin#27850: test: Add unit & functional test coverage for ↵Andrew Chow
blockstore de8f9123afbecc3b4f59fa80af8148bc865d0588 test: cover read-only blockstore (Matthew Zipkin) 5c2185b3b624ce87320ec16412f98ab591a5860c ci: enable chattr +i capability inside containers (Matthew Zipkin) e573f2420244c583e218f51cd0d3a3cac6731003 unit test: add coverage for BlockManager (Matthew Zipkin) Pull request description: This PR adds unit and functional tests to cover the behavior described in #2039. In particular, that bitcoind will crash on startup if a reindex is requested but the `blk` files are read-only. Eventually this behavior can be updated with https://github.com/bitcoin/bitcoin/pull/27039. This PR just commits the test coverage from #27039 as suggested in https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1584915782 ACKs for top commit: jonatack: ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 modulo suggestions in https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1319010039, tested on macOS, but not on Linux for the Linux-related change in the last push achow101: ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 MarcoFalke: lgtm ACK de8f9123afbecc3b4f59fa80af8148bc865d0588 📶 Tree-SHA512: b9bd684035dcea11c901b649fc39f397a2155a9a8459f3348e67947e387e45312fddeccb52981aef486f8a31deebb5356a7901c1bb94b78f82c24192a369af73
2023-09-14doc: Remove confusing assert linterMarcoFalke
2023-09-14test: cover read-only blockstoreMatthew Zipkin
Co-authored-by: Andrew Chow <github@achow101.com>
2023-09-14[fuzz] Don't use afl++ deferred forkserver modedergoegge
Deferring the forkserver initialization doesn't make sense for some of our targets since they involve state that can't be forked (e.g. threads). We therefore remove the use of __AFL_INIT entirely. We also increase the __AFL_LOOP count to 100000. Our fuzz targets are meant to all be deterministic and stateless therefore this should be fine.
2023-09-14build: use _LIBCPP_ENABLE_DEBUG_MODE over ENABLE_ASSERTIONSfanquake
`_LIBCPP_ENABLE_ASSERTIONS` is deprecated, and will be removed. [See (from libc++ __config in main)](https://github.com/llvm/llvm-project/blob/b57df9fe9a1a230f277d671bfa0884bbda9fc1c5/libcxx/include/__config#L205-L209): > TODO(hardening): remove this in LLVM 19. > This is for backward compatibility -- make enabling `_LIBCPP_ENABLE_ASSERTIONS` (which predates hardening modes) > equivalent to setting the safe mode. > ifdef _LIBCPP_ENABLE_ASSERTIONS > warning "_LIBCPP_ENABLE_ASSERTIONS is deprecated, please use _LIBCPP_ENABLE_SAFE_MODE instead." From LLVM 17, `_LIBCPP_ENABLE_DEBUG_MODE` can be used instead. See https://libcxx.llvm.org/Hardening.html. Related to #28476.
2023-09-14Merge bitcoin/bitcoin#28478: ci: Temporarily disable test-each-commitfanquake
fa2cb2f5d3125451270dc5ec6c86a6756afeb230 Revert "Merge bitcoin/bitcoin#28279: ci: Add test-each-commit task" (MarcoFalke) Pull request description: This should unbreak the GHA CI for now, and allow someone to fix the task in a follow-up. The issue is https://github.com/bitcoin/bitcoin/pull/28279#issuecomment-1719324530 . If no one fixes it, it can be replaced by a Cirrus CI self-hosted runner. ACKs for top commit: sipa: ACK fa2cb2f5d3125451270dc5ec6c86a6756afeb230 dergoegge: ACK fa2cb2f5d3125451270dc5ec6c86a6756afeb230 Tree-SHA512: d9c70915b3fb676f44054cee8033286910682ff6819d4ee71e432f7efb3043a0a9507f0052b6c24e3b0c431875f6337b1adccc5b48432970d7c29c6d6b00a2d7
2023-09-14Merge bitcoin/bitcoin#28460: fuzz: Use afl++ shared-memory fuzzingfanquake
97e2e1d641016cd7b74848b9560e3771f092c1ea [fuzz] Use afl++ shared-memory fuzzing (dergoegge) Pull request description: Using shared-memory is faster than reading from stdin, see https://github.com/AFLplusplus/AFLplusplus/blob/7d2122e0596132f9344a5d0896020ebc79cd33db/instrumentation/README.persistent_mode.md ACKs for top commit: MarcoFalke: review ACK 97e2e1d641016cd7b74848b9560e3771f092c1ea Tree-SHA512: 7e71b5f84835e41531c19ee959be2426da245869757de8e5dd1c730ae83ead650e2ef75f4d594d7965f661821a4ffbd27be84d3ce623702991501b34a8d02fc3
2023-09-14Revert "Merge bitcoin/bitcoin#28279: ci: Add test-each-commit task"MarcoFalke
This reverts commit 744e0e36703e26d06bc5cd1ef36a1c8568e71d05, reversing changes made to 8209e86eeb4ceb6dd0e06c45fb3c799bb42834ab.
2023-09-14Merge bitcoin/bitcoin#28465: ci: clang-17 for fuzz and tsanfanquake
fa23c9aa7c7d60ff4f914447e7d37dedca85e171 ci: clang-17 for fuzz and tsan (MarcoFalke) Pull request description: Bump clang in CI from 16 to 17, to: * Bump the CI "EOL" from Jan 2024 to July 2024, by bumping from Ubuntu lunar to mantic * Test, ensure compatibility, and make use of any new sanitizer features in clang-17 ACKs for top commit: dergoegge: utACK fa23c9aa7c7d60ff4f914447e7d37dedca85e171 Tree-SHA512: 25b625b98341e6e9c56e0b0c080347c2225ea1b0b7bf0e91068f4fc369eaa53fa380b636ffd8ecf1fc7426a1082539a493e176afa3531f1ec86059080a2646de
2023-09-14ci: use LLVM 17.0.0 in MSAN jobsfanquake
See https://libcxx.llvm.org/Hardening.html as well as https://discourse.llvm.org/t/rfc-removing-the-legacy-debug-mode-from-libc/71026.
2023-09-14Merge bitcoin/bitcoin#28423: kernel: Remove protocol.h/netaddress.h/compat.h ↵fanquake
from kernel headers d5067651991f3e6daf456ba13c7036ddc4545352 [refactor] Remove compat.h from kernel headers (TheCharlatan) 36193af47c8dcff53e59498c416b85b59e0d0f91 [refactor] Remove netaddress.h from kernel headers (TheCharlatan) 2b08c55f01996e0b05763f05eac50b83ba9d5a8e [refactor] Add CChainParams member to CConnman (TheCharlatan) f0d1d8b35c3aa9f2f923f74e3dbbf1e5ece4cd2f [refactor] Add missing includes for next commit (TheCharlatan) 534b314a7401d44f51aabd4565f97be9ee411740 kernel: Move MessageStartChars to its own file (TheCharlatan) 9be330b654cfbd792620295f3867f592059d6a7a [refactor] Define MessageStartChars as std::array (TheCharlatan) 37e2b011136ca1cf00dfb9e575d12f0d035a6a2c [refactor] Allow std::array<std::byte, N> in serialize.h (MarcoFalke) Pull request description: This removes the non-consensus critical `protocol.h` and `netaddress.h` headers from the kernel headers. With this patch, they are no longer required to include in order to use the libbitcoinkernel library. This also allows for the removal of the `compat.h` header from the kernel headers. As an added future benefit it also reduces the number of of kernel headers that include the platform specific `bitcoin-config.h`. For those interested, the currently required kernel headers can be inspected visually with the [sourcetrail](https://github.com/CoatiSoftware/Sourcetrail) tool by looking at the required includes of `bitcoin-chainstate.cpp`. --- This is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587), namely its stage 1 step 3: Decouple most non-consensus headers from libbitcoinkernel. ACKs for top commit: stickies-v: re-ACK d506765 hebasto: ACK d5067651991f3e6daf456ba13c7036ddc4545352. ajtowns: utACK d5067651991f3e6daf456ba13c7036ddc4545352 MarcoFalke: lgtm ACK d5067651991f3e6daf456ba13c7036ddc4545352 🍛 Tree-SHA512: 6f90ea510a302c2927e84d16900e89997c39b8ff3ce9d4effeb8a134bd29cc52bd9e81e51aaa11f7496bad00025b78a58b88c5a9e0bb3f4ebbe9a76309215fb7
2023-09-14Merge bitcoin/bitcoin#28279: ci: Add test-each-commit taskfanquake
fa5356cd49facf195447f0f5921dce1fa53cb25d ci: Limit test-each-commit to --max-count=6 (MarcoFalke) fafcd2e9ef1209d614de5763a2733098537919dd ci: Add test-each-commit task (MarcoFalke) Pull request description: Currently, if a pull request has more than one commit, previous commits may fail to compile, or may fail the tests. This is problematic, because it breaks git-bisect, or worse. Fix this by adding a CI task for this. ACKs for top commit: jonatack: ACK fa5356cd49facf195447f0f5921dce1fa53cb25d dergoegge: utACK fa5356cd49facf195447f0f5921dce1fa53cb25d hebasto: ACK fa5356cd49facf195447f0f5921dce1fa53cb25d Tree-SHA512: 304eff5545501ee499b881f0b0a0c1fe32a7c9f00d96b45bba731af08ac5ca917cef223f5c3d346e30c11a3e6e12e1da297929d3caea9644f3ec1de25dd97c37
2023-09-14Merge bitcoin/bitcoin#28458: refactor: Remove unused GetType() from ↵fanquake
CBufferedFile and CAutoFile fa19c914f7fe7be127c0fb330b41ff7c091f40aa scripted-diff: Rename CBufferedFile to BufferedFile (MarcoFalke) fa2f2413b87f5fc1e5c92bf510beebdcd0091714 Remove unused GetType() from CBufferedFile and CAutoFile (MarcoFalke) 5c2b3cd4b856f1bb536daaf7f576b1b1b42293ca dbwrapper: Use DataStream for batch operations (TheCharlatan) Pull request description: This refactor is required for https://github.com/bitcoin/bitcoin/pull/28052 and https://github.com/bitcoin/bitcoin/pull/28451 Thus, split it out. ACKs for top commit: ajtowns: utACK fa19c914f7fe7be127c0fb330b41ff7c091f40aa TheCharlatan: ACK fa19c914f7fe7be127c0fb330b41ff7c091f40aa Tree-SHA512: d9c232324702512e45fd73ec3e3170f1e8a8c8f9c49cb613a6b693a9f83358914155527ace2517a2cd626a0cedcada56ef70a2a7812edafb1888fd6765eebba2
2023-09-14scripted-diff: use SER_PARAMS_OPFUNCAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i 's/WithParams(\(CAddress::V[12]_[A-Z]*\) *, */\1(/g' $(git grep -l 'WithParams' src/) sed -i 's/WithParams(\(CNetAddr::V[12]\) *, */\1(/g' $(git grep -l 'WithParams' src/) sed -i 's@\(CNetAddr::V1.CService{}.*\) //@\1 //@' src/test/util/net.cpp -END VERIFY SCRIPT-
2023-09-14serialize: add SER_PARAMS_OPFUNCAnthony Towns
2023-09-14serialize: specify type for ParamsWrapper not refAnthony Towns
2023-09-14serialize: move ser_action functions out of global namespaceAnthony Towns
2023-09-13Amend bumpfee for inputs with overlapping ancestryMurch
At the end of coin selection reduce the fees by the difference between the individual bump fee estimates and the collective bump fee estimate.
2023-09-13Bump unconfirmed parent txs to target feerateMurch
When a transaction uses an unconfirmed input, preceding this commit it would not consider the feerate of the parent transaction. Given a parent transaction with a lower ancestor feerate, this resulted in the new transaction's ancestor feerate undershooting the target feerate. This commit changes how we calculate the effective value of unconfirmed UTXOs. The effective value of unconfirmed UTXOs is decreased by the fee necessary to bump its ancestry to the target feerate. This also impacts the calculation of the waste metric: since the estimate for the current fee is increased by the bump fees, unconfirmed UTXOs current fees appear less favorable compared to their unchanged long term fees. This has one caveat: if multiple UTXOs have overlapping ancestries, each of their individual estimates will account for bumping all ancestors.
2023-09-13coinselection: Move GetSelectionWaste into SelectionResultAndrew Chow
GetSelectionWaste will need to access more context within a selection result, and so should be a private member function rather than a static function. It's only use outside of SelectionResult was for tests which have now been updated to just make a SelectionResult. Co-authored-by: Murch <murch@murch.one>
2023-09-13[node] interface to get bump feesglozow
2023-09-13Make MiniMinerMempoolEntry fields privateMurch
Follow-up from #27021: accessing of fields in MiniMinerMempoolEntry was done inconsistently. Even though we had a getter, we would directly write to the fields when we needed to update them. This commits sets the fields to private and introduces a method for updating the ancestor information in transactions using the same method name as used for Mempool Entries.
2023-09-13Remove unused importsMurch
Follow-up from #27021
2023-09-13Fix calculation of ancestor set feerates in testMurch
Follow-up from #27021. Also included is an ASCII art visualization of the test’s transaction topology by theStack. Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
2023-09-13Match tx names to index in miniminer overlap testMurch
Follow-up from #27021: In the prior commit, the vector started counting at 0, but the transaction names started with 1. This commit matches the names to the transactions’ vector indices for better readability. Co-authored-by: theStack <sebastian.falbesoner@gmail.com>
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[test] mempool coins disappearing mid-package evaluationglozow
Test for scenario(s) outlined in PR 28251. Test what happens when a package transaction spends a mempool coin which is fetched and then disappears mid-package evaluation due to eviction or replacement.
2023-09-13[refactor] split setup in mempool_limit testglozow
We want to be able to re-use fill_mempool so that none of the tests affect each other. Change the logs from info to debug because they are otherwise repeated many times in the test output.
2023-09-13[test framework] add ability to spend only confirmed utxosglozow
Useful to ensure that the topologies of packages/transactions are as expected, preventing bugs caused by having unexpected mempool ancestors.
2023-09-13[validation] don't LimitMempoolSize in any subpackage submissionsglozow
Don't do any mempool evictions until package validation is done, preventing the mempool minimum feerate from changing. Whether we submit transactions separately or as a package depends on whether they meet the mempool minimum feerate threshold, so it's best that the value not change while we are evaluating a package. This avoids a situation where we have a CPFP package in which the parents meet the mempool minimum feerate and are submitted by themselves, but they are evicted before we have submitted the child.
2023-09-13[validation] return correct result when already-in-mempool tx gets evictedglozow
Bug fix: a transaction may be in the mempool when package evaluation begins (so it is added to results_final with MEMPOOL_ENTRY or DIFFERENT_WITNESS), but get evicted due to another transaction submission.
2023-09-13[refactor] back-fill results in AcceptPackageglozow
Instead of populating the last PackageMempoolAcceptResult with stuff from results_final and individual_results_nonfinal, fill results_final and create a PackageMempoolAcceptResult using that one. A future commit will add LimitMempoolSize() which may change the status of each of these transactions from "already in mempool" or "submitted to mempool" to "no longer in mempool". We will change those transactions' results here. A future commit also gets rid of the last AcceptSubPackage outside of the loop. It makes more sense to use results_final as the place where all results end up.
2023-09-13[validation] make PackageMempoolAcceptResult members mutableglozow
After the PackageMempoolAcceptResult is returned from AcceptMultipleTransactions, leave room for results to change due to LimitMempool() eviction.
2023-09-13[validation] add AcceptSubPackage to delegate Accept* calls and clean up m_viewglozow
(1) Call AcceptSingleTransaction when there is only 1 transaction in the subpackage. This avoids calling PackageMempoolChecks() which enforces rules that don't need to be applied for a single transaction, i.e. disabling CPFP carve out. There is a slight change in the error type returned, as shown in the txpackage_tests change. When a transaction is the last one left in the package and its fee is too low, this returns a PCKG_TX instead of PCKG_POLICY. This interface is clearer; "package-fee-too-low" for 1 transaction would be a bit misleading. (2) Clean up m_view and m_viewmempool so that coins created in this sub-package evaluation are not available for other sub-package evaluations. The contents of the mempool may change, so coins that are available now might not be later.
2023-09-13[CCoinsViewMemPool] track non-base coins and allow Resetglozow
Temporary coins should not be available in separate subpackage submissions. Any mempool coins that are cached in m_view should be removed whenever mempool contents change, as they may be spent or no longer exist.
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-09-13fuzz: Rework addr fuzzingMarcoFalke
* Replace ConsumeDeserializationParams with V1, because V2 is unconditionally checked as well. * Also fuzz CAddress::Format::Disk in the address_deserialize fuzz target.
2023-09-13fuzz: Drop unused params from serialize helpersMarcoFalke
With the ser-type and ser-version going away, it seems unlikely that there is need for them in the future, so just remove them.
2023-09-13make DisconnectedBlockTransactions responsible for its own memory managementglozow
Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2023-09-13MOVEONLY: DisconnectedBlockTransactions to its own fileglozow
This struct is only used in validation + tests and has very little to do with txmempool.
2023-09-13rewrite DisconnectedBlockTransactions as a list + mapglozow
And encapsulate underlying data structures to avoid misuse. It's better to use stdlib instead of boost when we can achieve the same thing. Behavior change: the number returned by DynamicMemoryUsage for the same transactions is higher (due to change in usage or more accurate accounting), which effectively decreases the maximum amount of transactions kept for resubmission in a reorg. Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 04:23:11 +0000