aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
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-16db: Change DatabaseCursor::Next to return status enumAndrew Chow
Next()'s result is a tri-state - failed, more to go, complete. Replace the way that this is returned with an enum with values FAIL, MORE, and DONE rather than with two booleans.
2022-12-16wallet: Have cursor users use DatabaseCursor directlyAndrew Chow
Instead of having the DatabaseBatch manage the cursor, having the consumer handle it directly
2022-12-16clang-tidy, qt: Force checks for headers in `src/qt`Hennadii Stepanov
2022-12-16clang-tidy, qt: Fix `modernize-use-default-member-init` in headersHennadii Stepanov
See https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-default-member-init.html
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-15For feebump, ignore abandoned descendant spendsJohn Moffett
To be eligible for fee-bumping, a transaction must not have any of its outputs (eg - change) spent in other unconfirmed transactions in the wallet. However, this check should not apply to abandoned transactions. A new test case is added to cover this case.
2022-12-15bench: add benchmark for wallet 'AvailableCoins' function.furszy
2022-12-15walletdb: refactor: drop unused `FindWalletTx` parameter and renameSebastian Falbesoner
Since commit 3340dbadd38f5624642cf0e14dddbe6f83a3863b ("Remove -zapwallettxes"), the `FindWalletTx` helper is only needed to read tx hashes, so drop the other parameter and rename the method accordingly.
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-14wallet: Introduce DatabaseCursor RAII class for managing cursorAndrew Chow
Instead of having DatabaseBatch deal with opening and closing database cursors, have a separate RAII class that deals with those. For now, DatabaseBatch manages DatabaseCursor, but this will change later.
2022-12-14test: move coins result test to wallet_tests.cppfurszy
The class `AvailableCoinsTestingSetup` inside `availablecoins_tests.cpp` is a plain copy of `ListCoinsTestingSetup` that is inside wallet_tests.cpp.
2022-12-14test: extend and simplify availablecoins_testsfurszy
Clean redundant code and add coverage for 'AvailableCoins' incremental result.
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-13wallet: Skip rescanning if wallet is more recent than tipAndrew Chow
If a wallet has key birthdates that are more recent than the currrent chain tip, or a bestblock height higher than the current tip, we should not attempt to rescan as there is nothing to scan for.
2022-12-13Add secp256k1_selftest callPieter Wuille
2022-12-13Adapt to libsecp256k1 API changesPieter Wuille
* Use SECP256K1_CONTEXT_NONE when creating signing context, as SECP256K1_CONTEXT_SIGN is deprecated and unnecessary. * Use secp256k1_static_context where applicable.
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-13mempool: log/halt when CalculateMemPoolAncestors fails unexpectedlystickies-v
When CalculateMemPoolAncestors fails unexpectedly (e.g. it exceeds ancestor/descendant limits even though we expect no limits to be applied), add an error log entry for increased visibility. For debug builds, the application will even halt completely since this is not supposed to happen.
2022-12-13mempool: add AssumeCalculateMemPoolAncestors helper functionstickies-v
There are quite a few places that assume CalculateMemPoolAncestors will return a value without raising an error. This helper function adds logging (and Assume for debug builds) that ensures robustness but increases visibility in case of unexpected failures
2022-12-13mempool: use util::Result for CalculateMemPoolAncestorsstickies-v
Avoid using setAncestors outparameter, simplify function signatures and avoid creating unused dummy strings.
2022-12-13txmempool: Remove unused clear() member functionMarcoFalke
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-12Squashed 'src/secp256k1/' changes from 44c2452fd3..21ffe4b22aPieter Wuille
21ffe4b22a Merge bitcoin-core/secp256k1#1055: Prepare initial release e025ccdf74 release: prepare for initial release 0.2.0 6d1784a2e2 build: add missing files to EXTRA_DIST 8c949f56da Merge bitcoin-core/secp256k1#1173: Don't use compute credits for now 13bf1b6b32 changelog: make order of change types match keepachangelog.com b1f992a552 doc: improve release process 7e5b22684f Don't use compute credits for now ad39e2dc41 build: change package version to 0.1.0-dev 5c789dcd73 Merge bitcoin-core/secp256k1#1168: Replace deprecated context flags with NONE in benchmarks and tests d6dc0f4ae3 tests: Switch to NONE contexts in module tests 0c8a5caddd tests: Switch to NONE contexts in tests.c 86540e9e1f tests: add test for deprecated flags and rm them from run_context caa0ad631e group: add gej_eq_var 37ba744f5b tests: Switch to NONE contexts in exhaustive and ctime tests 8d7a9a8eda benchmarks: Switch to NONE contexts 90618e9263 doc: move CHANGELOG from doc/ to root directory e3f84777eb Merge bitcoin-core/secp256k1#1126: API cleanup with respect to contexts 4386a2306c examples: Switch to NONE contexts 7289b51d31 docs: Use doxygen style if and only if comment is user-facing e7d0185c90 docs: Get rid of "initialized for signing" terminology 06126364ad docs: Tidy and improve docs about contexts and randomization e02d6862bd selftest: Expose in public API e383fbfa66 selftest: Rename internal function to make name available for API d2c6d48de3 tests: Use new name of static context 53796d2b24 contexts: Rename static context 72fedf8a6c docs: Improve docs for static context 316ac7625a contexts: Deprecate all context flags except SECP256K1_CONTEXT_NONE 477f02c4de Merge bitcoin-core/secp256k1#1165: gitignore: Add *.sage.py files autogenerated by sage [skip ci] 092be61c5e gitignore: Add *.sage.py files autogenerated by sage 1a553ee8be docs: Change signature "validation" to "verification" ee7341fbac docs: Never require a verification context 751c4354d5 Merge bitcoin-core/secp256k1#1152: Update macOS image for CI 2286f80902 Merge bitcoin-core/secp256k1#993: Enable non-experimental modules by default e40fd277b7 Merge bitcoin-core/secp256k1#1156: Followups to int128_struct arithmetic 99bd335599 Make int128 overflow test use secp256k1_[ui]128_mul a8494b02bf Use compute credits for macOS jobs 3afce0af7c Avoid signed overflow in MSVC AMR64 secp256k1_mul128 c0ae48c995 Update macOS image for CI 9b5f589d30 Heuristically decide whether to use int128_struct 63ff064d2f int128: Add test override for testing __(u)mulh on MSVC X64 f2b7e88768 Add int128 randomized tests 6138d73be4 Merge bitcoin-core/secp256k1#1155: Add MSan CI jobs ddf2b2910e Merge bitcoin-core/secp256k1#1000: Synthetic int128 type. 86e3b38a4a Merge bitcoin-core/secp256k1#1149: Remove usage of CHECK from non-test file 00a42b91b3 Add MSan CI job 44916ae915 Merge bitcoin-core/secp256k1#1147: ci: print env to allow reproducing the job outside of CI c2ee9175e9 Merge bitcoin-core/secp256k1#1146: ci: prevent "-v/--version: not found" irrelevant error e13fae487e Merge bitcoin-core/secp256k1#1150: ci: always cat test_env.log a340d9500a ci: add int128_struct tests dceaa1f579 int128: Tidy #includes of int128.h and int128_impl.h 2914bccbc0 Simulated int128 type. 6a965b6b98 Remove usage of CHECK from non-test file 5c9f1a5c37 ci: always cat all logs_snippets 49ae843592 ci: mostly prevent "-v/--version: not found" irrelevant error 4e54c03153 ci: print env to allow reproducing the job outside of CI a43e982bca Merge bitcoin-core/secp256k1#1144: Cleanup `.gitignore` file f5039cb66c Cleanup `.gitignore` file 798727ae1e Revert "Add test logs to gitignore" 41e8704b48 build: Enable some modules by default 694ce8fb2d Merge bitcoin-core/secp256k1#1131: readme: Misc improvements 88b00897e7 readme: Fix line break 78f5296da4 readme: Sell "no runtime dependencies" ef48f088ad readme: Add IRC channel 9f8a13dc8e Merge bitcoin-core/secp256k1#1128: configure: Remove pkgconfig macros again (reintroduced by mismerge) cabe085bb4 configure: Remove pkgconfig macros again (reintroduced by mismerge) 3efeb9da21 Merge bitcoin-core/secp256k1#1121: config: Set preprocessor defaults for ECMULT_* config values 6a873cc4a9 Merge bitcoin-core/secp256k1#1122: tests: Randomize the context with probability 15/16 instead of 1/4 17065f48ae tests: Randomize the context with probability 15/16 instead of 1/4 c27ae45144 config: Remove basic-config.h da6514a04a config: Introduce DEBUG_CONFIG macro for debug output of config 63a3565e97 Merge bitcoin-core/secp256k1#1120: ecmult_gen: Skip RNG when creating blinding if no seed is available d0cf55e13a config: Set preprocessor defaults for ECMULT_* config values 55f8bc99dc ecmult_gen: Improve comments about projective blinding 7a86955800 ecmult_gen: Simplify code (no observable change) 4cc0b1b669 ecmult_gen: Skip RNG when creating blinding if no seed is available af65d30cc8 Merge bitcoin-core/secp256k1#1116: build: Fix #include "..." paths to get rid of further -I arguments 40a3473a9d build: Fix #include "..." paths to get rid of further -I arguments 43756da819 Merge bitcoin-core/secp256k1#1115: Fix sepc256k1 -> secp256k1 typo in group.h 069aba8125 Fix sepc256k1 -> secp256k1 typo in group.h accadc94df Merge bitcoin-core/secp256k1#1114: `_scratch_destroy`: move `VERIFY_CHECK` after invalid scrach space check cd47033335 Merge bitcoin-core/secp256k1#1084: ci: Add MSVC builds 1827c9bf2b scratch_destroy: move VERIFY_CHECK after invalid scrach space check 49e2acd927 configure: Improve rationale for WERROR_CFLAGS 8dc4b03341 ci: Add a C++ job that compiles the public headers without -fpermissive 51f296a46c ci: Run persistent wineserver to speed up wine 3fb3269c22 ci: Add 32-bit MinGW64 build 9efc2e5221 ci: Add MSVC builds 2be6ba0fed configure: Convince autotools to work with MSVC's archiver lib.exe bd81f4140a schnorrsig bench: Suppress a stupid warning in MSVC 09f3d71c51 configure: Add a few CFLAGS for MSVC 3b4f3d0d46 build: Reject C++ compilers in the preprocessor 1cc0941414 configure: Don't abort if the compiler does not define __STDC__ cca8cbbac8 configure: Output message when checking for valgrind 1a6be5745f bench: Make benchmarks compile on MSVC git-subtree-dir: src/secp256k1 git-subtree-split: 21ffe4b22a9683cf24ae0763359e401d1284cc7a
2022-12-12Update secp256k1 subtree to upstream libsecp256k1 version 0.2.0Pieter Wuille
2022-12-12[fuzz] Add HeadersSyncState targetdergoegge
2022-12-12[headerssync] Make m_commit_offset protecteddergoegge
2022-12-12Move SafeDbt out of BerkeleyBatchAndrew Chow
2022-12-12mempool: use util::Result for CalculateAncestorsAndCheckLimitsstickies-v
Avoid using setAncestors outparameter, simplify function signatures and avoid creating unused dummy strings.
2022-12-12[fuzz] Enable erlay in process_message(s) targetsdergoegge
2022-12-12net: remove CService::ToStringPort()Vasil Dimov
It is used only internally in `CService::ToStringAddrPort()`.
2022-12-12gui: simplify OptionsDialog::updateDefaultProxyNets()Vasil Dimov
Do not create strings and compare them to check if one `addr:port` equals another. Use `CService::operator==()` instead. `strDefaultProxyGUI` was assigned the same value 3 times. Instead save it in `const CService ui_proxy` at the beginning of the function.
2022-12-12net: remove CService::ToString() use ToStringAddrPort() insteadVasil Dimov
Both methods do the same thing, so simplify to having just one. `ToString()` is too generic in this case and it is unclear what it does, given that there are similar methods: `ToStringAddr()` (inherited from `CNetAddr`), `ToStringPort()` and `ToStringAddrPort()`.
2022-12-12net: remove CNetAddr::ToString() and use ToStringAddr() insteadVasil Dimov
Both methods do the same thing, so simplify to having just one. Further, `CService` inherits `CNetAddr` and `CService::ToString()` overrides `CNetAddr::ToString()` but the latter is not virtual which may be confusing. Avoid such a confusion by not having non-virtual methods with the same names in inheritance.
2022-12-12scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]()Vasil Dimov
"IP" stands for "Internet Protocol". "IP address" is sometimes shortened to just "IP" or "address". However, Tor or I2P addresses are not "IP addresses", nor "IPs". Thus, use "Addr" instead of "IP" for addresses that could be IP, Tor or I2P addresses: `CService::ToStringIPPort()` -> `CService::ToStringAddrPort()` `CNetAddr::ToStringIP()` -> `CNetAddr::ToStringAddr()` -BEGIN VERIFY SCRIPT- sed -i 's/ToStringIPPort/ToStringAddrPort/g' -- $(git grep -l ToStringIPPort src) sed -i 's/ToStringIP/ToStringAddr/g' -- $(git grep -l ToStringIP src) -END VERIFY SCRIPT-
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-10refactor: modernize the implementation of uint256.*pasta
- Constructors of uint256 to utilize Span instead of requiring a std::vector - converts m_data into a std::array - Prefers using `WIDTH` instead of `sizeof(m_data)` - make all the things constexpr - replace C style functions with c++ equivalents - memset -> std::fill - memcpy -> std::copy Note: In practice, implementations of std::copy avoid multiple assignments and use bulk copy functions such as std::memmove if the value type is TriviallyCopyable and the iterator types satisfy LegacyContiguousIterator. (https://en.cppreference.com/w/cpp/algorithm/copy) - memcmp -> std::memcmp