aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-12-21Merge bitcoin/bitcoin#26265: POLICY: Relax MIN_STANDARD_TX_NONWITNESS_SIZE ↵Andrew Chow
to 65 non-witness bytes b2aa9e85289fc654106a890c35935e9c76c411fb Add release note for MIN_STANDARD_TX_NONWITNESS_SIZE relaxation (Greg Sanders) 8c5b3646b5afe8a61f5c66478d8e11f0d2ce5108 Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytes (Greg Sanders) Pull request description: Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was proposed as a consensus change, and is the simpler of the two suggestions. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes Related mailing list discussions here: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-October/020995.html And a couple years earlier: https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2020-May/017883.html ACKs for top commit: achow101: reACK b2aa9e85289fc654106a890c35935e9c76c411fb glozow: reACK b2aa9e85289fc654106a890c35935e9c76c411fb pablomartin4btc: re-ACK https://github.com/bitcoin/bitcoin/commit/b2aa9e85289fc654106a890c35935e9c76c411fb jonatack: ACK b2aa9e85289fc654106a890c35935e9c76c411fb with some suggestions Tree-SHA512: c1ec1af9ddcf31b2272209a4f1ee0c5607399f8172e5a1dfd4604cf98bfb933810dd9369a5917ad122add003327c9fcf6ee26995de3aca41d5c42dba527991ad
2022-12-21Merge bitcoin/bitcoin#26734: doc: Fixup getrawtransaction RPC docsfanquake
97115de1839c69d63cae6ea772c17905739bf0ae doc: Refactor/Format getrawtransaction RPC docs and add ScriptPubKeyDoc function (Douglas Chimento) Pull request description: Added `ScriptPubKeyDoc` function ACKs for top commit: MarcoFalke: ACK 97115de1839c69d63cae6ea772c17905739bf0ae kristapsk: cr utACK 97115de1839c69d63cae6ea772c17905739bf0ae Tree-SHA512: 1371375986177862e8c99923eb7f1800fef8da7a7ac9f0ec9037bf5b23681c3348d5afe913aab7457f029ee1774d160ac10d7f57238500a03c6385cc0c7013fc
2022-12-21Merge bitcoin-core/gui#687: Load PSBTs using istreambuf_iterator rather than ↵MarcoFalke
istream_iterator bb5ea1d9a954b7b9f443ee8fbbb04549cd0b08a7 qt: Load PSBTs using istreambuf_iterator rather than istream_iterator (Andrew Chow) Pull request description: `istream_iterator` eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters. `istreambuf_iterator` is the correct thing to use here. This is a regression in 24.0. https://github.com/bitcoin/bitcoin/pull/25001 accidentally changed the original `istreambuf_iterator` to `istream_iterator`. ACKs for top commit: furszy: Tested ACK bb5ea1d9 MarcoFalke: review ACK bb5ea1d9a954b7b9f443ee8fbbb04549cd0b08a7 🍇 Tree-SHA512: 35d90eee3efdcb6a360af69ac1727f9f2837ea621297196de3136299f5de6d9975df4e425e1fc5b8813c1ddb2a4d60c3969e1d5d968953a4628ca45e37d3bf05
2022-12-21doc: Refactor/Format getrawtransaction RPC docs and add ScriptPubKeyDoc functionDouglas Chimento
2022-12-20Merge bitcoin/bitcoin#21576: rpc, gui: bumpfee signer supportAndrew Chow
2c07cfacd1745844a1d3c57f2e8617549b9815d7 gui: bumpfee signer support (Sjors Provoost) 7e02a3329797211ed5d35e5f5e7b919c099b78ba rpc: bumpfee signer support (Sjors Provoost) 304ece994504220c355577170409b9200941f2af rpc: document bools in FillPSBT() calls (Sjors Provoost) Pull request description: The `bumpfee` RPC call and GUI fee bump interface now work with an external signer. ACKs for top commit: achow101: ACK 2c07cfacd1745844a1d3c57f2e8617549b9815d7 furszy: code review ACK 2c07cfac jarolrod: tACK 2c07cfa Tree-SHA512: 0c7b931f76fac67c9e33b9b935f29af6f69ac67a5ffcc586ed2f1676feac427735b1d971723b29ef332bb6fb5762949598ebbf728587e8f0ded95a9bfbb3e7a4
2022-12-20Merge bitcoin-core/gui#605: Delete splash screen widget earlyHennadii Stepanov
1b228497fa729c512a15bdfa80f61a610abfe8a5 qt: Drop no longer used `SplashScreen::finish()` slot (Hennadii Stepanov) 10811afff40efed1fda7eecab89884eaadd7146c qt: Drop no longer used `BitcoinApplication::splashFinished()` signal (Hennadii Stepanov) 5299cfe371ddad806b1c4538d17cde069e25b1c1 qt: Delete splash screen widget explicitly (Hennadii Stepanov) Pull request description: Fixes bitcoin-core/gui#604. Fixes bitcoin/bitcoin#25146. Fixes bitcoin/bitcoin#26340. `SplashScreen::deleteLater()` [does not guarantee](https://doc.qt.io/qt-5/qobject.html#deleteLater) deletion of the `m_splash` object prior to the wallet context deletion. If the latter happens first, the [segfault](https://github.com/bitcoin-core/gui/issues/604#issuecomment-1133907013) follows. ACKs for top commit: dooglus: ACK https://github.com/bitcoin-core/gui/commit/1b228497fa729c512a15bdfa80f61a610abfe8a5 furszy: ACK 1b228497 john-moffett: ACK 1b228497fa729c512a15bdfa80f61a610abfe8a5 Tree-SHA512: bb01d0bf2051f5b184dc415c4f5d32dfb7b8bd772feff7ec7754ded4c6482de27f004b9712df7d53c5ee82e153f48aef4372e4a49d7bcbbb137f73e9b4947962
2022-12-19Relax MIN_STANDARD_TX_NONWITNESS_SIZE to 65 non-witness bytesGreg Sanders
Since the original fix was set to be a "reasonable" transaction to reduce allocations and the true motivation later revealed, it makes sense to relax this check to something more principled. There are more exotic transaction patterns that could take advantage of a relaxed requirement, such as 1 input, 1 output OP_RETURN to burn a utxo to fees for CPFP purposes when change isn't practical. Two changes could be accomplished: 1) Anything not 64 bytes could be allowed 2) Anything above 64 bytes could be allowed In the Great Consensus Cleanup, suggestion (2) was the route taken. It would not allow an "empty" OP_RETURN but would reduce the required padding from 22 bytes to 5. The functional test is also modified to test the actual case we care about: 64 bytes
2022-12-19Merge bitcoin/bitcoin#26515: rpc: skip getpeerinfo for a peer without ↵MarcoFalke
CNodeStateStats 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 rpc: Require NodeStateStats object in getpeerinfo (Martin Zumsande) Pull request description: The objects `CNode`, `CNodeState` and `Peer` store different info about a peer - `InitializeNode()` and `FinalizeNode()` make sure that for the duration of a connection, we should always have one of each for a peer. Therefore, there is no situation in which, as part of getpeerinfo RPC, `GetNodeStateStats()` (which requires a `CNodeState` and a `Peer` entry for a `NodeId` to succeed) could fail for a legitimate reason while the peer is connected - this can only happen if there is a race condition between peer disconnection and the `getpeerinfo` processing (see also a more detailed description of this in https://github.com/bitcoin/bitcoin/pull/26457#pullrequestreview-1181641835). But in this case I think it's better to just not include the newly disconnected peer in the response instead of returning just parts of its data. An earlier version of this PR also made the affected `CNodeStateStats` fields non-optional (see https://github.com/mzumsande/bitcoin/commit/5f900e27d0e5ceaa3b800a2eb5a8e8ff6bccad3b). Since this conflicts with #25923 and should be a separate discussion, I removed that commit from this PR. ACKs for top commit: dergoegge: Approach ACK 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 MarcoFalke: review ACK 6fefd49527fa0ed9535e54f2a3e76fe2599b2350 👒 Tree-SHA512: 89c8f7318df4634c1630415de9c8350e6dc2d14d9d07e039e5b180c51bfd3ee2ce99eeac4f9f858af7de846f7a6b48fcae96ebac08495b30e431a5d2d4660532
2022-12-19Merge bitcoin/bitcoin#25311: refactor: remove CBlockIndex copy constructionfanquake
36c201feb74bbb87d22bd956373dbbb9c47fb7e7 remove CBlockIndex copy construction (James O'Beirne) Pull request description: Copy construction of CBlockIndex objects is a footgun because of the wide use of equality-by-pointer comparison in the code base. There are also potential lifetime confusions of using copied instances, since there are recursive pointer members (e.g. pprev). (See also https://github.com/bitcoin/bitcoin/pull/24008#discussion_r891949166) We can't just delete the copy constructors because they are used for derived classes (CDiskBlockIndex), so we mark them protected. ACKs for top commit: ajtowns: ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 - code review only MarcoFalke: re-ACK 36c201feb74bbb87d22bd956373dbbb9c47fb7e7 🏻 Tree-SHA512: b1cf9a1cb992464a4377dad609713eea63cc099435df374e4553bfe62d362a4eb5e3c6c6649177832f38c0905b23841caf9d62196cef8e3084bfea0bfc26374b
2022-12-18qt: Load PSBTs using istreambuf_iterator rather than istream_iteratorAndrew Chow
istream_iterator eats whitespace charactesr which causes parsing failures for PSBTs that contain the bytes corresponding to those characters.
2022-12-17Merge bitcoin/bitcoin#26710: refactor: Fix `performance-for-range-copy` in ↵MarcoFalke
headers 48033d43dc30fcee2c9a38c828d2f58d3bc11827 clang-tidy: Fix `performance-for-range-copy` in headers (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26705 as was requested in https://github.com/bitcoin/bitcoin/pull/26705#issuecomment-1353293405. To test this PR, consider applying a diff as follows: ```diff --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -12,17 +12,9 @@ readability-redundant-declaration, readability-redundant-string-init, ' WarningsAsErrors: ' -bugprone-argument-comment, -bugprone-use-after-move, -misc-unused-using-decls, -modernize-use-default-member-init, -modernize-use-nullptr, performance-for-range-copy, -performance-move-const-arg, -performance-unnecessary-copy-initialization, -readability-redundant-declaration, -readability-redundant-string-init, ' CheckOptions: - key: performance-move-const-arg.CheckTriviallyCopyableMove value: false +HeaderFilterRegex: '.' ``` ACKs for top commit: MarcoFalke: review ACK 48033d43dc30fcee2c9a38c828d2f58d3bc11827 Tree-SHA512: eaf7a0e9b4fdc4ce788f78e5675632f3c278fc24bee2434874cbabc3e25ad7059b0c53ab7834908e901872d5afee08acba860542b03454c09fe129be6ad03f09
2022-12-17Merge bitcoin/bitcoin#26708: clang-tidy: Fix `modernize-use-nullptr` in headersMarcoFalke
adb7dba9de95c166103ac7eaf97d5bd83dc19605 clang-tidy: Fix `modernize-use-nullptr` in headers (Hennadii Stepanov) Pull request description: Split from bitcoin/bitcoin#26705 as was requested in https://github.com/bitcoin/bitcoin/pull/26705#issuecomment-1353293405. To test this PR, consider applying a diff as follows: ```diff --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -12,17 +12,9 @@ readability-redundant-declaration, readability-redundant-string-init, ' WarningsAsErrors: ' -bugprone-argument-comment, -bugprone-use-after-move, -misc-unused-using-decls, -modernize-use-default-member-init, modernize-use-nullptr, -performance-for-range-copy, -performance-move-const-arg, -performance-unnecessary-copy-initialization, -readability-redundant-declaration, -readability-redundant-string-init, ' CheckOptions: - key: performance-move-const-arg.CheckTriviallyCopyableMove value: false +HeaderFilterRegex: '.' ``` ACKs for top commit: john-moffett: ACK adb7dba9de95c166103ac7eaf97d5bd83dc19605 Tree-SHA512: 67241fb212d837157a0a26f0d59e7f30a9d270d5b0ebfeb6ad9631e460fc7fba8c9a9dcd4c0520789353f68025a9f090f40f17176472a93cce1411e6d56f930b
2022-12-17Merge bitcoin/bitcoin#26120: refactor: Make bitcoin-util grind_task tsan ↵MarcoFalke
friendly fafcc9439838b3f084fc054b91bca4b50ee62df5 Make bitcoin-util grind_task tsan friendly (MacroFake) Pull request description: While there is no issue with the current code, `libtsan-12.2.1` on my machine does not seem to like it. This is understandable, because the nonce isn't protected by a mutex that the sanitizer can see (only by an atomic, which achieves the same). Fix this by guarding the nonce by the existing atomic bool, which tsan seems to understand. ACKs for top commit: ajtowns: ACK fafcc9439838b3f084fc054b91bca4b50ee62df5 hebasto: ACK fafcc9439838b3f084fc054b91bca4b50ee62df5, I have reviewed the code and it looks OK, I agree it can be merged. Confirming that initial bug has been fixed. Tree-SHA512: 4e67fab5833ec7d91678b85a300368892ee9f7cd89a52cc5e15a7df65b2da813b24eaffd8362d0d8a3c8951e024041d69ebddf25101b11d0a1a62c1208ddc9a5
2022-12-16Merge bitcoin/bitcoin#24865: rpc: Enable wallet import on pruned nodes and ↵Andrew Chow
add test 564b580bf07742483a140c7c095b896a6d5d6cad test: Introduce MIN_BLOCKS_TO_KEEP constant (Aurèle Oulès) 71d9a7c03b44236c2fea2b74f92a69234d29f717 test: Wallet imports on pruned nodes (Aurèle Oulès) e6906fcf9e4d5692ead6c9bf5a2e11673315a1f5 rpc: Enable wallet import on pruned nodes (Aurèle Oulès) Pull request description: Reopens #16037 I have rebased the PR, addressed the comments of the original PR and added a functional test. > Before this change importwallet fails if any block is pruned. This PR makes it possible to importwallet if all required blocks aren't pruned. This is possible because the dump format includes key timestamps. For reviewers: `python test/functional/wallet_pruning.py --nocleanup` will generate a large blockchain (~700MB) that can be used to manually test wallet imports on a pruned node. Node0 is not pruned, while node1 is. ACKs for top commit: kouloumos: ACK 564b580bf07742483a140c7c095b896a6d5d6cad achow101: reACK 564b580bf07742483a140c7c095b896a6d5d6cad furszy: ACK 564b580 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/24865/commits/564b580bf07742483a140c7c095b896a6d5d6cad Tree-SHA512: b345a6c455fcb6581cdaa5f7a55d79e763a55cb08c81d66be5b12794985d79cd51b9b39bdcd0f7ba0a2a2643e9b2ddc49310ff03d16b430df2f74e990800eabf
2022-12-16clang-tidy: Fix `performance-for-range-copy` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/performance/for-range-copy.html
2022-12-16Make bitcoin-util grind_task tsan friendlyMacroFake
This does not change behavior of the bitcoin-util binary.
2022-12-15clang-tidy: Fix `readability-redundant-string-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/readability/redundant-string-init.html
2022-12-15clang-tidy: Fix `modernize-use-nullptr` in headersHennadii Stepanov
https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-nullptr.html
2022-12-15remove CBlockIndex copy constructionJames O'Beirne
Copy construction of CBlockIndex objects is a footgun because of the wide use of equality-by-pointer comparison in the code base. There are also potential lifetime confusions of using copied instances, since there are recursive pointer references (e.g. pprev). We can't just delete the copy constructors because they are used for derived classes (CDiskBlockIndex), so we mark them protected. Delete move constructors and declare the destructor to satisfy the "rule of 5."
2022-12-14Merge bitcoin/bitcoin#26668: wallet: if only have one output type, don't ↵Andrew Chow
perform "mixed" coin selection 89c1491d35389eac0c1fecc59333cdfae3b1bd2c wallet: if only have one output type, don't perform "mixed" coin selection (furszy) Pull request description: For wallets that only have one output type, we are currently performing the same selection process over the same coins twice. The "mixed coin selection" doesn't add any value to the result (there is nothing to mix if the available coins struct has only one type). ACKs for top commit: achow101: ACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c john-moffett: ACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c kristapsk: cr utACK 89c1491d35389eac0c1fecc59333cdfae3b1bd2c Tree-SHA512: 672eaeed3ba911d13fa61a46f719c8fe1ebe4d2dc7d723040e71937c693659411bc99cdbd9f0014e836b70eebeff1b8ca861f4d81d39e6f79f437364a526edbe
2022-12-13Merge bitcoin/bitcoin#23319: rpc: Return fee and prevout (utxos) to ↵Andrew Chow
getrawtransaction f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 rpc: Return fee and prevout(s) to getrawtransaction (Douglas Chimento) Pull request description: Add fee response in BTC to getrawtransaction #23264 ### For Reviewers * Verbose arg is now an int * Verbose = 2 includes a `fee` field and `prevout` * [./test/functional/rpc_rawtransaction.py](./test/functional/rpc_rawtransaction.py) contains a new test to validate fields of new verbosity 2 (not the values) ``` bitcoin-cli -chain=test getrawtransaction 9ae533f7da9be4a34997db78343a8d8d6d6186b6bba3959e56f416a5c70e7de4 2 000000000000001d442e556146d5f2841d85150c200e8d8b8a4b5005b13878f6 ``` ``` "in_active_chain": true, "txid": "9ae533f7da9be4a34997db78343a8d8d6d6186b6bba3959e56f416a5c70e7de4", "hash": "7f23e3f3a0a256ddea1d35ffd43e9afdd67cc68389ef1a804bb20c76abd6863e", .... "vin": [ { "txid": "23fc75d6d74f6f97e225839af69ff36a612fe04db58a4414ec4828d1749a05a0", "vout": 0, "scriptSig": { "asm": "", "hex": "" }, "prevout": { "generated": false, "height": 2099486, "value": 0.00017764, "scriptPubKey": { "asm": "0 7846ce1ced3253d8bd43008db2ca364cc722f5a2", "hex": "00147846ce1ced3253d8bd43008db2ca364cc722f5a2", "address": "tb1q0prvu88dxffa302rqzxm9j3kfnrj9adzk49mlp", "type": "witness_v0_keyhash" } }, "sequence": 4294967295 }, ... "fee": 0.00000762 } ``` ACKs for top commit: achow101: ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 aureleoules: ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 hernanmarino: re ACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 pablomartin4btc: re-tACK f86697163e5cdbc3bc4a65cfb7dbaa3d9eb602a9 Tree-SHA512: 591fdc285d74fa7803e04ad01c7b70bc20fac6b1369e7bd5b8e2cde9b750ea52d6c70d79225b74bef4f4bbc0fb960877778017184e146119da4a55f9593d1224
2022-12-13Merge bitcoin-core/gui#682: Don't directly delete abandoned txes from GUIHennadii Stepanov
e75d2276324d54a01971afdf531df91748275bd5 Minor fix: Don't directly delete abandoned txes (John Moffett) Pull request description: This fully closes bitcoin/bitcoin#12179. Currently, when a user abandons a transaction by clicking "Abandon Transaction" in the context menu, a call is made to remove it from the GUI view: `model->getTransactionTableModel()->updateTransaction(hashQStr, CT_UPDATED, false);` (The `false` parameter is for `bool showTransaction`) This behavior is probably unwanted, as the transaction is not actually removed from the wallet and would show up again if the node is restarted. However, the previous line, `model->wallet().abandonTransaction(hash);`, changes the underlying model and calls `NotifyTransactionChanged(wtx.GetHash(), CT_UPDATED);`, which queues a signal that eventually calls back to `updateTransaction`, this time with `showTransaction` set to `true`. This runs on a separate thread, so it gets called *after* the 'subsequent' `updateTransaction`. The transaction gets removed from the GUI and immediately added back. In a nutshell, `updateTransaction` gets called twice. The first (direct) call deletes the transaction from the GUI. The second (sent via a queued signal) brings it back to the GUI. The first direct call is redundant and unwanted. Worse, if the `abandonTransaction` call fails for any reason, the transaction still gets removed from the GUI. (This is what caused bitcoin#12179. It can still be triggered if, eg., a user clicks "Abandon Transaction" the moment after a new block is found.) There are no conditions (to my knowledge) where an abandoned transaction should be directly removed from the GUI. If the underlying model changes, the deletion should be reflected anyway by the queued signal to `updateTransaction`. The behavior is borne out by the QT logs. To reproduce, send a transaction with RBF enabled, then bump the fee, then 'abandon transaction' on the first transaction. The logs will show something like this: ``` 2022-11-28T14:48:00Z [qt] GUI: "NotifyTransactionChanged: 2c5811484f1adec92a739a5e70b453b03eaed0f7cc0538fbd0ee1589e586b951 status= 1" 2022-11-28T14:48:00Z [qt] GUI: "TransactionTablePriv::updateWallet: 2c5811484f1adec92a739a5e70b453b03eaed0f7cc0538fbd0ee1589e586b951 1" 2022-11-28T14:48:00Z [qt] GUI: " inModel=1 Index=381-382 showTransaction=0 derivedStatus=2" 2022-11-28T14:48:00Z [qt] GUI: "TransactionTablePriv::updateWallet: 2c5811484f1adec92a739a5e70b453b03eaed0f7cc0538fbd0ee1589e586b951 1" 2022-11-28T14:48:00Z [qt] GUI: " inModel=0 Index=381-381 showTransaction=1 derivedStatus=0" ``` Notice the duplicate `updateWallet` calls with different `showTransaction` values. ACKs for top commit: hebasto: ACK e75d2276324d54a01971afdf531df91748275bd5 jarolrod: tACK e75d2276324d54a01971afdf531df91748275bd5 Tree-SHA512: 00f150f747c2ee1605af861a21d5c3b9773a4a9985e8dab62e48bd32885b1bfa4e8cbf805ad61af77aec9d3ccefaed3f4311a29086aa8c22d55d5326ba68ece6
2022-12-13Merge bitcoin/bitcoin#26643: wallet: Move fee underpayment check to after ↵Andrew Chow
all fee has been set 798430d127521d088c081ee625912a704f415990 wallet: Sanity check fee paid cannot be negative (Andrew Chow) c1a84f108e320bd44c172a4dd3bb486ab777ff69 wallet: Move fee underpayment check to after fee setting (Andrew Chow) e5daf976d5b064b585029d4bb38d68a8153ea13b wallet: Rename nFeeRet in CreateTransactionInternal to current_fee (Andrew Chow) Pull request description: Currently the fee underpayment check occurs right after we calculate what the transaction's fee should be. However the fee paid by the transaction at that time does not always match. Notably, when doing SFFO, the fee paid at that time will almost always be less than the fee required, which then required having a bypass of the underpayment check that results in SFFO payments going through when they should not. This PR moves the underpayment check to after fees have been finalized so that we always check whether the fee is being underpaid. This removes the exception for SFFO and unifies this behavior for both SFFO and non-SFFO txs. ACKs for top commit: S3RK: Code review ACK 798430d127521d088c081ee625912a704f415990 furszy: Code review ACK 798430d glozow: utACK 798430d127, code looks correct to me Tree-SHA512: 720e8a3dbdc9937b12ee7881eb2ad58332c9584520da87ef3080e6f9d6220ce8d3bd8b9317b4877e56a229113437340852976db8f64df0d5cc50723fa04b02f0
2022-12-13Merge bitcoin/bitcoin#26628: RPC: Reject RPC requests with same named ↵MarcoFalke
parameter specified multiple times 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa test: Suggested cleanups for rpc_namedparams test (Ryan Ofsky) d1ca56382512df3084fce7353bf1e8b66cae61bc bitcoin-cli: Make it an error to specify the "args" parameter two different ways (Ryan Ofsky) 6bd1d20b8cf27aa72ec2907342787e6fc9f94c50 rpc: Make it an error server-side to specify same named parameter multiple times (Ryan Ofsky) e2c3b18e671e347e422d696d1cbdd9f82b2ce468 test: Add RPC tests for same named parameter specified more than once (Ryan Ofsky) Pull request description: Make the JSON-RPC server reject requests with the same named parameter specified multiple times, instead of silently overwriting earlier parameter values with later ones. Generally JSON keys are supposed to unique, and their order isn't supposed to be significant, so having the server silently discard duplicate keys is error-prone. Most likely if an RPC client is sending a request with duplicate keys it means something is wrong with the request and there should be an error. After this change, named parameters are still allowed to specified multiple times on the `bitcoin-cli` command line, since `bitcoin-cli` automatically replaces earlier values with later values before sending the JSON-RPC request. This makes sense, since it's not unusual for the order of command line options to be significant or for later command line options to override earlier ones. ACKs for top commit: MarcoFalke: review ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa 🗂 kristapsk: ACK 8c3ff7d52ae3314959e1e66da8718a3f0d30abaa stickies-v: ACK 8c3ff7d52 Tree-SHA512: 2d1357dcc2c171da287aeefc7b333ba4e67babfb64fc14d7fa0940256e18010a2a65054f3bf7fa1571b144d2de8b82d53076111b5f97ba29320cfe84b6ed986f
2022-12-13Merge bitcoin/bitcoin#26477: validation: fix broken maxtipage comparisonfanquake
e4be0e9b0661a8af49c4e6d5472804913f04b8fc test: add -maxtipage test for the maximum allowable value (James O'Beirne) a451e832b46bcb984dfcd9478ea8ebb8b3de0c62 fix: validation: cast now() to seconds for maxtipage comparison (James O'Beirne) Pull request description: Since https://github.com/bitcoin/bitcoin/commit/faf44876db555f7488c8df96db9fa88b793f897c, the maxtipage comparison in IsInitialBlockDownload() has been broken, since the NodeClock::now() time_point is in the system's native denomination (nanoseconds). Without this patch, specifying the maximum allowable -maxtipage (9223372036854775807) results in a SIGABRT crash: ``` % gdb --args ./src/bitcoind -maxtipage=9223372036854775207 -minimumchainwork=0x00 -stopatheight=30000 ... 2022-11-09T15:55:17Z [dnsseed] dnsseed thread exit [Thread 0x7fff937fe640 (LWP 69883) exited] Thread 29 "b-msghand" received signal SIGABRT, Aborted. [Switching to Thread 0x7fff91ffb640 (LWP 69886)] __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 44 ./nptl/pthread_kill.c: No such file or directory. (gdb) bt #0 __pthread_kill_implementation (threadid=<optimized out>, signo=signo@entry=6, no_tid=no_tid@entry=0) at ./nptl/pthread_kill.c:44 #1 0x00007ffff768989f in __pthread_kill_internal (signo=6, threadid=<optimized out>) at ./nptl/pthread_kill.c:78 #2 0x00007ffff763da52 in __GI_raise (sig=sig@entry=6) at ../sysdeps/posix/raise.c:26 #3 0x00007ffff7628469 in __GI_abort () at ./stdlib/abort.c:79 #4 0x00007ffff7cf79a4 in __mulvdi3 () from /lib/x86_64-linux-gnu/libgcc_s.so.1 #5 0x00005555558d13ab in std::chrono::__duration_cast_impl<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, std::ratio<1000000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:521 #6 std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__d=...) at /usr/include/c++/12/bits/chrono.h:260 #7 std::chrono::duration<long, std::ratio<1l, 1000000000l> >::duration<long, std::ratio<1l, 1l>, void> (__d=..., this=<optimized out>) at /usr/include/c++/12/bits/chrono.h:514 #8 std::chrono::operator-<long, std::ratio<1l, 1000000000l>, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...) at /usr/include/c++/12/bits/chrono.h:650 #9 std::chrono::operator-<NodeClock, std::chrono::duration<long, std::ratio<1l, 1000000000l> >, long, std::ratio<1l, 1l> > (__rhs=..., __lhs=...) at /usr/include/c++/12/bits/chrono.h:1020 #10 Chainstate::IsInitialBlockDownload (this=0x555556071940) at ./src/validation.cpp:1545 #11 0x00005555556efd1e in operator() (__closure=<optimized out>) at ./src/net_processing.cpp:3369 #12 (anonymous namespace)::PeerManagerImpl::ProcessMessage (this=0x555556219be0, pfrom=..., msg_type=..., vRecv=..., time_received=..., interruptMsgProc=...) at ./src/net_processing.cpp:3369 #13 0x00005555556f75cc in (anonymous namespace)::PeerManagerImpl::ProcessMessages (this=0x555556219be0, pfrom=<optimized out>, interruptMsgProc=std::atomic<bool> = { false }) at ./src/net_processing.cpp:4985 #14 0x00005555556a83c9 in CConnman::ThreadMessageHandler (this=0x5555560ebc70) at ./src/net.cpp:2014 #15 0x0000555555c4d5d6 in std::function<void ()>::operator()() const (this=0x7fff91ffadb0) at /usr/include/c++/12/bits/std_function.h:591 #16 util::TraceThread(std::basic_string_view<char, std::char_traits<char> >, std::function<void ()>) ( thread_name="0\255\377\221\377\177\000\000\v\000\000\000\000\000\000\000TraceThread\000\000\000\000\000P\255\377\221\377\177\000\000\017\000\000\000\000\000\000\000util/thread.cpp\000\000\000\000\000\000\000\000\000\000ihB鵿6\000\000\000\000\000\000\000\000\260\255\377\221\377\177\000\000\277\211\321UUU\000\000p\324\304UUU\000\000\002\000\000\000\000\000\000\000\240xh\367\377\177\000\000\000\000\000\000\000\000\000\000]\340iUUU\000\000p\274\016VUU\000\000\000\000\000\000\000\000\000\000\300\303iUUU\000\000p\206jUUU", '\000' <repeats 11 times>, "ihB鵿6\200\251!VUU\000\000"..., thread_func=...) at util/thread.cpp:21 #17 0x000055555569e05d in std::__invoke_impl<void, void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__f=<optimized out>) at /usr/include/c++/12/bits/invoke.h:61 #18 std::__invoke<void (*)(std::basic_string_view<char>, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > (__fn=<optimized out>) at /usr/include/c++/12/bits/invoke.h:96 #19 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::_M_invoke<0, 1, 2> (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:252 #20 std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > >::operator() (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:259 #21 std::thread::_State_impl<std::thread::_Invoker<std::tuple<void (*)(std::basic_string_view<char, std::char_traits<char> >, std::function<void()>), char const*, CConnman::Start(CScheduler&, const Options&)::<lambda()> > > >::_M_run(void) (this=<optimized out>) at /usr/include/c++/12/bits/std_thread.h:210 #22 0x00007ffff7ad43d3 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6 #23 0x00007ffff7687b27 in start_thread (arg=<optimized out>) at ./nptl/pthread_create.c:435 #24 0x00007ffff770a78c in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 (gdb) ``` ACKs for top commit: MarcoFalke: review ACK e4be0e9b0661a8af49c4e6d5472804913f04b8fc 🏽 Tree-SHA512: d892d6264a284d952a68a8631a6301277373b8df939dafd9e2652f2f22ab60712cde63b90c27c67ea2d05f02443452e3e4e1b9f25479bfaca00d4c4de13b9fbd
2022-12-12Merge bitcoin/bitcoin#26199: p2p: Don't self-advertise during version processingMarcoFalke
956c67059caf3807b3ceacdd5de1caaae538f009 refactor, doc: Improve SetupAddressRelay call in version processing (Martin Zumsande) 3c43d9db1e0f84279b08a93afd730caeece061a9 p2p: Don't self-advertise during VERSION processing (Gleb Naumenko) Pull request description: This picks up the last commit from #19843. Previously, we would prepare to self-announce to a new peer while parsing a `version` message from that peer. This is redundant, because we do something very similar in `MaybeSendAddr()`, which is called from `SendMessages()` after the version handshake is finished. There are a couple of differences: 1) `MaybeSendAddr()` self-advertises to all peers we do address relay with, not just outbound ones. 2) `GetLocalAddrForPeer()` called from `MaybeSendAddr()` makes a probabilistic decision to either advertise what they think we are or what we think we are, while `PushAddress()` on `version` deterministically only does the former if the address from the latter is unroutable. 3) During `version` processing, we haven't received a potential sendaddrv2 message from our peer yet, so self-advertisements with addresses from addrV2-only networks would always be dropped in `PushAddress()`. Since it's confusing to have two slightly different mechanisms for self-advertising, and the one in `MaybeSendAddr()` is better, remove the one in `version`. ACKs for top commit: stratospher: ACK 956c670 naumenkogs: ACK 956c67059caf3807b3ceacdd5de1caaae538f009 amitiuttarwar: reACK 956c67059c Tree-SHA512: 933d40615289f055c022170dde7bad0ac0a1d4be377538bfe9ba64375cfeb03bcd803901591f0739ac4850c880e8475a68fd1ab0330800030ab7f19e38c00274
2022-12-10Merge bitcoin/bitcoin#26666: refactor: Deleted unreachable code in ↵MarcoFalke
httpserver.cpp 8f5c560e11a9f3c7c5877f444caa605bdc296e81 refactor: Refactored RequestMethodString function to follow developer notes (JoaoAJMatos) 7fd3b9491b0e553aa7d60a68bda82968a79da8f5 refactor: Deleted unreachable code in httpserver.cpp (JoaoAJMatos) Pull request description: Some of the code in httpserver.cpp was unreachable, and didn't follow the developer notes. Continuation of [#26570 ](https://github.com/bitcoin/bitcoin/pull/26570) ACKs for top commit: stickies-v: re-ACK [8f5c560](https://github.com/bitcoin/bitcoin/commit/8f5c560e11a9f3c7c5877f444caa605bdc296e81) Tree-SHA512: ba8cf4c6dde9e2bb0ca9d63a0de86dfa37b070803dde71ac8384c261045835697a2335652cf5894511b3af8fd99f30e1cbda4e4234815b8b39538ade90fab3f9
2022-12-10Merge bitcoin/bitcoin#26673: univalue: Remove confusing getBool methodfanquake
293849a26088a16f6187dfdc36bef1d4d83ebb9d univalue: Remove confusing getBool method (Ryan Ofsky) Pull request description: Drop `UniValue::getBool` method because it is easy to confuse with the `UniValue::get_bool` method, and could potentially cause bugs. Unlike `get_bool`, `getBool` doesn't ensure that the value is a boolean and returns false for all integer, string, array, and object values instead of throwing an exception. The `getBool` method is also redundant because it is an alias for `isTrue`. There were only 5 `getBool()` calls in the codebase, so this commit replaces them with `isTrue()` or `get_bool()` calls as appropriate. These changes were originally made by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/26213 but were dropped to limit the scope of that PR. ACKs for top commit: justinpickering: ACK https://github.com/bitcoin/bitcoin/pull/26673/commits/293849a26088a16f6187dfdc36bef1d4d83ebb9d sipa: utACK 293849a26088a16f6187dfdc36bef1d4d83ebb9d w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26673/commits/293849a26088a16f6187dfdc36bef1d4d83ebb9d hebasto: ACK 293849a26088a16f6187dfdc36bef1d4d83ebb9d, also verified that the removed `getBool` method is not mentioned in any docs: furszy: ACK 293849a2 Tree-SHA512: 9fbfe5e2083410f123b18703a0cc0161ecbbb4958f331c9ff808dcfcc6ad499b0e896abd16fb8ea200c53ba29878db9812ce141e59cc5e0fd174741b0bcb192d
2022-12-10Merge bitcoin/bitcoin#26213: rpc: Strict type checking for RPC boolean ↵fanquake
parameters fa0153e609caf61a59efb0779e754861edc1684d refactor: Replace isTrue with get_bool (MarcoFalke) fa2cc5d1d66aa00e828d1bb65b9923f76fbdf4e1 bugfix: Strict type checking for RPC boolean parameters (MarcoFalke) Pull request description: ACKs for top commit: ryanofsky: Code review ACK fa0153e609caf61a59efb0779e754861edc1684d furszy: Code ACK fa0153e6 Tree-SHA512: b221f823c69d90c94447fd491071ff3659cfd512872b495ebc3e711f50633351974102c9ef7e50fa4a393c4131d349adea8fd41cc9d66f1f31e1f5e7a5f78757
2022-12-09wallet: Sanity check fee paid cannot be negativeAndrew Chow
We need to check that the fee is not negative even before it is finalized. The setting of fees for SFFO may adjust the fee to be "correct" and no longer negative, but erroneously reduce the amounts too far. So we need to check this condition before we do those adjustments.
2022-12-09wallet: Move fee underpayment check to after fee settingAndrew Chow
It doesn't make sense to be checking whether the fee paid is underpaying before we've finished setting the fees. So do that after we have done the reduction for SFFO and change adjustment for fee overpayment.
2022-12-09refactor: Refactored RequestMethodString function to follow developer notesJoaoAJMatos
Removed the default case in the switch statement in order to comply with the Developer Notes
2022-12-09refactor: Deleted unreachable code in httpserver.cppJoaoAJMatos
Removed all break statements from both RequestMethodString and GetRequestMethod functions as they were unreachable
2022-12-09test: Suggested cleanups for rpc_namedparams testRyan Ofsky
No changes in behavior, just implements review suggestions from https://github.com/bitcoin/bitcoin/pull/19762#discussion_r1025573943 https://github.com/bitcoin/bitcoin/pull/19762#discussion_r1035955247 https://github.com/bitcoin/bitcoin/pull/26628#discussion_r1038765894
2022-12-09univalue: Remove confusing getBool methodRyan Ofsky
Drop UniValue::getBool method because it is easy to confuse with the UniValue::get_bool method, and could potentially cause bugs. Unlike get_bool, getBool doesn't ensure that the value is a boolean and returns false for all integer, string, array, and object values instead of throwing an exceptions. The getBool method is also redundant because it is an alias for isTrue. There were only 5 getBool() calls in the codebase, so this commit replaces them with isTrue() or get_bool() calls as appropriate. These changes were originally made by MarcoFalke in https://github.com/bitcoin/bitcoin/pull/26213 but were dropped to limit the scope of that PR. Co-authored-by: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz>
2022-12-08wallet: if only have one output type, don't perform "mixed" coin selectionfurszy
there is nothing to mix.
2022-12-08Merge bitcoin/bitcoin#26378: refactor: Pass reference to last header, not ↵MarcoFalke
pointer fa579f306363ed03c1138121415b67b9b36b4d53 refactor: Pass reference to last header, not pointer (MacroFake) Pull request description: It is never a nullptr, otherwise an assertion would fire in UpdatePeerStateForReceivedHeaders. Passing a reference makes the code easier to read and less brittle. ACKs for top commit: john-moffett: ACK fa579f3 aureleoules: ACK fa579f306363ed03c1138121415b67b9b36b4d53 Tree-SHA512: 9725195663a31df57ae46bb7b11211cc4963a8f3d100f60332bfd4a3f3327a73ac978b3172e3007793cfc508dfc7c3a81aab57a275a6963a5ab662ce85743fd0
2022-12-08Merge bitcoin/bitcoin#26513: Make static nLastFlush and nLastWrite ↵fanquake
Chainstate members 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 Make static nLastFlush and nLastWrite Chainstate members (Aurèle Oulès) Pull request description: Fixes #22189. The `static std::multimap<uint256, FlatFilePos> mapBlocksUnknownParent; ` referenced in the issue was already fixed by #25571. I don't believe Chainstate references any other static variables. ACKs for top commit: jamesob: ACK 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 ([`jamesob/ackr/26513.1.aureleoules.make_static_nlastflush_a`](https://github.com/jamesob/bitcoin/tree/ackr/26513.1.aureleoules.make_static_nlastflush_a)) theStack: Concept and code-review ACK 07dfbb5bb8115c680621f361c65d9cde2f8c52f2 Tree-SHA512: 0f26463c079bbc5e0e62707d4ca4c8c9bbb99edfa3391d48d4915d24e2a1190873ecd4f9f11da25b44527671cdc82c41fd8234d56a4592a246989448d34406b0
2022-12-08rpc: Enable wallet import on pruned nodesAurèle Oulès
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com> Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2022-12-08Merge bitcoin/bitcoin#26308: rpc/rest/zmq: reduce LOCK(cs_main) scope: ~6 ↵MarcoFalke
times as many requests per second d7f61e7d5909da7227c9e34be06ce9eb872ba074 rpc: reduce LOCK(cs_main) scope in gettxoutproof (Andrew Toth) 4d92b5aabaf371225cdc37a7b114adc040bf4da5 rpc: reduce LOCK(cs_main) scope in GetUndoChecked and getblockstats (Andrew Toth) efd82aec8a2dd0fca8f2597c3f84cefe057d1243 rpc: reduce LOCK(cs_main) scope in blockToJSON (Andrew Toth) f00808e932c1f67bc76702a3f05778074b10025c rpc: reduce LOCK(cs_main) scope in GetBlockChecked and getblock (Andrew Toth) 7d253c943f44612431be894b198ffb49ff988fff zmq: remove LOCK(cs_main) from NotifyBlock (Andrew Toth) c75e3d2772b00acc3850f72a8cb733a0345aa773 rest: reduce LOCK(cs_main) scope in rest_block (Andrew Toth) Pull request description: Picking up from #21006. After commit https://github.com/bitcoin/bitcoin/commit/ccd8ef65f93ed82a87cee634660bed3ac17d9eb5 it is no longer required to hold `cs_main` when calling `ReadBlockFromDisk`. This can be verified in `master` at https://github.com/bitcoin/bitcoin/blob/master/src/node/blockstorage.cpp#L755. Same can be seen for `UndoReadFromDisk` https://github.com/bitcoin/bitcoin/blob/master/src/node/blockstorage.cpp#L485. The first commit moves `ReadBlockFromDisk` outside the lock scope in `rest_block`, where we can see a huge performance improvement when fetching blocks with multiple threads. My test setup, on an Intel i7 with 8 cores (16 threads): 1. Start a fully synced bitcoind, with this `bitcoin.conf`: ``` rest=1 rpcthreads=16 rpcworkqueue=64 rpcuser=user rpcpassword=password ``` 2. Run ApacheBench: 10000 requests, 16 parallel threads, fetching block nr. 750000 in binary: ``` ab -n 10000 -c 16 "http://127.0.0.1:8332/rest/block/0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e.bin" ``` Time per request (mean) 183 ms on master 30 ms this branch So this can process 6.1 times as many requests, and saturates all the cores instead of keeping them partly idle waiting in the lock. With 8 threads the mean times were 90 ms on master and 19 ms on this branch, a speedup of 4.7x. Big thanks to martinus for finding this and the original PR. The second commit is from a suggestion on the original PR by jonatack to remove the unnecessary `LOCK(cs_main)` in the zmq notifier's `NotifyBlock`. I also found that this approach could be applied to rpcs `getblock` (including `verbosity=3`), `getblockstats`, and `gettxoutproof` with similar very good results. The above benchmarks steps need to be modified slightly for RPC. Run the following ApacheBench command with different request data in a file named `data.json`: ``` ab -p data.json -n 10000 -c 16 -A user:password "http://127.0.0.1:8332/" ``` For `getblock`, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e"]} ``` master - 184 ms mean request time branch - 28 ms mean request time For `getblock` with verbosity level 3, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblock", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", 3]} ``` This verbosity level fetches an undo file from disk, so it benefits from this approach as well. However, a lot of time is spent serializing to JSON so the performance gain is not as severe. master - 818 ms mean request time branch - 505 ms mean request time For `getblockstats`, use the following in `data.json`: ``` {"jsonrpc": "1.0", "id": "curltest", "method": "getblockstats", "params": ["0000000000000000000592a974b1b9f087cb77628bb4a097d5c2c11b3476a58e", ["minfeerate","avgfeerate"]]} ``` This request used a lock on reading both a block and undo file, so the results are very good. master - 244 ms mean request time branch - 28 ms mean request time ACKs for top commit: MarcoFalke: re-ACK d7f61e7d5909da7227c9e34be06ce9eb872ba074 💫 hebasto: ACK d7f61e7d5909da7227c9e34be06ce9eb872ba074, I have reviewed the code and it looks OK. Did not make benchmarking though. Tree-SHA512: 305ac945b4571c5f47646d4f0e78180d7a3d40b2f70ee43e4b3e00c96a465f6d0b9c750b8e85c89ed833e557e2cdb5896743f07ef90e4e53d4ad85452b545886
2022-12-07Merge bitcoin/bitcoin#25934: wallet, rpc: add `label` to `listsinceblock`Andrew Chow
4e362c2b7269ae0426010850c605e5c1d0d58234 doc: add release note for 25934 (brunoerg) fe488b4c4b7aa07fb83d528e2942ef914fd188c0 test: add coverage for `label` in `listsinceblock` (brunoerg) 722e9a418d078ed34aedd1ca55c1ae104f29a7d3 wallet, rpc: add `label` to `listsinceblock` (brunoerg) 852891ff98cffd37a74b9cb96394f43b2e6ca30e refactor, wallet: use optional for `label` in `ListTransactions` (brunoerg) Pull request description: This PR adds `label` parameter to `listsinceblock` to be able to fetch all incoming transactions having the specified label since a specific block. It's possible to use it in `listtransactions`, however, it's only possible to set the number of transactions to return, not a specific block to fetch from. `getreceivedbylabel` only returns the total amount received, not the txs info. `listreceivedbylabel` doesn't list all the informations about the transactions and it's not possible to fetch since a block. ACKs for top commit: achow101: ACK 4e362c2b7269ae0426010850c605e5c1d0d58234 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/25934/commits/4e362c2b7269ae0426010850c605e5c1d0d58234 aureleoules: ACK 4e362c2b7269ae0426010850c605e5c1d0d58234 Tree-SHA512: fbde5db8cebf7a27804154fa61997b5155ad512e978cebb78c17acab9efcb624ea5f39d649899d12e5e675f80d4d0064cae8132b864de0d93a8d1e6fbcb9a737
2022-12-07refactor: Replace isTrue with get_boolMarcoFalke
This makes the code more robust, see previous commit. In general replacing isTrue with get_bool is not equivalent because get_bool can throw exceptions, but in this case, exceptions won't happen because of RPCTypeCheck() and isNull() checks in the preceding code.
2022-12-07bugfix: Strict type checking for RPC boolean parametersMarcoFalke
2022-12-07Merge bitcoin/bitcoin#26298: refactor: Move src/interfaces/*.cpp files to ↵fanquake
libbitcoin_common.a b19c4124b3c9a15fe3baf8792c55eb26eca51c0f refactor: Rename ambiguous interfaces::MakeHandler functions (Ryan Ofsky) dd6e8bd71c6025a51d88000caf28121ec00499db build: remove BOOST_CPPFLAGS from libbitcoin_util (fanquake) 82e272a109281f750909d1feade784c778d8b592 refactor: Move src/interfaces/*.cpp files to libbitcoin_common.a (Ryan Ofsky) Pull request description: These belong in `libbitcoin_common.a`, not `libbitcoin_util.a`, because they aren't general-purpose utilities, they just contain some common glue code that is used by both the node and the wallet. Another reason not to include these in `libbitcoin_util.a` is to prevent them from being used by the kernel library. Also rename ambiguous `MakeHandler` functions to `MakeCleanupHandler` and `MakeSignalHandler`. Cleanup function handler was introduced after boost signals handler, so original naming didn't make much sense. This just contains a move-only commit, and a rename commit. There are no actual code or behavior changes. This PR is an alternative to #26293, and solves the same issue of removing a boost dependency from the _util_ library. The advantages of this PR compared to #26293 are that it keeps the source directory structure more flat, and it avoids having to change #includes all over the codebase. ACKs for top commit: hebasto: ACK b19c4124b3c9a15fe3baf8792c55eb26eca51c0f Tree-SHA512: b3a1d33eedceda7ad852c6d6f35700159d156d96071e59acae2bc325467fef81476f860a8855ea39cf3ea706a1df2a341f34fb2dcb032c31a3b0e9cf14103b6a
2022-12-07Merge bitcoin/bitcoin#26645: util: Include full version id in bug reportsMarcoFalke
fa825bd227b9d2bace896a2d29b5ce78bbd4e59c util: Include full version id in bug reports (MarcoFalke) Pull request description: This will show the unique id of the full source code when the bug occurred, which can help debugging ACKs for top commit: 1440000bytes: utACK https://github.com/bitcoin/bitcoin/commit/fa825bd227b9d2bace896a2d29b5ce78bbd4e59c theStack: ACK fa825bd227b9d2bace896a2d29b5ce78bbd4e59c john-moffett: ACK fa825bd227b9d2bace896a2d29b5ce78bbd4e59c Tree-SHA512: a7a775718f5f9796b5cffafbb3ace8adb5c163414ec584a57143157fc9dfb86f799e3b9c8365fcb831ee1e9eafc59d699d1653d772c68392de421b3de74dcd61
2022-12-06wallet: Rename nFeeRet in CreateTransactionInternal to current_feeAndrew Chow
nFeeRet represents the fee that the transaction currently pays. Update it's name to reflect that.
2022-12-06rpc: reduce LOCK(cs_main) scope in gettxoutproofAndrew Toth
2022-12-06rpc: reduce LOCK(cs_main) scope in GetUndoChecked and getblockstatsAndrew Toth
2022-12-06rpc: reduce LOCK(cs_main) scope in blockToJSON Andrew Toth
2022-12-06rpc: reduce LOCK(cs_main) scope in GetBlockChecked and getblockAndrew Toth
generated by cgit v1.2.3 (git 2.26.2) at 2024-11-30 12:21:44 +0000