aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2023-05-09Add missing definitions in prep for scripted diffTheCharlatan
The missing include and ArgsManager were found after applying the scripted diff in the following commit.
2023-05-09refactor: Replace string chain name constants with ChainTypesTheCharlatan
This commit effectively moves the definition of these constants out of the chainparamsbase to their own file. Using the ChainType enums provides better type safety compared to passing around strings. The commit is part of an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager and other functionality that should not be part of the kernel library.
2023-05-09refactor: Introduce ChainType getters for ArgsManagerTheCharlatan
These are introduced for the next commit where the usage of the ChainType is adopted throughout the code. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-05-09refactor: Create chaintype filesTheCharlatan
This is the first of a number of commits with the goal of moving the chain type definitions out of chainparamsbase to their own file and implementing them as enums instead of constant strings. The goal is to allow the kernel chainparams to no longer include chainparamsbase. The commit is part of an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager and other functionality that should not be part of the kernel library.
2023-05-06Merge bitcoin/bitcoin#17860: fuzz: BIP 30, CVE-2018-17144fanquake
fa2d8b61f9343d350b67357a12f39b613c8ee8ad fuzz: BIP 42, BIP 30, CVE-2018-17144 (MarcoFalke) faae7d5c00c99b0f3e99a1fbffbf369645716dd1 Move LoadVerifyActivateChainstate to ChainTestingSetup (MarcoFalke) fa26e3462a0fb1a9ad116ed58afa6897798f2c24 Avoid dereferencing interruption_point if it is nullptr (MarcoFalke) fa846ee074822160077f3f7476b2af62a876dec7 test: Add util to mine invalid blocks (MarcoFalke) Pull request description: Add a validation fuzz test for BIP 30 and CVE-2018-17144 ACKs for top commit: dergoegge: Code review ACK fa2d8b61f9343d350b67357a12f39b613c8ee8ad mzumsande: Tested ACK fa2d8b61f9343d350b67357a12f39b613c8ee8ad Tree-SHA512: 1f4620cc078709487abff24b304a6bb4eeab2e7628b392e2bc6de9cc0ce6745c413388ede6e93025d0c56eec905607ba9786633ef183e5779bf5183cc9ff92c0
2023-05-06Merge bitcoin/bitcoin#27405: util: Use steady clock instead of system clock ↵fanquake
to measure durations fa83fb31619c19a1a30b4181486601a944941b16 wallet: Use steady clock to calculate number of derive iterations (MarcoFalke) fa2c099cec1780a498e198750be0cf5bf0ca315a wallet: Use steady clock to measure scanning duration (MarcoFalke) fa976218044f3ff244abbd797b183a1408375c74 qt: Use steady clock to throttle GUI notifications (MarcoFalke) fa1d8044abc2cd0f149a2d526b3b03441443cdb0 test: Use steady clock in index tests (MarcoFalke) fa454dcb20b9e7943cc25e6eeea72912b5f1c7b5 net: Use steady clock in InterruptibleRecv (MarcoFalke) Pull request description: `GetTimeMillis` has multiple issues: * It doesn't denote the underlying clock type * It isn't type-safe * It is used incorrectly in places that should use a steady clock Fix all issues here. ACKs for top commit: willcl-ark: ACK fa83fb3161 martinus: Code review ACK https://github.com/bitcoin/bitcoin/commit/fa83fb31619c19a1a30b4181486601a944941b16, also ran all tests. All usages of the steady_clock are just for duration measurements, so the change to a different epoch is ok. Tree-SHA512: 5ec4fede8c7f97e2e08863c011856e8304f16ba30a68fdeb42f96a50a04961092cbe46ccf9ea6ac99ff5203c09f9e0924eb483eb38d7df0759addc85116c8a9f
2023-05-05fuzz: BIP 42, BIP 30, CVE-2018-17144MarcoFalke
2023-05-05Move LoadVerifyActivateChainstate to ChainTestingSetupMarcoFalke
2023-05-04Remove unused chainparams from BlockManager methodsMarcoFalke
Also, replace pointer with reference while touching the signature.
2023-05-04Replace pindex pointer with block referenceMarcoFalke
pindex can not be nullptr, so document that, and clear it up in the next commit.
2023-05-04Add BlockManagerOpts::chainparams referenceMarcoFalke
and use it in blockstorage.cpp
2023-05-03Merge bitcoin/bitcoin#24957: prune, import: allow pruning to work during ↵Andrew Chow
loadblock import c4981e7f63a3e0aeec1ef3dec040261e993dd724 prune, import: fixes #23852 (mruddy) Pull request description: Fixes #23852 This allows pruning to work during the `-loadblock` import process. An example use case is where you have a clean set of block files and you want to create a pruned node from them, but you don't want to alter the input set of block files. #23852 noted that pruning was not working reliably during the loadblock import process. The reason why the loadblock process was not pruning regularly as it progressed is that the pruning process (`BlockManager::FindFilesToPrune`) checks the tip height of the active chainstate, and `CChainState::ActivateBestChain` was not called (which updates that tip height) in `ThreadImport` until after all the import files were processed. An example bash command line that makes it easy to import a bunch of block files: ``` ./src/qt/bitcoin-qt -debug -logthreadnames -datadir=/tmp/btc -prune=550 -loadblock=/readonly/btc/main/blk{00000..00043}.dat ``` One interesting side note is that `CChainState::ActivateBestChain` can be called while the import process is running (in the `loadblk` thread) by concurrent network message processing activity in the `msghand` thread. For example, one way to reproduce this easily is with the `getblockfrompeer` RPC (requesting a block with height greater than 100000) run from a node connected to an importing node. There are other ways too, but this is an easy way. I only mention this to explain how the `max_prune_height=225719` log message in the original issue came to occur. ACKs for top commit: achow101: re-ACK c4981e7f63a3e0aeec1ef3dec040261e993dd724 Tree-SHA512: d287c7753952c22f598ba782914c47f45ad44ce60b0fbce9561354e701f1a2a98bafaaaa106c8428690b814e281305ca3622b177ed3cb2eb7559f07c958ab537
2023-05-03Merge bitcoin/bitcoin#26066: wallet: Refactor and document CoinControlAndrew Chow
daba95700b0b77a2e898299f218c47a69ed2c7d0 refactor: Make ListSelected return vector (Sebastian Falbesoner) 94776621ba6a79f3197ec71250bc48e974ad5e4a wallet: Move CoinCointrol definitions to .cpp (Aurèle Oulès) 1db23da6e163e793458ec702a9601d2837aea9cb wallet: Use std::optional for GetExternalOutput and fixups (Aurèle Oulès) becc45b589d07c4523276e4ba2dad8852d0d2632 scripted-diff: Rename setSelected->m_selected_inputs (Aurèle Oulès) Pull request description: - Moves CoinControl function definitions from `coincontrol.h` to `coincontrol.cpp` - Adds more documentation - Renames class member for an improved comprehension - Use `std::optional` for `GetExternalOutput` ACKs for top commit: achow101: ACK daba95700b0b77a2e898299f218c47a69ed2c7d0 Xekyo: ACK daba95700b0b77a2e898299f218c47a69ed2c7d0 Tree-SHA512: 3bf2dc834a3246c2f53f8c55154258e605fcb169431d3f7b156931f33c7e3b1ae28e03e16b37f9140a827890eb7798be485b2c36bfc23ff29bb01763f289a07c
2023-05-03clarify processing of mempool-msgs when NODE_BLOOM0xb10c
Under which circumstances we process received 'mempool' P2P messages caused confusion in #27426. Rather than bikeshedding the formulation of the IF-statement, this adds a comment clarifing when we process the message. Also, correcting the comment of `m_send_mempool`. Co-authored-by: willcl-ark <will8clark@gmail.com>
2023-05-02Avoid dereferencing interruption_point if it is nullptrMarcoFalke
2023-05-02Merge bitcoin/bitcoin#26094: rpc: Return block hash & height in getbalances, ↵Andrew Chow
gettransaction and getwalletinfo 710b83938ab5bbc4bd324d8b2e69461a2a1d2eec rpc: return block hash & height in getbalances, gettransaction & getwalletinfo JSONs (Harris) Pull request description: Reopens #18570 and closes #18567. I have rebased the original PR. Not sure why the original got closed as it was about to get merged. ACKs for top commit: achow101: ACK 710b83938ab5bbc4bd324d8b2e69461a2a1d2eec Tree-SHA512: d4478d990be98b1642e9ffb2930987f4a224e8bd64e2e35a5dda927a54c509ec9d712cd0eac35dc2bb89f00a1678e530ce14d7445f1dd93aa3a4cce2bc9b130d
2023-05-02test: Add util to mine invalid blocksMarcoFalke
With the current utils it is only possible to mine valid blocks. This commit adds new util methods to mine invalid blocks.
2023-05-02Merge bitcoin/bitcoin#27191: blockstorage: Adjust fastprune limit if block ↵fanquake
exceeds blockfile size 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9 test: cover fastprune with excessive block size (Matthew Zipkin) 271c23e87f61276d7acab74e115b25a35144c8b4 blockstorage: Adjust fastprune limit if block exceeds blockfile size (Martin Zumsande) Pull request description: The debug-only `-fastprune` option used in several tests is not always safe to use: If a `-fastprune` node receives a block larger than the maximum blockfile size of `64kb` bad things happen: The while loop in `BlockManager::FindBlockPos` never terminates, and the node runs oom because memory for `m_blockfile_info` is allocated in each iteration of the loop. The same would happen if a naive user used `-fastprune` on anything other than regtest (so this can be tested by syncing on signet for example, the first block that crashes the node is at height 2232). Change the approach by raising the blockfile size to the size of the block, if that block otherwise wouldn't fit (idea by TheCharlatan). ACKs for top commit: ryanofsky: Code review ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9. Added new assert, test, and comment since last review TheCharlatan: ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9 pinheadmz: ACK 8f14fc86225d8fe77353f61ebd6b0bdda8d13aa9 Tree-SHA512: df2fea30613ef9d40ebbc2416eacb574f6d7d96847db5c33dda22a29a2c61a8db831aa9552734ea4477e097f253dbcb6dcb1395d43d2a090cc0588c9ce66eac3
2023-05-01Merge bitcoin/bitcoin#26780: rpc: simplify scan blocksAndrew Chow
b922f6b5262884f42d7483f1e9af35650bdb53a7 rpc: scanblocks, add "completed" flag to the result obj (furszy) ce50acc54fa313a92d48ed03e46ce8aabcf267e5 rpc: scanblocks, do not traverse the whole chain block by block (furszy) Pull request description: Coming from https://github.com/bitcoin/bitcoin/pull/23549#pullrequestreview-1105712566 The current `scanblocks` flow walks-through every block in the active chain until hits the chain tip or processes 10k blocks, then calls `lookupFilterRange` function to obtain all filters from that particular range. This is only done to obtain the heights range to look up the block filters. Which is unneeded. As `scanblocks` only lookup block filters in the active chain, we can directly calculate the lookup range heights, by using the chain tip, without requiring to traverse the chain block by block. ACKs for top commit: achow101: ACK b922f6b5262884f42d7483f1e9af35650bdb53a7 TheCharlatan: ACK b922f6b5262884f42d7483f1e9af35650bdb53a7 Tree-SHA512: 0587e6d9cf87a59184adb2dbc26a4e2bce3a16233594c6c330f69feb49bf7dc63fdacf44fc20308e93441159ebc604c63eb7de19204d3e745a2ff16004892b45
2023-05-01Merge bitcoin/bitcoin#27195: bumpfee: allow send coins back to yourselfAndrew Chow
be72663a1521bc6cdf16d43a4feae7c5b57735c0 test: bumpfee, add coverage for "send coins back to yourself" (furszy) 7bffec6715a75bb2c8631177d39f984aabc656ba bumpfee: enable send coins back to yourself (furszy) Pull request description: Simple example: 1) User_1 sends 0.1 btc to user_2 on a low fee transaction. 2) After few hours, the tx is still in the mempool, user_2 is not interested anymore, so user_1 decides to cancel it by sending coins back to himself. 3) User_1 has the bright idea of opening the explorer and copy the change output address of the transaction. Then call bumpfee providing such output (in the "outputs" arg). Currently, this is not possible. The wallet fails with "Unable to create transaction. Transaction must have at least one recipient" error. The error reason is because we discard the provided output from the recipients list and set it inside the coin control so the process adds it later (when the change is calculated). But.. there is no later if the tx has no outputs. ACKs for top commit: ishaanam: reACK be72663a1521bc6cdf16d43a4feae7c5b57735c0 achow101: ACK be72663a1521bc6cdf16d43a4feae7c5b57735c0 Tree-SHA512: c2c38290a998f9b426a830d9624c7feb730158980ac186f8fb0138d5e200935d6538307bc60a2c3d0b7b6ee2b4ffb77a1e98baf8feb1d20a7d825f6055ac377f
2023-05-01Merge bitcoin/bitcoin#25680: rpc, docs: Add note for commands that supports ↵Andrew Chow
only legacy wallets 9141e4395a5f923059ad61ac6ba42a1a89e92cb0 rpc, docs: Add note for commands that supports only legacy wallets (Yusuf Sahin HAMZA) Pull request description: Refs #25363, apparently issue is not updated since over a month, so i decided to put the same `importaddress` note in #25368 to other rpc commands that needs this note. Note is added for following commands: - `importprivkey` - `importpubkey` - `importwallet` - `dumpprivkey` - `dumpwallet` - `importmulti` - `addmultisigaddress` - `sethdseed` ACKs for top commit: achow101: ACK 9141e4395a5f923059ad61ac6ba42a1a89e92cb0 Tree-SHA512: f3dc05d26577fd8dbe2bd69cb5c14ffccebacd6010402af44427b3d01be8484895dfcf33d55dfa766eadb7f9f3bae5cc4c2add3ac816a2ac60e8beb5a97527f3
2023-05-01Merge bitcoin/bitcoin#27224: refactor: Remove CAddressBookData::destdataAndrew Chow
a5986e82dd2b8f923d60f9e31760ef62b9010105 refactor: Remove CAddressBookData::destdata (Ryan Ofsky) 5938ad0bdb013953861c7cd15a95f00998a06f44 wallet: Add DatabaseBatch::ErasePrefix method (Ryan Ofsky) Pull request description: This is cleanup that doesn't change external behavior. Benefits of the cleanup are: - Removes awkward `StringMap` intermediate representation for wallet address metadata. - Simplifies `CWallet`, deals with used address and received request serialization in `walletdb.cpp` instead of higher level wallet code - Adds test coverage and documentation This PR doesn't change externally observable behavior. Internally, the only change in behavior is that `EraseDestData` deletes rows directly from the database because they are no longer stored in memory. This is more direct and efficient because it uses a single lookup and scan instead of multiple lookups. Motivation for this cleanup is making changes like #18550, #18192, #13756 easier to reason about and less likely to result in unintended behavior and bugs --- This PR is a rebased copy of #18608. For some reason that PR is locked and couldn't be reopened or commented on. This PR is an alternative to #27215 with differences described in https://github.com/bitcoin/bitcoin/pull/27215#pullrequestreview-1329028143 ACKs for top commit: achow101: ACK a5986e82dd2b8f923d60f9e31760ef62b9010105 furszy: Code ACK a5986e82 Tree-SHA512: 6bd3e402f1f60263fbd433760bdc29d04175ddaf8307207c4a67d59f6cffa732e176ba57886e02926f7a1615dce0ed9cda36c8cbc6430aa8e5b56934c23f3fe7
2023-04-30rpc: scanblocks, add "completed" flag to the result objfurszy
To tell the user whether the process was aborted or not. Plus, as the process can be aborted prior to the end range, have also changed the "to_height" result value to return the last scanned block instead of the end range block.
2023-04-30rpc: scanblocks, do not traverse the whole chain block by blockfurszy
The current flow walks-through every block in the active chain until hits the chain tip or processes 10k blocks, then calls `lookupFilterRange()` to obtain all the filters from that particular range. This is only done to obtain the heights range to look up the block filters. Which is unneeded. As `scanblocks` only lookup block filters in the active chain, we can directly calculate the lookup range heights, by using the chain tip, without requiring to traverse the chain block by block.
2023-04-26rpc: return block hash & height in getbalances, gettransaction & ↵Harris
getwalletinfo JSONs Co-authored-by: Aurèle Oulès <aurele@oules.com>
2023-04-26Merge bitcoin/bitcoin#25158: rpc, wallet: add abandoned field for all ↵Andrew Chow
categories of transaction in ListTransaction 0c520679ab5f0ba99584cb356ec28ef089f14735 doc: add release notes for `abandoned` field in `gettransaction` and `listtransactions` (brunoerg) a1aaa7f51f4205ae4d27fbceb2c3a97bc114e828 rpc, wallet: add `abandoned` field for all categories of transactions in ListTransactions (brunoerg) Pull request description: Fixes #25130 ACKs for top commit: achow101: re-ACK 0c520679ab5f0ba99584cb356ec28ef089f14735 Tree-SHA512: 1864460d76decab7898737c96517d722055eb8f81ca52248fe1035723258c6cd4a93251e06a86ecbbb0b0a80af1466b2c86fb142ace4ccb74cc40d5dc3967d7f
2023-04-26Merge bitcoin/bitcoin#26933: mempool: disallow txns under min relay fee, ↵glozow
even in packages bf77fc9cb45209b9c560208c65abc94209cd7919 [test] mempool full in package accept (glozow) b51ebccc28e66c1822ab22d2d178be55c6618196 [validation] set PackageValidationState when mempool full (glozow) 563a2ee4f564c8ea5f8313d711b196e260568c04 [policy] disallow transactions under min relay fee, even in packages (glozow) c4554fe894d7af8e666f5d424deccddf516713ef [test] package cpfp bumps parents <mempoolminfee but >=minrelaytxfee (glozow) ac463e87df728689701810c3961155c49fdc5b31 [test util] mock mempool minimum feerate (glozow) Pull request description: Part of package relay, see #27463. Note that this still allows packages to bump transactions that are below the dynamic mempool minimum feerate, which means this still solves the "mempool is congested and my presigned 1sat/vB tx is screwed" problem for all transactions. On master, the package policy (only accessible through regtest-only RPC submitpackage) allows 0-fee (or otherwise below min relay feerate) transactions if they are bumped by a child. However, with default package limits, we don't yet have a DoS-resistant way of ensuring these transactions remain bumped throughout their time in the mempool. Primarily, the fee-bumping child may later be replaced by another transaction that doesn't bump the parent(s). The parent(s) could potentially stay bumped by other transactions, but not enough to ever be selected by the `BlockAssembler` (due to `blockmintxfee`). For example, (tested [here](https://github.com/glozow/bitcoin/commits/26933-motivation)): - The mempool accepts 24 below-minrelayfeerate transactions ("0-fee parents"), all bumped by a single high-fee transaction ("the fee-bumping child"). The fee-bumping child also spends a confirmed UTXO. - Two additional children are added to each 0-fee parent. These children each pay a feerate slightly above the minimum relay feerate (e.g. 1.9sat/vB) such that, for each 0-fee parent, the total fees of its two children divided by the total size of the children and parent is above the minimum relay feerate. - If a block template is built now, all transactions would be selected. - A transaction replaces the the fee-bumping child, spending only the confirmed UTXO and not any of the outputs from the 0-fee parents. - The 0-fee parents now each have 2 children. Their descendant feerates are above minrelayfeerate, which means that they remain in the mempool, even if the mempool evicts all below-minrelayfeerate packages. - If a block template is built now, none of the 0-fee parents or their children would be selected. - Even more low-feerate descendants can be added to these below-minrelayfeerate packages and they will not be evicted until they expire or the mempool reaches capacity. Unless we have a DoS-resistant way of ensuring package CPFP-bumped transactions are always bumped, allowing package CPFP to bump below-minrelayfeerate transactions can result in these problematic situations. See #27018 which proposes a partial solution with some limitations, and contains discussion about potential improvements to eviction strategy. While no adequate solution exists, for now, avoid these situations by requiring all transactions to meet min relay feerate. ACKs for top commit: ajtowns: reACK bf77fc9cb45209b9c560208c65abc94209cd7919 instagibbs: re-ACK https://github.com/bitcoin/bitcoin/pull/26933/commits/bf77fc9cb45209b9c560208c65abc94209cd7919 Tree-SHA512: 28940f41493a9e280b010284316fb8caf1ed7b2090ba9a4ef8a3b2eafc5933601074b142f4f7d4e3c6c4cce99d3146f5c8e1393d9406c6f2070dd41c817985c9
2023-04-26refactor: Make ListSelected return vectorSebastian Falbesoner
2023-04-26wallet: Move CoinCointrol definitions to .cppAurèle Oulès
Move definitions to coincontrol.cpp and add documentation.
2023-04-26wallet: Use std::optional for GetExternalOutput and fixupsAurèle Oulès
2023-04-26scripted-diff: Rename setSelected->m_selected_inputsAurèle Oulès
-BEGIN VERIFY SCRIPT- sed -i 's/setSelected/m_selected_inputs/g' src/wallet/coincontrol.h src/wallet/coincontrol.cpp -END VERIFY SCRIPT-
2023-04-21Merge bitcoin/bitcoin#27506: test: prevent intermittent failuresfanquake
10a354f1740a5c1b913d0b6951e80fb5401ab43a test: prevent intermittent failures (Amiti Uttarwar) Pull request description: Follow up to #27214 - add an address to the tried table before the new table to make sure a new table collision is not possible. ACKs for top commit: mzumsande: Code review ACK 10a354f1740a5c1b913d0b6951e80fb5401ab43a - the fix is what I suggested [here](https://github.com/bitcoin/bitcoin/pull/27214#discussion_r1169169601) and should make these intermittent failures impossible. Tree-SHA512: 24099f02e1915395130065af0ef6a2a1893955d222517d156d928765541d9c427da00172a9b5a540163f4d6aae93ca3882e8267eeb35ecc595d42178abc6191c
2023-04-21Merge bitcoin/bitcoin#25939: rpc: In `utxoupdatepsbt` also look for the tx ↵Andrew Chow
in the txindex 6e9f8bb050785dbc754b6bb493aad9139908ef98 rpc, tests: in `utxoupdatepsbt` also look for the transaction in the txindex (ishaanam) a5b4883fb43d01bfef1244df62c64bf8691ca63a rpc: extract psbt updating logic into ProcessPSBT (ishaanam) Pull request description: Previously the `utxoupdatepsbt` RPC, added in #13932, only updated the inputs spending segwit outputs with the `witness_utxo`, because the full transaction is needed for legacy inputs. Before this RPC looked for just the utxos in the utxo set and the mempool. This PR makes it so that if the user has txindex enabled, then the full transaction is looked for there for all inputs. If it is not found in the txindex or txindex isn't enabled, then the mempool is checked for the full transaction. If the transaction an input is spending from is still not found at that point, then the utxo set is searched and the inputs spending segwit outputs are updated with just the utxo. ACKs for top commit: achow101: ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98 Xekyo: ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98 Tree-SHA512: 078db3c37a1ecd5816d80a42e8bd1341e416d661f508fa5fce0f4e1249fefd7b92a0d45f44957781cb69d0953145ef096ecdd4545ada39062be27742402dac6f
2023-04-21Merge bitcoin/bitcoin#27464: fuzz: re-enable prioritisetransaction & ↵fanquake
analyzepsbt RPC faa7144d3cf41e6410d942a3c485982ee65b3c6e fuzz: re-enable prioritisetransaction & analyzepsbt RPC (MarcoFalke) Pull request description: The linked issue seems fixed, so it should be fine to re-enable ACKs for top commit: dergoegge: utACK faa7144d3cf41e6410d942a3c485982ee65b3c6e Tree-SHA512: a681c726fceacc27ab5a03d455c7808d33f3cb11fe7d253d455526568af840b29f0c3c1d97c54785ef9277e7891a3aa742ac73ccd3cf115b7606eba50864aaa9
2023-04-21Merge bitcoin/bitcoin#27419: move-only: Extract common/args from util/systemfanquake
be55f545d53d44fdcf2d8ae802e9eae551d120c6 move-only: Extract common/args and common/config.cpp from util/system (TheCharlatan) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". It is part of a series of patches splitting up the `util/system` files. Its preceding pull request is https://github.com/bitcoin/bitcoin/pull/27254. The pull request contains an extraction of ArgsManager related functions from util/system into their own common/ file. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See [doc/design/libraries.md](https://github.com/bitcoin/bitcoin/blob/master/doc/design/libraries.md) for more information on this rationale. ACKs for top commit: MarcoFalke: re-ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6 🚲 ryanofsky: Code review ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6. Just small cleanups since the last review. hebasto: ACK be55f545d53d44fdcf2d8ae802e9eae551d120c6, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 90eb03334af0155b823030b4f2ecf286d35058d700ee2ddbbaa445be19e31eb0fe982656f35bd14ecee3ad2c3d0db3746855cb8f3777eff7253713e42873e111
2023-04-20test: prevent intermittent failuresAmiti Uttarwar
Add to the tried table before the new table to make sure a new table collision is not possible Co-authored-by: Martin Zumsande <mzumsande@gmail.com>
2023-04-20Merge bitcoin/bitcoin#27412: logging, net: add ASN from peers on logsAndrew Chow
0076bed45eb2b42111fa3f4c95181393c685a42e logging: log ASN when using `-asmap` (brunoerg) 9836c76ae048698e4f7dab21e3be37313e8392ae net: add `GetMappedAS` in `CConnman` (brunoerg) Pull request description: When using `-asmap`, you can check the ASN assigned to the peers only with the RPC command `getpeerinfo` (check `mapped_as` field), however, it's not possible to check it in logs (e.g. see in logs the ASN of the peers when a new outbound peer has been connected). This PR includes the peers' ASN in debug output when using `-asmap`. Obs: Open this primarily to chase some Concept ACK, I've been using this on my node to facilitate to track the peers' ASN especially when reading the logs. ACKs for top commit: Sjors: tACK 0076bed45eb2b42111fa3f4c95181393c685a42e jamesob: ACK 0076bed45eb2b42111fa3f4c95181393c685a42e ([`jamesob/ackr/27412.1.brunoerg.logging_net_add_asn_from`](https://github.com/jamesob/bitcoin/tree/ackr/27412.1.brunoerg.logging_net_add_asn_from)) achow101: ACK 0076bed45eb2b42111fa3f4c95181393c685a42e Tree-SHA512: c19cd11e8ab49962021f390459aadf6d33d221ae9a2c3df331a25d6865a8df470e2c8828f6e5219b8a887d6ab5b3450d34be9e26c00cca4d223b4ca64d51111b
2023-04-20Merge bitcoin/bitcoin#26720: wallet: coin selection, don't return results ↵Andrew Chow
that exceed the max allowed weight 25ab14712b9d80276016f9fc9bff7fb9c1d09635 refactor: coinselector_tests, unify wallet creation code (furszy) ba9431c505e1590db6103b9632134985cd4704dc test: coverage for bnb max weight (furszy) 5a2bc45ee0b123e461c5191322ed0b43524c3d82 wallet: clean post coin selection max weight filter (furszy) 2d112584e384de10021c64e4700455d71326824e coin selection: BnB, don't return selection if exceeds max allowed tx weight (furszy) d3a1c098e4b5df2ebbae20c6e390c3d783950e93 test: coin selection, add coverage for SRD (furszy) 9d9689e5a657956db8a30829c994600ec7d3098b coin selection: heap-ify SRD, don't return selection if exceeds max tx weight (furszy) 6107ec2229c5f5b4e944a6b10d38010c850094ac coin selection: knapsack, select closest UTXO above target if result exceeds max tx size (furszy) 1284223691127e76135a46d251c52416104f0ff1 wallet: refactor coin selection algos to return util::Result (furszy) Pull request description: Coming from the following comment https://github.com/bitcoin/bitcoin/pull/25729#discussion_r1029324367. The reason why we are adding hundreds of UTXO from different sources when the target amount is covered only by one of them is because only SRD returns a usable result. Context: In the test, we create 1515 UTXOs with 0.033 BTC each, and 1 UTXO with 50 BTC. Then perform Coin Selection to fund 49.5 BTC. As the selection of the 1515 small UTXOs exceeds the max allowed tx size, the expectation here is to receive a selection result that only contain the big UTXO. Which is not happening for the following reason: Knapsack returns a result that exceeds the max allowed transaction size, when it should return the closest utxo above the target, so we fallback to SRD who selects coins randomly up until the target is met. So we end up with a selection result with lot more coins than what is needed. ACKs for top commit: S3RK: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 achow101: ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Xekyo: reACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 theStack: Code-review ACK 25ab14712b9d80276016f9fc9bff7fb9c1d09635 Tree-SHA512: 2425de4cc479b4db999b3b2e02eb522a2130a06379cca0418672a51c4076971a1d427191173820db76a0f85a8edfff100114e1c38fb3b5dc51598d07cabe1a60
2023-04-20Merge bitcoin/bitcoin#25325: Add pool based memory resourceAndrew Chow
9f947fc3d4b779f017332135323b34e8f216f613 Use PoolAllocator for CCoinsMap (Martin Leitner-Ankerl) 5e4ac5abf54f8e6d6330df0c73119aa0cca4c103 Call ReallocateCache() on each Flush() (Martin Leitner-Ankerl) 1afca6b663bb54022afff193fd9d83856606b189 Add PoolResource fuzzer (Martin Leitner-Ankerl) e19943f049ed8aa4f32a1d8440a9fbf160367f0f Calculate memory usage correctly for unordered_maps that use PoolAllocator (Martin Leitner-Ankerl) b8401c3281978beed6198b2f9782b6a8dd35cbd7 Add pool based memory resource & allocator (Martin Leitner-Ankerl) Pull request description: A memory resource similar to `std::pmr::unsynchronized_pool_resource`, but optimized for node-based containers. The goal is to be able to cache more coins with the same memory usage, and allocate/deallocate faster. This is a reimplementation of #22702. The goal was to implement it in a way that is simpler to review & test * There is now a generic `PoolResource` for allocating/deallocating memory. This has practically the same API as `std::pmr::memory_resource`. (Unfortunately I cannot use std::pmr because libc++ simply doesn't implement that API). * Thanks to sipa there is now a fuzzer for PoolResource! On a fast machine I ran it for ~770 million executions without finding any issue. * The estimation of the correct node size is now gone, PoolResource now has multiple pools and just needs to be created large enough to have space for the unordered_map nodes. I ran benchmarks with #22702, mergebase, and this PR. Frequency locked Intel i7-8700, clang++ 13.0.1 to reindex up to block 690000. ```sh bitcoind -dbcache=5000 -assumevalid=00000000000000000002a23d6df20eecec15b21d32c75833cce28f113de888b7 -reindex-chainstate -printtoconsole=0 -stopatheight=690000 ``` The performance is practically identical with #22702, just 0.4% slower. It's ~21% faster than master: ![Progress in Million Transactions over Time(2)](https://user-images.githubusercontent.com/14386/173288685-91952ade-f304-4825-8bfb-0725a71ca17b.png) ![Size of Cache in MiB over Time](https://user-images.githubusercontent.com/14386/173291421-e6b410be-ac77-479b-ad24-5fafcebf81eb.png) Note that on cache drops mergebase's memory doesnt go so far down because it does not free the `CCoinsMap` bucket array. ![Size of Cache in Million tx over Time(1)](https://user-images.githubusercontent.com/14386/173288703-a80c9c9e-93c8-4a16-9df8-610c89c61cc4.png) ACKs for top commit: LarryRuane: ACK 9f947fc3d4b779f017332135323b34e8f216f613 achow101: re-ACK 9f947fc3d4b779f017332135323b34e8f216f613 john-moffett: ACK 9f947fc3d4b779f017332135323b34e8f216f613 jonatack: re-ACK 9f947fc3d4b779f017332135323b34e8f216f613 Tree-SHA512: 48caf57d1775875a612b54388ef64c53952cd48741cacfe20d89049f2fb35301b5c28e69264b7d659a3ca33d4c714d47bafad6fd547c4075f08b45acc87c0f45
2023-04-20Merge bitcoin/bitcoin#27214: addrman: Enable selecting addresses by networkAndrew Chow
17e705428ddf80c7a7f31fe5430d966cf08a37d6 doc: clarify new_only param for Select function (Amiti Uttarwar) b0010c83a1b4a3d21719cb68e37faf9b1172522a bench: test select for a new table with only one address (Amiti Uttarwar) 9b91aae08579c77d2fd5506804c8e2e0cda0d274 bench: add coverage for addrman select with network parameter (Amiti Uttarwar) 22a4d1489c0678a90c00318203cfce61672f20b7 test: increase coverage of addrman select (without network) (Amiti Uttarwar) a98e542e0c18f7cb2340179631806f14b07430c3 test: add addrman test for special case (Amiti Uttarwar) 5c8b4baff27e0ccd27fda6e915b956d1e8dd7ce2 tests: add addrman_select_by_network test (Amiti Uttarwar) 6b229284fd2209938ee8fdffed4d080395b3aa05 addrman: add functionality to select by network (Amiti Uttarwar) 26c3bf11e2487ed0ac578fb92619c148336003cb scripted-diff: rename local variables to match modern conventions (Amiti Uttarwar) 48806412e2bcd023b78fc05f6c9ce092360d1db1 refactor: consolidate select logic for new and tried tables (Amiti Uttarwar) ca2a9c5f8f14b792a14e81f73b1910a4c8799b93 refactor: generalize select logic (Amiti Uttarwar) 052fbcd5a791855406141e85d32e42e297220fe9 addrman: Introduce helper to generalize looking up an addrman entry (Amiti Uttarwar) 9bf078f66c8f286e1ab5e34b8eeed7d80290a897 refactor: update Select_ function (Amiti Uttarwar) Pull request description: For the full context & motivation of this patch, see #27213 This is joint work with mzumsande. This PR adds functionality to `AddrMan::Select` to enable callers to specify a network they are interested in. Along the way, it refactors the function to deduplicate the logic, updates the local variables to match modern conventions, adds test coverage for both the new and existing `Select` logic, and adds bench tests for the worst case performance of both the new and existing `Select` logic. This functionality is used in the parent PR. ACKs for top commit: vasild: ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 brunoerg: re-ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 ajtowns: ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 mzumsande: Code Review ACK 17e705428ddf80c7a7f31fe5430d966cf08a37d6 Tree-SHA512: e99d1ce0c44a15601a3daa37deeadfc9d26208a92969ecffbea358d57ca951102d759734ccf77eacd38db368da0bf5b6fede3cd900d8a77b3061f4adc54e52d8
2023-04-20p2p: update hardcoded mainnet seeds for 25.xJon Atack
2023-04-19blockstorage: Adjust fastprune limit if block exceeds blockfile sizeMartin Zumsande
If the added block exceeds the blockfile size in test-only -fastprune mode, the node would get stuck in an infinite loop and run out of memory. Avoid this by raising the blockfile size to the size of the added block in this situation. Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-04-19move-only: Extract common/args and common/config.cpp from util/systemTheCharlatan
This is an extraction of ArgsManager related functions from util/system into their own common file. Config file related functions are moved to common/config.cpp. The background of this commit is an ongoing effort to decouple the libbitcoinkernel library from the ArgsManager. The ArgsManager belongs into the common library, since the kernel library should not depend on it. See doc/design/libraries.md for more information on this rationale.
2023-04-18kernel: update m_assumed_* chain params for 25.xfanquake
Co-authored-by: johnny9 <985648+johnny9@users.noreply.github.com>
2023-04-18kernel: update chainTxData for 25.xfanquake
Co-authored-by: johnny9 <985648+johnny9@users.noreply.github.com>
2023-04-18kernel: update nMinimumChainWork & defaultAssumeValid for 25.xfanquake
Co-authored-by: johnny9 <985648+johnny9@users.noreply.github.com>
2023-04-18doc: update references to kernel/chainparams.cppfanquake
2023-04-17Merge bitcoin/bitcoin#27473: bugfix: Properly handle "unknown" Address TypeAndrew Chow
0d6383fda04a99726654945a737bbb1369e0e44a Don't return OutputType::UNKNOWN in ParseOutputType (Pttn) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/27472 by also handling at the relevant places the case where ParseOutputType returns `OutputType::UNKNOWN`, and not just when it returns `std::nullopt`. ACKs for top commit: achow101: ACK 0d6383fda04a99726654945a737bbb1369e0e44a MarcoFalke: lgtm ACK 0d6383fda04a99726654945a737bbb1369e0e44a furszy: ACK https://github.com/bitcoin/bitcoin/commit/0d6383fda04a99726654945a737bbb1369e0e44a Tree-SHA512: 776793027b926283d3162e69fb9c8883c814b19bcce4574ccdf8e3140a1ec4ebc4aa8ccd1abae7ef3571f942d2e6c35305fd1244259540d90605106e01afc34c
2023-04-17Merge bitcoin/bitcoin#27468: bugfix: rest: avoid segfault for invalid URIfanquake
11422cc5720c8d73a87600de8fe8abb156db80dc bugfix: rest: avoid segfault for invalid URI (pablomartin4btc) Pull request description: Minimal fix to get it promptly into 25.0 release (suggested by [stickies-v](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385130381) and supported by [vasild](https://github.com/bitcoin/bitcoin/pull/27253#pullrequestreview-1385842606) ) Please check #27253 for reviewers comments and acks regarding this PR and read the commit comment message body for more details about the fix. ACKs for top commit: achow101: ACK 11422cc5720c8d73a87600de8fe8abb156db80dc stickies-v: re-ACK 11422cc Tree-SHA512: 5af6b53fb266a12b463f960910556d5e97bc88b3c2a4f437ffa343886b38749e1eb058cf7bc64d62e82e1acf6232a186bddacd8f3b4500c87bf9e550a0153386
2023-04-17bugfix: rest: avoid segfault for invalid URIpablomartin4btc
`evhttp_uri_parse` can return a nullptr, for example when the URI contains invalid characters (e.g. "%"). `GetQueryParameterFromUri` passes the output of `evhttp_uri_parse` straight into `evhttp_uri_get_query`, which means that anyone calling a REST endpoint in which query parameters are used (e.g. `rest_headers`) can cause a segfault. This bugfix is designed to be minimal and without additional behaviour change. Follow-up work should be done to resolve this in a more general and robust way, so not every endpoint has to handle it individually.