aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2023-09-19fuzz: Add missing PROVIDE_FUZZ_MAIN_FUNCTION guard to __AFL_FUZZ_INITMarcoFalke
2023-09-19rpc: Introduce getaddrmaninfo for count of addresses stored in new/tried tablestratospher
2023-09-19ci: Reintroduce fixed "test-each-commit" jobHennadii Stepanov
The new job checks at most 6 commits of a pull request, excluding the top one.
2023-09-16Merge bitcoin/bitcoin#28479: build: use _LIBCPP_ENABLE_DEBUG_MODE over ↵fanquake
ENABLE_ASSERTIONS 4a825039a509c43ba20b2cd7aab448b3be16bcc3 build: use _LIBCPP_ENABLE_DEBUG_MODE over ENABLE_ASSERTIONS (fanquake) Pull request description: `_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, which also performs more checks than safe mode: > Enables the debug mode which contains all the checks from the hardened mode and additionally more expensive checks that may affect the complexity of algorithms. The debug mode is intended to be used for testing, not in production. Mutually exclusive with `_LIBCPP_ENABLE_HARDENED_MODE` and `_LIBCPP_ENABLE_SAFE_MODE`. See https://libcxx.llvm.org/Hardening.html. Related to #28476. ACKs for top commit: MarcoFalke: lgtm ACK 4a825039a509c43ba20b2cd7aab448b3be16bcc3 🙏 Tree-SHA512: ca52603f86214e8e9350bd2b2baa44fbde0f72f1b186da7aecd8690256dff5b2be75fe89383158298a6f683bbd6ae0dff528d2ba4cc5ece1f56cfbdee0e1dc5d
2023-09-16Merge bitcoin/bitcoin#28489: tests: fix incorrect assumption in v2transport_testfanquake
3f4e1bb9ae5ee43da9503da37b9894037d613c6d tests: fix incorrect assumption in v2transport_test (Pieter Wuille) Pull request description: One part of the current `v2transport_test` introduced in #28196 assumes that if a bit gets modified in a message, failure should instantly be detected after sending that message. This is not correct in case the length descriptor is modified, as that may cause the receiver to need more data first. Fix this by sending more messages until failure actually occurs. Discovered in https://github.com/bitcoin/bitcoin/pull/27495#issuecomment-1719934041. ACKs for top commit: theStack: ACK 3f4e1bb9ae5ee43da9503da37b9894037d613c6d Tree-SHA512: faa90bf91996cbaaef62d764e746cb222eaf6796316b0d0e13709e528750b7c0ef09172f7fecfe814dbb8c136c5259f65ca1ac79318e6768a0bfc4e626a63249
2023-09-15doc: add release noteismaelsadeeq
2023-09-15test: check `descriptorprocesspsbt` return hex encoded txismaelsadeeq
Test that if the processed psbt is complete the hex encoded tx is returned and remove unneccessary rpc call to finalize the psbt.
2023-09-15rpc: `descriptorprocesspsbt` return hex encoded txismaelsadeeq
If processed psbt is complete return hex encoded network transaction in the output.
2023-09-15Refactor: Replace 'isMockableChain' with inline 'ChainType' check for ↵Tim Neubauer
'submitpackage'
2023-09-15Merge bitcoin/bitcoin#28473: refactor: Serialization parameter cleanupsfanquake
fb6a2ab63e310d8b600352ef41aab6dafccfbff0 scripted-diff: use SER_PARAMS_OPFUNC (Anthony Towns) 5e5c8f86b60a8018e8801fb44bbe56ce97d9deef serialize: add SER_PARAMS_OPFUNC (Anthony Towns) 33203f59b482bddfe0bbe7d497cb8731ce8334a4 serialize: specify type for ParamsWrapper not ref (Anthony Towns) bf147bfffa1afb11721f30e83eec1fa829f64d5f serialize: move ser_action functions out of global namespace (Anthony Towns) Pull request description: Cleanups after #25284: * ser_action namespacing - https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1316189977 * make reference implicit - https://github.com/bitcoin/bitcoin/pull/25284#discussion_r1316277030 * function notation - https://github.com/bitcoin/bitcoin/pull/25284#issuecomment-1710714821 ACKs for top commit: MarcoFalke: lgtm ACK fb6a2ab63e310d8b600352ef41aab6dafccfbff0 💨 TheCharlatan: ACK fb6a2ab63e310d8b600352ef41aab6dafccfbff0 Tree-SHA512: aacca2ee9cfec360ade6b394606e13d1dfe05bc29c5fbdd48a4e6992bd420312d4ed0d32218d95c560646af326e9977728dc2e759990636298e326947f6f9526
2023-09-15build: remove dmg dependenciesfanquake
2023-09-15macdeploy: remove DMG generation from deploy scriptfanquake
2023-09-15build: produce a .zip for macOS distributionHennadii Stepanov
Instead of a .dmg. Co-authored-by: fanquake <fanquake@gmail.com>
2023-09-15build: add -zip option to macdeployqtplusfanquake
This zips the app bundle in /dist.
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.
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 10:59:00 +0000