aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2024-06-04Merge bitcoin/bitcoin#28979: wallet, rpc: document and update `sendall` ↵Ava Chow
behavior around unconfirmed inputs 71aae72e1fc998b2629d68a7301d85dc1b65641e test: test sendall does ancestor aware funding (ishaanam) 36757941a05b65c2b61a83820afdf5effd8fc9a2 wallet, rpc: implement ancestor aware funding for sendall (ishaanam) 544131f3fba9ea07fee29f9d3ee0116cd5d8a5b2 rpc, test: test sendall spends unconfirmed change and unconfirmed inputs when specified (ishaanam) Pull request description: This PR: - Adds a functional test that `sendall` spends unconfirmed change - Adds a functional test that `sendall` spends regular unconfirmed inputs when specified by user - Adds ancestor aware funding to `sendall` by using `calculateCombinedBumpFee` and adjusting the effective value accordingly - Adds a functional test for ancestor aware funding in `sendall` ACKs for top commit: S3RK: ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e achow101: ACK 71aae72e1fc998b2629d68a7301d85dc1b65641e furszy: ACK 71aae72e1f Tree-SHA512: acaeb7c65166ce53123a1d6cb5012197202246acc02ef9f37a28154cc93afdbd77c25e840ab79bdc7e0b88904014a43ab1ddea79d5337dc310ea210634ab61f0
2024-06-04Merge bitcoin/bitcoin#28366: Fix waste calculation in SelectionResultAva Chow
bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 Use `exact_target` shorthand in coinselector_tests (Murch) 7aa7e30441fe77bf8e8092916e36b004bbbfe2a7 Fold GetSelectionWaste() into ComputeAndSetWaste() (Murch) Pull request description: PR #26152 moved waste calculation into SelectionResult to be able to correct the waste score on basis of the bump_fee_group_discount for overlapping ancestries. This left two functions with largely overlapping purpose, where one was simply a wrapper of the other. This PR cleans up the overlap, and fixes the double-meaning of `change_cost` where the `GetChange()` function assumed that no change was created when `change_cost` was set to 0. This behavior was exploited in a bunch of tests, but is problematic, because a `change_cost` of 0 is permitted with custom settings for feerate and discard_feerate (i.e. when they’re both 0). ACKs for top commit: achow101: ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 furszy: Code ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 ismaelsadeeq: Code Review ACK bd34dd85e7b8b4cc26d2173d84bbeda2e9c27624 Tree-SHA512: 83a2688d45d719dc61a64b5180fe136107faccf401a59df65245c05d701748a03e85ed56fde8c9b7ef39a3ab54374dd3718c559bda5b3f55dafedfd7fed25161
2024-06-04Merge bitcoin/bitcoin#30211: fuzz: Make FuzzedSock fuzz friendliermerge-script
22d0f1a27ef7733b51b3c2138a8d01713df8f248 [fuzz] Avoid endless waiting in FuzzedSock::{Wait,WaitMany} (marcofleon) a7fceda68bb62fe3d9060fcf52e33b2f64a2acf9 [fuzz] Make peeking through FuzzedSock::Recv fuzzer friendly (dergoegge) 865cdf3692590bc6b1121524fe1bee188788b791 [fuzz] Use fuzzer friendly ConsumeRandomLengthByteVector in FuzzedSock::Recv (dergoegge) Pull request description: `FuzzedSock` has a few issues that block a fuzzer from making progress. See commit messages for details. ACKs for top commit: marcofleon: Tested ACK 22d0f1a27ef7733b51b3c2138a8d01713df8f248 brunoerg: utACK 22d0f1a27ef7733b51b3c2138a8d01713df8f248 Tree-SHA512: 2d66fc94ba58b6652ae234bd1dcd33b7d685b5054fe83e0cd624b053dd51519c23148f43a865ab8c8bc5fc2dc25e701952831b99159687474978a90348faa4c5
2024-06-03Merge bitcoin/bitcoin#30215: doc: JSON-RPC request Content-Type is ↵merge-script
application/json 3c08e11c3ea4499e8d20609e2417cac859b3e98e doc: JSON-RPC request Content-Type is application/json (Luke Dashjr) Pull request description: Specify json content type in RPC examples. Picks up #29946. Which needed rebasing and the commit message fixing, ACKs for top commit: laanwj: ACK 3c08e11c3ea4499e8d20609e2417cac859b3e98e tdb3: ACK for 3c08e11c3ea4499e8d20609e2417cac859b3e98e Tree-SHA512: 770bbbc0fb324cb63628980b13583cabf02e75079851850170587fb6eca41a70b01dcedaf1926bb6488eb9816a3cc6616fe8cee8c4b7e09aa39b7df5834ca0ec
2024-06-03Merge bitcoin/bitcoin#30134: fuzz: add more coverage for `ScriptPubKeyMan`merge-script
e3249f21111f1dd4beb66f10af933c34a36c30ac fuzz: add more coverage for `ScriptPubKeyMan` (brunoerg) Pull request description: This PR adds more coverage for `ScriptPubKeyMan`: - Check `GetKey` and `HasPrivKey` after adding descriptor key. - Cover `GetEndRange` and `GetKeyPoolSize`. - Cover `MarkUnusedAddresses` with the scripts from ScriptPubKeys and `GetMetadata` with the destinations from them. ACKs for top commit: marcofleon: Tested ACK e3249f21111f1dd4beb66f10af933c34a36c30ac. I ran the updated harness for ~9 hours on an empty corpus, generated a coverage report, and checked that the new functions mentioned were hit. Coverage of `scriptpubkeyman.cpp` increased. murchandamus: Tested ACK e3249f21111f1dd4beb66f10af933c34a36c30ac Tree-SHA512: cfab91f6c8401174842e79209c0e9225c08f011fe9b41d0a58bcec716ae4545eaf803867f899ed7b5fbcefea45711f91894e36df082ba19732dd310cd9e61a79
2024-06-03Merge bitcoin/bitcoin#30216: build: Fix building `fuzz` binary on on SunOS / ↵merge-script
illumos 3299abce948f205bb1354993614b669189f9b89f build: Fix building `fuzz` binary on on SunOS / illumos (Hennadii Stepanov) Pull request description: On master branch @ 457e1846d2bf6ef9d54b9ba1a330ba8bbff13091, building the `fuzz` binary fails: ``` $ ./autogen.sh $ ./configure $ gmake -C src test/fuzz/fuzz < snip > CXX test/fuzz/fuzz-http_request.o test/fuzz/http_request.cpp:13:10: fatal error: event2/buffer.h: No such file or directory 13 | #include <event2/buffer.h> | ^~~~~~~~~~~~~~~~~ compilation terminated. gmake: *** [Makefile:17138: test/fuzz/fuzz-http_request.o] Error 1 gmake: Leaving directory '/export/home/hebasto/git/bitcoin/src' ``` The testing system: ``` $ uname -a SunOS openindiana 5.11 illumos-82079dec87 i86pc i386 i86pc ``` This PR fixes this issue. ACKs for top commit: maflcko: ACK 3299abce948f205bb1354993614b669189f9b89f Tree-SHA512: 43048cf0d3db47d71263da179e07225afd901ed2039ee4d17314ff7b581ab36f41282fde3b1210926cecda546320dc573937c564520f61fbb236c2b9914ed0d4
2024-06-03[fuzz] Avoid endless waiting in FuzzedSock::{Wait,WaitMany}marcofleon
Currently, when the FuzzedDataProvider of a FuzzedSock runs out of data, FuzzedSock::Wait and WaitMany will simulate endless waiting as the requested events are never simulated as occured. Fix this by simulating event occurence when ConsumeBool() returns false (e.g. when the data provider runs out). Co-authored-by: dergoegge <n.goeggi@gmail.com>
2024-06-03[fuzz] Make peeking through FuzzedSock::Recv fuzzer friendlydergoegge
FuzzedSock only supports peeking at one byte at a time, which is not fuzzer friendly when trying to receive long data. Fix this by supporting peek data of arbitrary length instead of only one byte.
2024-06-03Merge bitcoin/bitcoin#30167: doc, rpc: Release notes and follow-ups for #29612merge-script
efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 test: Add coverage for txid coins count check when loading snapshot (Fabian Jahr) 6b6084850b8c2ebcdbeecdb406e8732adaa6d23c assumeutxo: Add network magic ctor param to SnapshotMetadata (Fabian Jahr) 1f1f9984555d49f07ae20cb3a8153a177c546beb assumeutxo: Deserialize trailing byte instead of Txid (Fabian Jahr) 359967e310794e0bbdbe2bca38ee440a88bc4f43 doc: Add release notes for #29612 (Fabian Jahr) Pull request description: This adds release notes for #29612 and addresses post-merge review comments. ACKs for top commit: maflcko: utACK efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 theStack: utACK efc1b5be8a4696c0db19ba18316b2d4ed09e10f2 Tree-SHA512: 3b270202e4f7b2576090ef1d970fd54a6840d96fc3621dddd28e888fb8696a97ff69af2e000bcee3b364316ca3f6e2a9b2f1694c6184f0e704dc487823127ce4
2024-06-03Merge bitcoin/bitcoin#30186: fuzz: increase `txorphan` harness stabilitymerge-script
8defc182a31d828ad0f53ebf7e3be9e9cfc42d09 scripted-diff: Replace nNextSweep with m_next_sweep (marcofleon) 0048680467e15037023ceae44bc2dc8309f82f39 increase txorphan harness stability (marcofleon) Pull request description: This moves `nNextSweep` from being a static variable in `LimitOrphans` to being a member of the `TxOrphanage` class. This improves the stability of the `txorphan` fuzz harness, as each orphanage (created every iteration) now has its own value for `nNextSweep`. ACKs for top commit: maflcko: utACK 8defc182a31d828ad0f53ebf7e3be9e9cfc42d09 dergoegge: Code review ACK 8defc182a31d828ad0f53ebf7e3be9e9cfc42d09 glozow: utACK 8defc182a31d828ad0f53ebf7e3be9e9cfc42d09, I can rebase on this pretty easily Tree-SHA512: 54d4a5074def764f6c895308b94e417662d2f21f157925421131745f22743907df59971f4ce545063658cd74ec133792cdd8df96ae3e69af8314e9b0ff899d48
2024-06-02build: Fix building `fuzz` binary on on SunOS / illumosHennadii Stepanov
2024-05-31doc: JSON-RPC request Content-Type is application/jsonLuke Dashjr
Specify json content type in RPC examples
2024-05-31[fuzz] Use fuzzer friendly ConsumeRandomLengthByteVector in FuzzedSock::Recvdergoegge
See comment on FuzzedDataProvider::ConsumeRandomLengthString.
2024-05-30clang-tidy: Add `bugprone-move-forwarding-reference` checkHennadii Stepanov
2024-05-30Merge bitcoin/bitcoin#30049: build, test, doc: Temporarily remove ↵merge-script
Android-related stuff 5deb0b024e14c7c63d405c651d1ca323560a1c21 build, test, doc: Temporarily remove Android-related stuff (Hennadii Stepanov) Pull request description: Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++ (see https://github.com/bitcoin/bitcoin/issues/29360). All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x. This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not used at this moment. ACKs for top commit: vasild: ACK 5deb0b024e14c7c63d405c651d1ca323560a1c21 fanquake: ACK 5deb0b024e14c7c63d405c651d1ca323560a1c21 - given none of this is currently tested/wont compile. Can be revisted in future. Tree-SHA512: 3bc2ccfe881e11cc1d78c27acd6f1d86cfba86821ef3bb5eca2e80d978fdfa13659ec82284dcaadc507e2394524dea91d4b8f81d0030c1cef9708df8be76bf07
2024-05-29scripted-diff: Replace nNextSweep with m_next_sweepmarcofleon
-BEGIN VERIFY SCRIPT- sed -i 's/nNextSweep/m_next_sweep/g' $(git grep -l 'nNextSweep') -END VERIFY SCRIPT- fixing to match style
2024-05-29increase txorphan harness stabilitymarcofleon
initialize variable
2024-05-29Merge bitcoin/bitcoin#30122: bench: enable wallet creation benchmarks on all ↵merge-script
platforms 7c8abf3c2001152423da06d25f9f4906611685ea bench: bugfix, properly release wallet before erasing directory (furszy) Pull request description: Simple fix for #29816. Since the wallet is appended to the global `WalletContext` during creation, merely calling `reset()` on the benchmark shared_pointer is insufficient to destruct the wallet. This no destruction of the wallet object results in keeping the db connection open, which was causes the `fs::remove_all()` failure on Windows. ACKs for top commit: maflcko: utACK 7c8abf3c2001152423da06d25f9f4906611685ea kevkevinpal: utACK [7c8abf3](https://github.com/bitcoin/bitcoin/pull/30122/commits/7c8abf3c2001152423da06d25f9f4906611685ea) hebasto: re-ACK 7c8abf3c2001152423da06d25f9f4906611685ea, I agree with changes since my recent [review](https://github.com/bitcoin/bitcoin/pull/30122#pullrequestreview-2061694682). Tree-SHA512: 279df65bea8f7aa02af0a2efed62dca9bf9b29cb748eb369c602d223e08a8a907dea7b1bffbd3dab91b1656c1d91b18a9a0534bc3f153bd751414b0e6230b3a4
2024-05-29Merge bitcoin/bitcoin#30172: fuzz: Handle missing BDBRO errorsmerge-script
9ddf39dd87a3729ceedaa05a207621a02c532536 fuzz: Handle missing BDBRO errors (Ava Chow) Pull request description: Adds error messages that were not being handled. Also removes error messages that no longer exist. Fixes #30166 ACKs for top commit: dergoegge: reACK 9ddf39dd87a3729ceedaa05a207621a02c532536 TheCharlatan: ACK 9ddf39dd87a3729ceedaa05a207621a02c532536 Tree-SHA512: 2597536a1e5d030653dfcb02fd892f7492f5a091def787f6cbd421b8bca9544847684a498e9458ea99ae7de5a8a6d91532ff904d1e39222d324939d31d2eb3f0
2024-05-29fuzz: Handle missing BDBRO errorsAva Chow
Adds error messages that were not being handled. Also removes error messages that no longer exist.
2024-05-29Merge bitcoin/bitcoin#30156: fuzz: More accurate coverage reportsmerge-script
949abebea0059edd929b653b4b475a5880fc0a3e [fuzz] Avoid collecting initialization coverage (dergoegge) Pull request description: Our coverage reports include coverage of initialization code, which can be misleading when trying to evaluate the coverage a fuzz harness achieves through fuzzing alone. This PR proposes to make fuzz coverage reports more accurate by resetting coverage counters after initialization code has been run. This makes it easier to evaluate which code was actually reached through fuzzing (e.g. to spot fuzz blockers). ACKs for top commit: maflcko: utACK 949abebea0059edd929b653b4b475a5880fc0a3e brunoerg: nice, utACK 949abebea0059edd929b653b4b475a5880fc0a3e Tree-SHA512: c8579bda4f3d71d199b9331fbe6316fce375a906743d0bc216bb94958dc03fdc9a951ea50cfeb487494a75668ae3c16471a82f7e5fdd912d781dc29d063e2c5b
2024-05-29Merge bitcoin/bitcoin#30170: refactor: Use type-safe time in txorphanagemerge-script
fa6d4891c7815025ab1cd58d0906466af31bb648 refactor: Use type-safe time in txorphanage (MarcoFalke) Pull request description: This allows to remove manual conversions like multiplication by `60`, and uses a type-safe type instead of a raw `int64_t`. ACKs for top commit: epiccurious: utACK fa6d4891c7815025ab1cd58d0906466af31bb648. dergoegge: Code review ACK fa6d4891c7815025ab1cd58d0906466af31bb648 brunoerg: utACK fa6d4891c7815025ab1cd58d0906466af31bb648 Tree-SHA512: c187d0e579b1131afcef8c901f5662c18ab867fa2a99fbb13b67bb1e10b2047128194bfef8329cde0d51e1c35d6227ae292b823968f37ea9422975e46e01846a
2024-05-28Use `exact_target` shorthand in coinselector_testsMurch
2024-05-24Fold GetSelectionWaste() into ComputeAndSetWaste()Murch
Both `GetSelectionWaste()` and `ComputeAndSetWaste()` now are part of `SelectionResult`. Instead of `ComputeAndSetWaste()` being a wrapper for `GetSelectionWaste()`, we combine them to a new function `RecalculateWaste()`. As I was combining the logic of the two functions, I noticed that `GetSelectionWaste()` was making the odd assumption that the `change_cost` being set to zero means that no change is created. However, if we build transactions at a feerate of zero with the `discard_feerate` also set to zero, we'd organically have a `change_cost` of zero, even when we create change on a transaction. This commit cleans up this duplicate meaning of `change_cost` and relies on `GetChange()` to figure out whether there is change on basis of the `min_viable_change` and whatever is left after deducting fees. Since this broke a bunch of tests that relied on the double-meaning of `change_cost` a bunch of tests had to be fixed.
2024-05-24Merge bitcoin/bitcoin#30169: fuzz: Fix wallet_bdb_parser stdlib error matchingAva Chow
fac72985292b516919a216d9a78cf84418cd7f96 fuzz: Fix wallet_bdb_parser stdlib error matching (MarcoFalke) Pull request description: The stdlib error string is an implementation detail and can not be relied upon. Ref: `libc++abi: terminating due to uncaught exception of type std::runtime_error: AutoFile::read: end of file: unspecified iostream_category error` ACKs for top commit: achow101: ACK fac72985292b516919a216d9a78cf84418cd7f96 Tree-SHA512: 588acc71a05d97855d6bb65380411e8486692536434eadee7697de09f80b128ff2f90a31fd0e8384d084b554d2f3978efd076082e070e721cf05b07c94cc83b1
2024-05-24assumeutxo: Add network magic ctor param to SnapshotMetadataFabian Jahr
This prevents SnapshotMetadata from using any globals implicitly.
2024-05-24fuzz: Fix wallet_bdb_parser stdlib error matchingMarcoFalke
2024-05-24assumeutxo: Deserialize trailing byte instead of TxidFabian Jahr
2024-05-24Merge bitcoin/bitcoin#30072: refactor prep for package rbfglozow
2fd34ba504957331f5a08614b6e1f8317260f04d Add sanity checks for various ATMPArgs booleans (Greg Sanders) 20d8936d8bb6137a5a3722d34e0d7ae756031009 [refactor] make some members MemPoolAccept-wide (glozow) cbbfe719b223b9e05398227cef68c99eb97670bd cpfp carveout is excluded in packages (glozow) 69f7ab05bafec1cf06fd7a58351f78e32bbfa2cf Add m_allow_sibling_eviction as separate ATMPArgs flag (Greg Sanders) 57ee3029ddadaee5cb48224e0a87f704b7971bd8 Add description for m_test_accept (Greg Sanders) Pull request description: First few commits of https://github.com/bitcoin/bitcoin/pull/28984 to set the stage for the package RBF logic. These refactors are preparation for evaluating an RBF in a multi-proposed-transaction context instead of only a single proposed transaction. Also, carveouts and sibling evictions only should work in single RBF cases so add logic to preclude multi-tx cases in the future. No behavior changes aside from bailing earlier from failed carve-outs. ACKs for top commit: glozow: reACK 2fd34ba504957331f5a08614b6e1f8317260f04d via range-diff sr-gi: utACK [2fd34ba](https://github.com/bitcoin/bitcoin/pull/30072/commits/2fd34ba504957331f5a08614b6e1f8317260f04d) theStack: re-ACK 2fd34ba504957331f5a08614b6e1f8317260f04d Tree-SHA512: 5071c5b8d9b8d2c9faa278c8c4df31de288cb407a68e4d55544c588caff6c86160cce7825453549c6ed69e29d9ccb5ee2d4a518b18f563bfb12f2ced073fe42a
2024-05-23Merge bitcoin/bitcoin#29612: rpc: Optimize serialization and enhance ↵Ava Chow
metadata of dumptxoutset output 542e13b2937356810bda2c41be83c3b1675e2f2f rpc: Enhance metadata of the dumptxoutset output (Fabian Jahr) 4d8e5edbaa94805be41ae4c8aa2f4bf7aaa276fe assumeutxo: Add documentation on dumptxoutset serialization format (Fabian Jahr) c14ed7f384075330361df636f40121cf25a066d6 assumeutxo: Add test for changed coin size value (Fabian Jahr) de95953d870c41436de67d56c93259bc66fe1434 rpc: Optimize serialization disk space of dumptxoutset (Fabian Jahr) Pull request description: The second attempt at implementing the `dumptxoutset` space optimization as suggested in #25675. Closes #25675. This builds on the work done in #26045, addresses open feedback, adds some further improvements (most importantly usage of compact size), documentation, and an additional test. The [original snapshot at height 830,000](https://github.com/bitcoin/bitcoin/pull/29551) came in at 10.82 GB. With this change, the same snapshot is 8.94 GB, a reduction of 17.4%. This also enhances the metadata of the output file and adds the following data to allow for better error handling and make future upgrades easier: - A newly introduced utxo set magic - A version number - The network magic - The block height ACKs for top commit: achow101: ACK 542e13b2937356810bda2c41be83c3b1675e2f2f TheCharlatan: Re-ACK 542e13b2937356810bda2c41be83c3b1675e2f2f theStack: ACK 542e13b2937356810bda2c41be83c3b1675e2f2f Tree-SHA512: 0825d30e5c3c364062db3c6cbca4e3c680e6e6d3e259fa70c0c2b2a7020f24a47406a623582040988d5c7745b08649c31110df4c10656aa25f3f27eb35843d99
2024-05-23[fuzz] Avoid collecting initialization coveragedergoegge
2024-05-23Merge bitcoin/bitcoin#27064: system: use %LOCALAPPDATA% as default datadir ↵Ava Chow
on windows 84900ac34f6888b7a851d0a6a5885192155f865c doc: add release-notes-27064.md (Matthew Zipkin) 855dd8d592c951a2b3239867ffbf66bb8677d470 system: use %LOCALAPPDATA% as default datadir on windows (Matthew Zipkin) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/2391 This PR changes the default datadir location on Windows from `C:\Users\Username\AppData\Roaming\Bitcoin` to `C:\Users\Username\AppData\Local\Bitcoin`. This change only applies to fresh installs. To preserve backwards compatibility, on startup we check for the existence of `C:\Users\Username\AppData\Roaming\Bitcoin\chainstate` and if it is there, we continue using the "Roaming" directory as the default datadir location. [Note that in Windows 11 this change may be moot:](https://learn.microsoft.com/en-us/uwp/api/windows.storage.applicationdata.roamingfolder?view=winrt-22621) > Roaming data and settings is no longer supported as of Windows 11. The recommended replacement is [Azure App Service](https://learn.microsoft.com/en-us/azure/app-service/). Azure App Service is widely supported, well documented, reliable, and supports cross-platform/cross-ecosystem scenarios such as iOS, Android and web. Settings stored here no longer roam (as of Windows 11), but the settings store is still available. ACKs for top commit: achow101: ACK 84900ac34f6888b7a851d0a6a5885192155f865c BenWestgate: crACK https://github.com/bitcoin/bitcoin/commit/84900ac34f6888b7a851d0a6a5885192155f865c hebasto: re-ACK 84900ac34f6888b7a851d0a6a5885192155f865c, only addressed feedback since my recent [review](https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2028718273). Tree-SHA512: 807c6e89571287e2c8f4934229aec91ef28e7d0a675234acf1b7d085c24c7b73a08b6e345fbfc9038e6239187b6b69c08490ddaa1c057de5ea975c4a000bba42
2024-05-23Add sanity checks for various ATMPArgs booleansGreg Sanders
2024-05-23[refactor] make some members MemPoolAccept-wideglozow
No change in behavior. For single transaction acceptance, this is a simple refactor: Workspace::m_all_conflicting Workspace::m_conflicting_fees Workspace::m_conflicting_size Workspace::m_replaced_transactions are now grouped under a new SubPackageState struct that is a member of MemPoolAccept. And local variables m_total_vsize and m_total_modified_fees are now SubpackageState members so they can be accessed from PackageMempoolChecks. We want these to be package-wide variables because - Transactions could conflict with the same tx (just not the same prevout), or their conflicts could share descendants. - We want to compare conflicts with the package fee rather than individual transaction fee. We reset these MemPoolAccept-wide fields for each subpackage evaluation to not cause state leaking, similar to temporary coins.
2024-05-23cpfp carveout is excluded in packagesglozow
The behavior is not new, but this rule exits earlier than before. Previously, a carve out could have been granted in PreChecks() but then nullified in PackageMempoolChecks() when CheckPackageLimits() is called with the default limits.
2024-05-23Add m_allow_sibling_eviction as separate ATMPArgs flagGreg Sanders
2024-05-23Add description for m_test_acceptGreg Sanders
2024-05-23Merge bitcoin/bitcoin#29873: policy: restrict all TRUC (v3) transactions to ↵Ava Chow
10kvB 154b2b2296edccb5ed24e829798dacb6195edc11 [fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limits (glozow) a29f1df289cf27c6cbd565448548b3dc1392a9b0 [policy] restrict all v3 transactions to 10kvB (glozow) d578e2e3540e085942001350ff3aeb047bdac973 [policy] explicitly require non-v3 for CPFP carve out (glozow) Pull request description: Opening for discussion / conceptual review. We like the idea of a smaller maximum transaction size because: - It lowers potential replacement cost (i.e. harder to do Rule 3 pinning via gigantic transaction) - They are easier to bin-pack in block template production - They equate to a tighter memory limit in data structures that are bounded by a number of transactions (e.g. orphanage and vExtraTxnForCompact). For example, the current memory bounds for orphanage is 100KvB * 100 = 40MB, and guaranteeing 1 tx per peer would require reserving a pretty large space. History for `MAX_STANDARD_TX_WEIGHT=100KvB` (copied from https://github.com/bitcoin/bitcoin/pull/29873#issuecomment-2115459510): - 2010-09-13 In https://github.com/bitcoin/bitcoin/commit/3df62878c3cece15a8921fbbdee7859ee9368768 satoshi added a 100kB (MAX_BLOCK_SIZE_GEN/5 with MBS_GEN = MAX_BLOCK_SIZE/2) limit on new transactions in CreateTransaction() - 2013-02-04 https://github.com/bitcoin/bitcoin/pull/2273 In gavin gave that constant a name, and made it apply to transaction relay as well Lowering `MAX_STANDARD_TX_WEIGHT` for all txns is not being proposed, as there are existing apps/protocols that rely on large transactions. However, it's been brought up that we should consider this for TRUCs (which is especially designed to avoid Rule 3 pinning). This reduction should be ok because using nVersion=3 isn't standard yet, so this wouldn't break somebody's existing use case. If we find that this is too small, we can always increase it later. Decreasing would be much more difficult. ~[Expected size of a commitment transaction](https://github.com/lightning/bolts/blob/master/03-transactions.md#expected-weight-of-the-commitment-transaction) is within (900 + 172 * 483 + 224) / 4 = 21050vB~ EDIT: this is incorrect, but perhaps not something that should affect how we choose this number. ACKs for top commit: sdaftuar: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 achow101: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 instagibbs: ACK 154b2b2296edccb5ed24e829798dacb6195edc11 t-bast: ACK https://github.com/bitcoin/bitcoin/commit/154b2b2296edccb5ed24e829798dacb6195edc11 murchandamus: crACK 154b2b2296edccb5ed24e829798dacb6195edc11 Tree-SHA512: 89392a460908a8ea9f547d90e00f5181de0eaa9d2c4f2766140a91294ade3229b3d181833cad9afc93a0d0e8c4b96ee2f5aeda7c50ad7e6f3a8320b9e0c5ae97
2024-05-23Merge bitcoin/bitcoin#30115: rpc: avoid copying into UniValueRyan Ofsky
d7707d9843b03f20d2a8c5a45d7b3db58e169e6f rpc: avoid copying into UniValue (Cory Fields) Pull request description: These are the simple (and hopefully obviously correct) copies that can be moves instead. This is a follow-up from https://github.com/bitcoin/bitcoin/pull/30094#issuecomment-2108751842 As it turns out, there are hundreds of places where we copy UniValues needlessly. It should be the case that moves are always preferred over copies, so there should be no downside to these changes. willcl-ark, however, noticed that memory usage may increase in some cases. Logically this makes no sense to me. The only plausible explanation imo is that because the moves are faster, more ops/second occur in some cases. This list of moves was obtained by changing the function signatures of the UniValue functions to accept only rvalues, then compiling and fixing them up one by one. There still exist many places where copies are being made. These can/should be fixed up, but weren't done here for the sake of doing the easy ones first. I ran these changes through clang-tidy with `performance-move-const-arg` and `bugprone-use-after-move` and no bugs were detected (though that's obviously not to say it can be trusted 100%). As stated above, there are still lots of other less trivial fixups to do after these including: - Using non-const UniValues where possible so that moves can happen - Refactoring code in order to be able to move a UniValue without introducing a use-after-move - Refactoring functions to accept UniValues by value rather than by const reference ACKs for top commit: achow101: ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f ryanofsky: Code review ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f. No changes since last review other than rebase. I agree benchmarks showing increased peak memory usage and RSS are surprising, but number of allocations is down as expected, and runtime is also decreased. willcl-ark: ACK d7707d9843b03f20d2a8c5a45d7b3db58e169e6f Tree-SHA512: 7f511be73984553c278186286a7d161a34b2574c7f5f1a0edc87c2913b4c025a0af5241ef9af2df17547f2e4ef79710aa5bbb762fc9472435781c0488dba3435
2024-05-23Merge bitcoin/bitcoin#30062: net: add ASMap info in `getrawaddrman` RPCglozow
1e54d61c4698debf3329d1960e06078ccbf8063c test: add coverage for `mapped_as` from `getrawaddrman` (brunoerg) 8c2714907d1e1ffc58487b3b43e018c1ec10065b net: rpc: return peer's mapped AS in getrawaddrman (brunoerg) Pull request description: This PR adds two new fields in `getrawaddrman` RPC: "mapped_as" and "source_mapped_as". These fields are used to return the ASN (Autonomous System Number) mapped to the peer and its source. With these informations we can have a better view of the bucketing logic with ASMap specially in projects like [addrman-observer](https://github.com/0xb10c/addrman-observer). ACKs for top commit: fjahr: Code review ACK 1e54d61c4698debf3329d1960e06078ccbf8063c virtu: ACK [1e54d61](https://github.com/bitcoin/bitcoin/commit/1e54d61c4698debf3329d1960e06078ccbf8063c) 0xB10C: ACK 1e54d61c4698debf3329d1960e06078ccbf8063c glozow: ACK 1e54d61c4698debf3329d1960e06078ccbf8063c Tree-SHA512: af86bcc7a2e69bebd3fa9eaa2e527e0758c44c0a958de7292514d5f99f8f01f5df3bae11400451268e0255f738ff3acccc77f48fe129937512f1e9d9963c4c5e
2024-05-22Merge bitcoin/bitcoin#30131: wallet, tests: Avoid stringop-overflow warning ↵merge-script
in PollutePubKey 2289d4524053ab71c0d9133987cb36412797c1a2 wallet, tests: Avoid stringop-overflow warning in PollutePubKey (Ava Chow) Pull request description: Fixes #30114 ACKs for top commit: maflcko: ACK 2289d4524053ab71c0d9133987cb36412797c1a2 with g++ 14.1.1 🦄 theStack: utACK 2289d4524053ab71c0d9133987cb36412797c1a2 laanwj: ACK 2289d4524053ab71c0d9133987cb36412797c1a2 Tree-SHA512: 173c3c299bdd890f73e8a67a37880dbf816265e8b3c8298557ef2fc4670f5447005c0d2d81726f9bc43f6a69d677365d90a604354b3cbab0e3c52c4526d0407e
2024-05-22net: rpc: return peer's mapped AS in getrawaddrmanbrunoerg
This information can be used to check bucketing logic.
2024-05-22Merge bitcoin/bitcoin#30120: Update libsecp256k1 subtree to current mastermerge-script
a057869aa3c42457570765966cb66accb2375b13 build: pass --with-ecmult-gen-kb=86 to secp256k1 (fanquake) ca3d945dc66e177e8fa3e83c77236de89cc0072a Squashed 'src/secp256k1/' changes from d8311688bd..06bff6dec8 (fanquake) Pull request description: This includes changes from the 0.5.0 release: https://github.com/bitcoin-core/secp256k1/releases/tag/v0.5.0 > New function secp256k1_ec_pubkey_sort that sorts public keys using lexicographic (of compressed serialization) order. > The implementation of the point multiplication algorithm used for signing and public key generation was changed, resulting in improved performance for those operations. > The related configure option --ecmult-gen-precision was replaced with --ecmult-gen-kb (ECMULT_GEN_KB for CMake). > This changes the supported precomputed table sizes for these operations. The new supported sizes are 2 KiB, 22 KiB, or 86 KiB (while the old supported sizes were 32 KiB, 64 KiB, or 512 KiB). ACKs for top commit: hebasto: ACK a057869aa3c42457570765966cb66accb2375b13, I've got a zero diff with my local branch, which reproduces the subtree update, and `ecmult gen table size = 86 KiB` in the configure summary. jonasnick: utACK a057869aa3c42457570765966cb66accb2375b13 Tree-SHA512: 907012b0d7e0a6bd68b245c238e968f2318d8ac5de5ec9070245de8391c996eb5ec6428184d028f6f0f54d3b2f5a8292ad7081177e1c331397879505436dc38e
2024-05-21Merge bitcoin/bitcoin#30137: build: Remove `--enable-threadlocal`merge-script
17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af build: remove --enable-threadlocal (fanquake) 1e7c20bc19a216269c646177ab90cfa084c096a5 doc: remove comment about using thread_local (fanquake) 5bba43312c0ceccfe18bd4d086e12ec0497ed926 build: Enable `thread_local` for MinGW-w64 builds (Hennadii Stepanov) Pull request description: Includes a commit from #30099. Closes #29952. ACKs for top commit: laanwj: Code review ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af maflcko: utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af hebasto: ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af. theuni: utACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af Tree-SHA512: 2aad6d19e79c4d6d7aefd0f41b215ac8d9320f5908808221d78e6ee1c77503832a02759bee2ad397e235b6739e22aca8dcf5c5ef8854deb8c697b18ac56a06da
2024-05-21Merge bitcoin/bitcoin#29421: net: make the list of known message types a ↵Ava Chow
compile time constant b3efb486732f3caf8b8a8e9d744e6d20ae4255ef protocol: make message types constexpr (Vasil Dimov) 2fa9de06c2c8583ee8e2434dc97014b26e218ab5 net: make the list of known message types a compile time constant (Vasil Dimov) Pull request description: Turn the `std::vector` to `std::array` because it is cheaper and allows us to have the number of the messages as a compile time constant: `ALL_NET_MESSAGE_TYPES.size()` which can be used in future code to build other `std::array`s with that size. --- This change is part of https://github.com/bitcoin/bitcoin/pull/29418 but it makes sense on its own and would be good to have it, regardless of the fate of https://github.com/bitcoin/bitcoin/pull/29418. Also, if this is merged, that would reduce the size of https://github.com/bitcoin/bitcoin/pull/29418, thus the current standalone PR. ACKs for top commit: achow101: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef jonatack: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef maflcko: utACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef 🎊 willcl-ark: ACK b3efb486732f3caf8b8a8e9d744e6d20ae4255ef Tree-SHA512: 6d3860c138c64514ebab13d97ea67893e2d346dfac30a48c3d9bc769a1970407375ea4170afdb522411ced306a14a9af4eede99e964d1fb1ea3efff5d5eb57af
2024-05-21wallet, tests: Avoid stringop-overflow warning in PollutePubKeyAva Chow
2024-05-21[fuzz] V3_MAX_VSIZE and effective ancestor/descendant size limitsglozow
2024-05-21[policy] restrict all v3 transactions to 10kvBglozow
2024-05-21[policy] explicitly require non-v3 for CPFP carve outglozow
This carve out is intended to allow a second child under restricted circumstances, but this topology is not allowed for v3 transactions. As CPFP carve out does not explicitly require a second child to actually exist, it has the effect of granting a free +10KvB descendant size limit when a single child is enough to bust the descendant limit.
2024-05-21rpc: Enhance metadata of the dumptxoutset outputFabian Jahr
The following data is added: - A newly introduced utxo set magic - A version number - The network magic - The block height