aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2022-10-19Merge bitcoin/bitcoin#26142: Use `PACKAGE_NAME` in messages rather than ↵MacroFake
hardcoding "Bitcoin Core" b147322a7a387423164de5a7d91a33eacad51689 Use `PACKAGE_NAME` in messages rather than hardcoding "Bitcoin Core" (Hennadii Stepanov) Pull request description: Usually, we do not hardcode "Bitcoin Core" in the user-faced messages. See: - bitcoin/bitcoin#18646 - bitcoin/bitcoin#19282 Also grammar has been improved -- singular instead of plural. ACKs for top commit: jarolrod: ACK b147322a7a387423164de5a7d91a33eacad51689 Tree-SHA512: b135c18703dfdd7b63d4cb27d1ac48f6a9dbf69382142ae381f33bf561cbf57477a11d1c73263aa834f705206d7dd5716df2523d38ed0d4cfec8babc38bb017a
2022-10-19Merge bitcoin/bitcoin#26179: bench: Add missed `ECCVerifyHandle` instanceMacroFake
f09d47b263459e27faf5416287255eb3aed369a2 bench: Add missed `ECCVerifyHandle` instance (Hennadii Stepanov) Pull request description: To clearly observe the lack of an `ECCVerifyHandle` instance, - apply the following diff: ```diff --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -19,11 +19,9 @@ bench_bench_bitcoin_SOURCES = \ bench/bench.h \ bench/bench_bitcoin.cpp \ bench/block_assemble.cpp \ - bench/ccoins_caching.cpp \ bench/chacha20.cpp \ bench/chacha_poly_aead.cpp \ bench/checkblock.cpp \ - bench/checkqueue.cpp \ bench/crypto_hash.cpp \ bench/data.cpp \ bench/data.h \ @@ -46,8 +44,7 @@ bench_bench_bitcoin_SOURCES = \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ bench/strencodings.cpp \ - bench/util_time.cpp \ - bench/verify_script.cpp + bench/util_time.cpp nodist_bench_bench_bitcoin_SOURCES = $(GENERATED_BENCH_FILES) ``` - then ``` $ ./autogen $ ./configure $ make clean $ make ``` - then ``` $ ./src/bench/bench_bitcoin -filter=ExpandDescriptor bench_bitcoin: pubkey.cpp:296: bool CPubKey::IsFullyValid() const: Assertion `secp256k1_context_verify && "secp256k1_context_verify must be initialized to use CPubKey."' failed. Aborted (core dumped) ``` ACKs for top commit: achow101: ACK f09d47b263459e27faf5416287255eb3aed369a2 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26179/commits/f09d47b263459e27faf5416287255eb3aed369a2 Tree-SHA512: e1f33f88d427c57fe31d5810d12e9f46fed2911f5736208ebf7d4a968de0dd8c1f6b73a0d1093316da117dd3bcfda5dde6e41d6c95fcdb99bdea62e19df5ad20
2022-10-18gui: update peers window "Transaction Relay" label and tooltipJon Atack
to current v24.0 p2p behavior
2022-10-18test: Use C++11 member initializers for TestMemPoolEntryHelperMacroFake
Co-authored-by: Aurèle Oulès <aurele@oules.com>
2022-10-18iwyu: Add missing includesMacroFake
2022-10-18Remove g_parallel_script_checksMacroFake
2022-10-18Move ::fCheckBlockIndex into ChainstateManagerMacroFake
This changes the flag for the bitcoin-chainstate executable. Previously it was false, now it is the chain's default value (still false for the main chain).
2022-10-18Move ::fCheckpointsEnabled into ChainstateManagerMacroFake
2022-10-18Move ::nMinimumChainWork into ChainstateManagerMacroFake
This changes the minimum chain work for the bitcoin-chainstate executable. Previously it was uint256{}, now it is the chain's default minimum chain work.
2022-10-18Move ::hashAssumeValid into ChainstateManagerMacroFake
This changes the assumed valid block for the bitcoin-chainstate executable. Previously it was uint256{}, now it is defaultAssumeValid.
2022-10-18Move ::nMaxTipAge into ChainstateManagerMacroFake
2022-10-18test: Remove unused txmempool include from testsMacroFake
2022-10-18Merge bitcoin/bitcoin#26313: doc: consolidate library documentation to ↵MacroFake
libraries.md af781bf4b2998eb17e89b6b24d26a2590e548259 doc: fix typo in doc/libraries.md (fanquake) 9e9ae6101ff505b97a514148e86a5699acba20df doc: remove library commentary from src/Makefile.am (fanquake) Pull request description: Deduplicate the makefile comments, in favour of doc/libraries.md. I think a single, more comprehensive source of truth is preferable. Diagrams are also useful. Came up in https://github.com/bitcoin/bitcoin/pull/26292#issuecomment-1275094478. ACKs for top commit: ryanofsky: Code review ACK af781bf4b2998eb17e89b6b24d26a2590e548259, nice cleanups hebasto: ACK af781bf4b2998eb17e89b6b24d26a2590e548259, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: df61ed1394102221701ae2dfa42886dfabe9d9fd7f601b794e2195f93d8f7c2a1cd1c000a77d0a969b42328e8ebc0387755c57291837b283fdf376dbd98fdda1
2022-10-17Verify PSBT inputs rather than check for fields being emptyGreg Sanders
2022-10-17test: Add unit tests for reconciliation negotiationGleb Naumenko
2022-10-17p2p: clear txreconciliation state for non-wtxid peersGleb Naumenko
We optimistically pre-register a peer for txreconciliations upon sending txreconciliation support announcement. But if, at VERACK, we realize that the peer never sent WTXIDRELAY message, we should unregister the peer from txreconciliations, because txreconciliations rely on wtxids.
2022-10-17p2p: Finish negotiating reconciliation supportGleb Naumenko
Once we received a reconciliation announcement support message from a peer and it doesn't violate our protocol, we store the negotiated parameters which will be used for future reconciliations.
2022-10-17Add helper to see if a peer is registered for reconciliationsGleb Naumenko
2022-10-17p2p: Forget peer's reconciliation state on disconnectGleb Naumenko
2022-10-17p2p: Announce reconciliation supportGleb Naumenko
If we're connecting to the peer which might support transaction reconciliation, we announce we want to reconcile with them. We store the reconciliation salt so that when the peer responds with their salt, we are able to compute the full reconciliation salt. This behavior is enabled with a CLI flag.
2022-10-17log: Add tx reconciliation log categoryGleb Naumenko
2022-10-14doc: remove library commentary from src/Makefile.amfanquake
This duplicates and is less explanatory than doc/libraries.md.
2022-10-13add lock annotation for FeeFilterRounder::round()glozow
Calling WITH_LOCK() on a non-recursive mutex requires not holding it beforehand. Co-authored-by: Niklas Gögge <n.goeggi@gmail.com>
2022-10-13Merge bitcoin/bitcoin#24851: init: ignore BIP-30 verification in ↵Andrew Chow
DisconnectBlock for problematic blocks e899d4ca6f44785b6e5481e200a6a9a8d2448612 init: limit bip30 exceptions to coinbase txs (Chris Geihsler) 511eb7fdeac36da9d6576c878a1c8d390b38f1bd Ignore problematic blocks in DisconnectBlock (Chris Geihsler) Pull request description: Fixes https://github.com/bitcoin/bitcoin/issues/22596 When using checklevel=4, block verification fails because of duplicate coinbase transactions involving blocks 91812 and 91722. There was already a check in place within `ConnectBlock` to ignore the problematic blocks, but `DisconnectBlock` did not contain a similar check to ignore these blocks when called from `VerifyDB`. By ignoring these two blocks in `DisconnectBlock`, the block verification process succeeds at checklevel=4. (Note to reviewers: this is my first contribution to Bitcoin Core, so any feedback is most welcome. Thanks in advance for reviewing!) ## Steps to reproduce: Use the following bitcoin.conf file and start bitcoind. I only used block data through block ~100000 so that the verification process was much faster. ``` assumevalid=0 checkblocks=0 checklevel=4 ``` Without this change, you will see the following error when the blocks are verified: ``` 2022-04-14T02:56:44Z init message: Verifying blocks… 2022-04-14T02:56:44Z Verifying last 101881 blocks at level 4 2022-04-14T02:56:44Z [0%]...[10%]...[20%]...[30%]...[40%]...ERROR: VerifyDB(): *** coin database inconsistencies found (last 10160 blocks, 142571 good transactions before that) 2022-04-14T02:57:01Z : Corrupted block database detected. Please restart with -reindex or -reindex-chainstate to recover. : Corrupted block database detected. Please restart with -reindex or -reindex-chainstate to recover. ``` With this change, you will see this instead: ``` 2022-04-14T02:32:29Z init message: Verifying blocks… 2022-04-14T02:32:29Z Verifying last 101746 blocks at level 4 2022-04-14T02:32:29Z [0%]...[10%]...[20%]...[30%]...[40%]...[50%]...[60%]...[70%]...[80%]...[90%]...[DONE]. 2022-04-14T02:32:48Z No coin database inconsistencies in last 101746 blocks (226126 transactions) ``` ACKs for top commit: laanwj: Code review ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 achow101: ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 jamesob: (Biased) ACK e899d4ca6f44785b6e5481e200a6a9a8d2448612 ([`jamesob/ackr/24851.2.seejee.init_ignore_bip_30_verif`](https://github.com/jamesob/bitcoin/tree/ackr/24851.2.seejee.init_ignore_bip_30_verif)) Tree-SHA512: d2f6d25e9619aee32c1a73fe846b1b587698eaa5a4994fa6424f1038f45654f9fd52b74a69843cc84d90168d74827130ccf8e9201502f5d52281acdb20429291
2022-10-13Merge bitcoin/bitcoin#25412: rest: add `/deploymentinfo` endpointAndrew Chow
a8250e30f16f2919ea5aa122b2880b076bd398a3 doc: add release note about `/rest/deploymentinfo` (brunoerg) 5c960200242d237f2cf74309b8fd29e8162682ed doc: add `/deploymentinfo` in REST-interface (brunoerg) 3e44bee08eb93e086179b92007649d47652aa439 test: add coverage for `/rest/deploymentinfo` (brunoerg) 91497031cbd74a0665b7fc31eb6b73bfb7bd0d40 rest: add `/deploymentinfo` (brunoerg) Pull request description: #23508 added a new RPC named `getdeploymentinfo`, it moved the softfork section from `getblockchaininfo` into this new one. In the REST interface, we have an endpoint named`/rest/chaininfo.json` (which refers to `getblockchaininfo`), so, this PR adds a new REST endpoint named `/deploymentinfo` which refers to `getdeploymentinfo`. You can use it by passing a block hash, e.g: '/rest/deploymentinfo/<BLOCKHASH>.json' or you can use it without passing a block hash to get the 'deploymentinfo' for the last block. ACKs for top commit: jonatack: re-ACK a8250e30f16f2919ea5aa122b2880b076bd398a3 rebase-only since my last review at c65f82bb achow101: ACK a8250e30f16f2919ea5aa122b2880b076bd398a3 stickies-v: re-ACK https://github.com/bitcoin/bitcoin/commit/a8250e30f16f2919ea5aa122b2880b076bd398a3 Tree-SHA512: 0735183b6828d51a72ed0e2be5a09b314ac4693f548982c6e9adaa0ef07a55aa428d3b2d1b1de70b83169811a663a8624b686166e5797f624dcc00178b9796e6
2022-10-13Merge bitcoin/bitcoin#24407: fees: make the class FeeFilterRounder thread-safeAndrew Chow
8173f160e085186c9bcc7f3506205c309ee66af6 style: rename variables to match coding style (Vasil Dimov) 8b4ad203d06c5ded6ecebbd7277b29a442d88bcf fees: make FeeFilterRounder::feeset const (Vasil Dimov) e7a5bf6be79e341e037305a4c2d8a1a510a8d709 fees: make the class FeeFilterRounder thread-safe (Vasil Dimov) Pull request description: Make the class `FeeFilterRounder` thread-safe so that its methods can be called concurrently by different threads on the same object. Currently it has just one method (`round()`). The second commit is optional, but it improves readability, showing that the `feeset` member will never be changed, thus does not need protection from concurrent access. ACKs for top commit: jonatack: re-ACK 8173f160e085186c9bcc7f3506205c309ee66af6 laanwj: Code review ACK 8173f160e085186c9bcc7f3506205c309ee66af6 promag: Code review ACK 8173f160e085186c9bcc7f3506205c309ee66af6 Tree-SHA512: 94b809997c485c0d114fa702d0406b980be8eaaebcfefa56808ed670aa943959c2f16cfd0ef72b4752fe2a409a23af1b4b7f2f236e51212957759569e3bbbefd
2022-10-13Merge bitcoin/bitcoin#25526: wallet: avoid double keypool TopUp() call on ↵Andrew Chow
descriptor wallets bfb9b94ebefdb95ac7656836975b3d5afc428744 wallet: remove duplicate descriptor type check in GetNewDestination (furszy) 76b982a4a5328c1357dbc5361317f682db160876 wallet: remove unused `nAccountingEntryNumber` field (furszy) 599ff5adfc7e1227c6d97d861d0715aee57611dd wallet: avoid double TopUp() calls on descriptor wallets (furszy) Pull request description: Found it while was digging over a `getnewaddress` timeout on the functional test suite. ### Context: We are calling `TopUp()` twice in the following flows for descriptor wallets: A) `CWallet::GetNewDestination`: 1) Calls spk_man->TopUp() 2) Calls spk_man->GetNewDestination() --> which, after the basic script checks, calls TopUp() again. B) `CWallet::GetReservedDestination`: 1) Calls spk_man->TopUp() 2) Calls spk_man->GetReservedDestination() --> which calls to GetNewDestination (which calls to TopUp again). ### Changes: Move `TopUp()` responsibility from the wallet class to each scriptpubkeyman. So each spkm can decide to call it or not after perform the basic checks for the new destination request. Aside from that, remove the unused `nAccountingEntryNumber` wallet field. And a duplicated descriptor type check in `GetNewDestination` ACKs for top commit: aureleoules: re-ACK bfb9b94ebefdb95ac7656836975b3d5afc428744. achow101: ACK bfb9b94ebefdb95ac7656836975b3d5afc428744 theStack: Code-review ACK bfb9b94ebefdb95ac7656836975b3d5afc428744 Tree-SHA512: 3ab73f37729e50d6c6a4434f676855bc1fb404619d63c03e5b06ce61c292c09c59d64cb1aa3bd9277b06f26988956991d62c90f9d835884f41ed500b43a12058
2022-10-13Merge bitcoin/bitcoin#26109: rpc, doc: getpeerinfo updatesAndrew Chow
a3789c700b5a43efd4b366b4241ae840d63f2349 Improve getpeerinfo pingtime, minping, and pingwait help docs (Jon Atack) df660ddb1cce1ee330346fe1728d868f41ad0256 Update getpeerinfo/-netinfo/TxRelay#m_relay_txs relaytxes docs (for v24 backport) (Jon Atack) 1f448542e79452a48f93f53ebbcb3b6df45aeef0 Always return getpeerinfo "minfeefilter" field (for v24 backport) (Jon Atack) 9cd6682545e845277d2207654250d1ed78d0c695 Make getpeerinfo field order consistent with its help (for v24 backport) (Jon Atack) Pull request description: Various updates and fixups, mostly targeting v24. Please refer to the commit messages for details. ACKs for top commit: achow101: ACK a3789c700b5a43efd4b366b4241ae840d63f2349 brunoerg: ACK a3789c700b5a43efd4b366b4241ae840d63f2349 vasild: ACK a3789c700b5a43efd4b366b4241ae840d63f2349 Tree-SHA512: b8586a9b83c1b18786b5ac1fc1dba91573c13225fc2cfc8d078f4220967c95056354f6be13327f33b4fcf3e9d5310fa4e1bdc93102cbd6574f956698993a54bf
2022-10-13Merge bitcoin/bitcoin#23549: Add scanblocks RPC call (attempt 2)Andrew Chow
626b7c8493ea1063f30ae4f62e1b36eb87adf685 fuzz: add scanblocks as safe for fuzzing (James O'Beirne) 94fe5453c7f8a86136dc9a6e0b370afd32564209 test: rpc: add scanblocks functional test (Jonas Schnelli) 6ef2566b68cb8570220c13df11c5cb5a5f4f7a4d rpc: add scanblocks - scan for relevant blocks with descriptors (Jonas Schnelli) a4258f6e81a476ce1687e2d58f7d2bf16162a172 rpc: move-only: consolidate blockchain scan args (James O'Beirne) Pull request description: Revives #20664. All feedback from the previous PR has either been responded to inline or incorporated here. --- Major changes from Jonas' PR: - consolidated arguments for scantxoutset/scanblocks - substantial cleanup of the functional test Here's the range-diff (`git range-diff master jonasschnelli/2020/12/filterblocks_rpc jamesob/2021-11-scanblocks`): https://gist.github.com/jamesob/aa4a975344209f0316444b8de2ec1d18 ### Original PR description > The `scanblocks` RPC call allows one to get relevant blockhashes from a set of descriptors by scanning all blockfilters in a given range. > > **Example:** > > `scanblocks start '["addr(<bitcoin_address>)"]' 661000` (returns relevant blockhashes for `<bitcoin_address>` from blockrange 661000->tip) > > ## Why is this useful? > **Fast wallet rescans**: get the relevant blocks and only rescan those via `rescanblockchain getblockheader(<hash>)[height] getblockheader(<hash>)[height])`. A future PR may add an option to allow to provide an array of blockhashes to `rescanblockchain`. > > **prune wallet rescans**: (_needs additional changes_): together with a call to fetch blocks from the p2p network if they have been pruned, it would allow to rescan wallets back to the genesis block in pruned mode (relevant #15946). > > **SPV mode** (_needs additional changes_): it would be possible to build the blockfilterindex from the p2p network (rather then deriving them from the blocks) and thus allow some sort of hybrid-SPV mode with moderate bandwidth consumption (related #9483) ACKs for top commit: furszy: diff re-ACK 626b7c8 Tree-SHA512: f84e4dcb851b122b39e9700c58fbc31e899cdcf9b587df9505eaf1f45578cc4253e89ce2a45d1ff21bd213e31ddeedbbcad2c80810f46755b30acc17b07e2873
2022-10-13Merge bitcoin/bitcoin#25667: assumeutxo: snapshot initializationAndrew Chow
bf9597606166323158bbf631137b82d41f39334f doc: add note about snapshot chainstate init (James O'Beirne) e4d799528696c5ede38c257afaffd367917e0de8 test: add testcases for snapshot initialization (James O'Beirne) cced4e7336d93a2dc88e4a61c49941887766bd72 test: move-only-ish: factor out LoadVerifyActivateChainstate() (James O'Beirne) 51fc9241c08a00f1f407f1534853a5cddbbc0a23 test: allow on-disk coins and block tree dbs in tests (James O'Beirne) 3c361391b8f5971eb3c7b620aa7ad9b437cc515e test: add reset_chainstate parameter for snapshot unittests (James O'Beirne) 00b357c215ed900145bd770525a341ba0ed9c027 validation: add ResetChainstates() (James O'Beirne) 3a29dfbfb2c16a50d854f6f81428a68aa9180509 move-only: test: make snapshot chainstate setup reusable (James O'Beirne) 8153bd9247dad3982d54488bcdb3960470315290 blockmanager: avoid undefined behavior during FlushBlockFile (James O'Beirne) ad67ff377c2b271cb4683da2fb25fd295557f731 validation: remove snapshot datadirs upon validation failure (James O'Beirne) 34d159033106cc595cfa852695610bfe419c989c add utilities for deleting on-disk leveldb data (James O'Beirne) 252abd1e8bc5cdf4368ad55e827a873240535b28 init: add utxo snapshot detection (James O'Beirne) f9f1735f139b6a1f1c7fea50717ff90dc4ba2bce validation: rename snapshot chainstate dir (James O'Beirne) d14bebf100aaaa25c7558eeed8b5c536da99885f db: add StoragePath to CDBWrapper/CCoinsViewDB (James O'Beirne) Pull request description: This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: https://github.com/bitcoin/bitcoin/pull/15606) --- Half of the replacement for #24232. The original PR grew larger than expected throughout the review process. This change adds the ability to initialize a snapshot-based chainstate during init if one is detected on disk. This is of course unused as of now (aside from in unittests) given that we haven't yet enabled actually loading snapshots. Don't be scared! There are some big move-only commits in here. Accompanying changes include: - moving the snapshot coinsdb directory from being called `chainstate_[base blockhash]` to `chainstate_snapshot`, since we only support one snapshot in use at a time. This simplifies some logic, but it necessitates writing that base blockhash out to a file within the coinsdb dir. See [discussion here](https://github.com/bitcoin/bitcoin/pull/24232#discussion_r832762880). - adding a simple fix in `FlushBlockFile()` that avoids a crash when attemping to flush to disk before `LoadBlockIndexDB()` is called, which happens when calling `MaybeRebalanceCaches()` during multiple chainstate init. - improving the unittest to allow testing with on-disk chainstates - necessary to test a simulated restart and re-initialization. ACKs for top commit: naumenkogs: utACK bf9597606166323158bbf631137b82d41f39334f ariard: Code Review ACK bf9597606 ryanofsky: Code review ACK bf9597606166323158bbf631137b82d41f39334f. Changes since last review: rebasing, switching from CAutoFile to AutoFile, adding comments, switching from BOOST_CHECK to Assert in test util, using chainman.GetMutex() in tests, destroying one ChainstateManager before creating a new one in tests fjahr: utACK bf9597606166323158bbf631137b82d41f39334f aureleoules: ACK bf9597606166323158bbf631137b82d41f39334f Tree-SHA512: 15ae75caf19f8d12a12d2647c52897904d27b265a7af6b4ae7b858592eeadb8f9da6c2394b6baebec90adc28742c053e3eb506119577dae7c1e722ebb3b7bcc0
2022-10-13Merge bitcoin/bitcoin#25858: psbt: Only include PSBT_OUT_TAP_TREE when the ↵glozow
output has a script path 9e386afb67bf8fa71b72f730da1695eeb11828cd tests: Test that PSBT_OUT_TAP_TREE is included correctly (Andrew Chow) 30ff25cf37eec4b09ab40424eb5d6a4a80410955 psbt: Only include m_tap_tree if it has scripts (Andrew Chow) 0577d423adda8e719d7611d03355680c8fbacab8 psbt: Change m_tap_tree to store just the tuples (Andrew Chow) 22c051ca70bae73e0430b05fb9d879591df27699 tests: Test that PSBT_OUT_TAP_TREE is combined correctly (Andrew Chow) 7df6e1bb77a96eac4fbcba424bbe780636b86650 psbt: Fix merging of m_tap_tree (Andrew Chow) 0652dc53b291bd295caff4093ec2854fd4b34645 [BugFix]: Do not allow deserializing PSBT with empty PSBT_OUT_TAP_TREE (Jeremy Rubin) Pull request description: PSBT_OUT_TAP_TREE should not be included for outputs that do not have such a tree. This should be disallowed during parsing, as well as prior to serialization when the field is populated during updating. Also added some test cases. Alternative to #25856 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/25858/commits/9e386afb67bf8fa71b72f730da1695eeb11828cd darosior: ACK 9e386afb67bf8fa71b72f730da1695eeb11828cd Tree-SHA512: ce5c02a69752d176dbd967c1e8d30129b1905c8f186aeeef034576c1de82059271a1ee846bd040f5be4e66bb77ba711dcf14ac1e597c5707d7e7e2293f6cfefb
2022-10-13refactor: Use type-safe time point for CWallet::m_next_resendMacroFake
2022-10-13Merge bitcoin/bitcoin#26205: wallet: #25768 follow upsfanquake
b01682a812f0841170657708ef0e896b904fcd77 refactor: revert m_next_resend to not be std::atomic (stickies-v) 9245f456705b285e2d9afcc01a6155e1b3f92fad wallet: only update m_next_resend when actually resending (stickies-v) 7fbde8af5c06694eecd4ce601109bd826a54bd6f refactor: carve out tx resend timer logic into ShouldResend (stickies-v) 01f3534632d18c772901fb6ce22f6394eae96799 refactor: remove unused locks for ResubmitWalletTransactions (stickies-v) c6e8e11fb030ef406752761530421a9e2f0f5d4f wallet: fix capitalization in docstring (stickies-v) Pull request description: This PR addresses the outstanding comments/issues from #25768: - capitalization [typo](https://github.com/bitcoin/bitcoin/pull/25768#discussion_r958572522) in docstring - remove [unused locks](https://github.com/bitcoin/bitcoin/commit/01f3534632d18c772901fb6ce22f6394eae96799) that we previously needed for `ReacceptWalletTransactions()` - before #25768, only `ResendWalletTransactions()` would reset `m_next_resend` (formerly called `nNextResend`). By unifying it with `ReacceptWalletTransactions()` into `ResubmitWalletTransactions()`, the number of callsites that would reset the `m_next_resend` timer increased - since `m_next_resend` is only used in case of `relay=true` (formerly `ResendWalletTransactions()`), this is unintuitive - it leads to [unexpected behaviour](https://github.com/bitcoin/bitcoin/pull/25768#issuecomment-1252619427) such as transactions potentially never being rebroadcasted. - it makes the ResubmitWalletTransactions()` logic [more complicated than strictly necessary](https://github.com/bitcoin/bitcoin/pull/25768#discussion_r962828563) - since #25768, we relied on an earlier call of `ResubmitWalletTransactions(relay=false, force=true)` to initialize `m_next_resend()`, I think we can more elegantly do that by just providing `m_next_resend` with a default value - just to highlight: this commit introduces behaviour change Note: the `if (!fBroadcastTransactions)` in `CWallet:ShouldResend()` is duplicated on purpose, since it potentially avoids the slightly more expensive `if (!chain().isReadyToBroadcast())` check afterwards. I don't have a strong view on it, so happy to remove that additional check to reduce the diff, too. ACKs for top commit: aureleoules: ACK b01682a812f0841170657708ef0e896b904fcd77 achow101: ACK b01682a812f0841170657708ef0e896b904fcd77 Tree-SHA512: ac5f1d8858f8dd736dd1480f385984d660c1916b62a42562317020e8f9fd6a30bd8f23d973d47e4c9480d744c5ba39fdbefd69568a5eb0589a8422d7e5971c1c
2022-10-13Merge bitcoin/bitcoin#26188: test: silence TSAN false positive in ↵fanquake
coinstatsindex_initial_sync 861cb3fadce88cfaee27088185a48f03fb9dafe7 test: move SyncWithValidationInterfaceQueue() before Stop() in txindex_tests (Vasil Dimov) 6526dc3b78d9ca2b5c67564b04dcacbc75b857e1 test: silence TSAN false positive in coinstatsindex_initial_sync (Vasil Dimov) Pull request description: Silence false positives from TSAN about unsynchronized calls to `BaseIndex::~BaseIndex()` and `BaseIndex::SetBestBlockIndex()`. They are synchronized, but beyond the comprehension of TSAN - by `SyncWithValidationInterfaceQueue()`, called from `BaseIndex::BlockUntilSyncedToCurrentChain()`. Fixes https://github.com/bitcoin/bitcoin/issues/25365 ACKs for top commit: MarcoFalke: review ACK 861cb3fadce88cfaee27088185a48f03fb9dafe7 ryanofsky: Code review ACK 861cb3fadce88cfaee27088185a48f03fb9dafe7. Just comment change since last review. Tree-SHA512: 8c30fdf2fd11d54e9adfa68a67185ab820bd7bd9f7f3ad6456e7e6d219fa9cf6d34b41e98e723eae86cb0c1baef7f3fc57b1b011a13dc3fe3d78334b9b5596de
2022-10-12Merge bitcoin/bitcoin#25421: net: convert standalone IsSelectableSocket() ↵glozow
and SetSocketNonBlocking() to Sock methods b527b549504672704a61f70d2565b9489aaaba91 net: convert standalone SetSocketNonBlocking() to Sock::SetNonBlocking() (Vasil Dimov) 29f66f76826056f53d971ac734b7ed49b39848d3 moveonly: move SetSocketNonBlocking() from netbase to util/sock (Vasil Dimov) b4bac556791b5bb8aa118d4c1fed42c3fe45550c net: convert standalone IsSelectableSocket() to Sock::IsSelectable() (Vasil Dimov) 5db7d2ca0aa51ff25f97bf21ce0cbc9e6b741cbd moveonly: move IsSelectableSocket() from compat.h to sock.{h,cpp} (Vasil Dimov) Pull request description: _This is a piece of #21878, chopped off to ease review._ * convert standalone `IsSelectableSocket()` to `Sock::IsSelectable()` * convert standalone `SetSocketNonBlocking()` to `Sock::SetNonBlocking()` This further encapsulates syscalls inside the `Sock` class and makes the callers mockable. ACKs for top commit: jonatack: ACK b527b549504672704a61f70d2565b9489aaaba91 review/debug build/unit tests at each commit, cross-referenced the changes with `man select` and `man errno`, ran a signet node on the last commit with ip4/ip6//tor/i2p/cjdns and network connections were nominal dergoegge: Code review ACK b527b549504672704a61f70d2565b9489aaaba91 Tree-SHA512: af783ce558c7a89e173f7ab323fb3517103d765c19b5d14de29f64706b4e1fea3653492e8ea73ae972699986aaddf2ae72c7cfaa7dad7614254283083b7d2632
2022-10-12Merge bitcoin/bitcoin#24858: incorrect blk file size calculation during ↵glozow
reindex results in recoverable blk file corruption bcb0cacac28e98a39dc856c574a0872fe17059e9 reindex, log, test: fixes #21379 (mruddy) Pull request description: Fixes #21379. The blocks/blk?????.dat files are mutated and become increasingly malformed, or corrupt, as a result of running the re-indexing process. The mutations occur after the re-indexing process has finished, as new blocks are appended, but are a result of a re-indexing process miscalculation that lingers in the block manager's `m_blockfile_info` `nSize` data until node restart. These additions to the blk files are non-fatal, but also not desirable. That is, this is a form of data corruption that the reading code is lenient enough to process (it skips the extra bytes), but it adds some scary looking log messages as it encounters them. The summary of the problem is that the re-index process double counts the size of the serialization header (magic message start bytes [4 bytes] + length [4 bytes] = 8 bytes) while calculating the blk data file size (both values already account for the serialization header's size, hence why it is over accounted). This bug manifests itself in a few different ways, after re-indexing, when a new block from a peer is processed: 1. If the new block will not fit into the last blk file processed while re-indexing, while remaining under the 128MiB limit, then the blk file is flushed to disk and truncated to a size that is 8 greater than it should be. The truncation adds zero bytes (see `FlatFileSeq::Flush` and `TruncateFile`). 1. If the last blk file processed while re-indexing has logical space for the new block under the 128 MiB limit: 1. If the blk file was not already large enough to hold the new block, then the zeros are, in effect, added by `fseek` when the file is opened for writing. Eight zero bytes are added to the end of the last blk file just before the new block is written. This happens because the write offset is 8 too great due to the miscalculation. The result is 8 zero bytes between the end of the last block and the beginning of the next block's magic + length + block. 1. If the blk file was already large enough to hold the new block, then the current existing file contents remain in the 8 byte gap between the end of the last block and the beginning of the next block's magic + length + block. Commonly, when this occcurs, it is due to the blk file containing blocks that are not connected to the block tree during reindex and are thus left behind by the reindex process and later overwritten when new blocks are added. The orphaned blocks can be valid blocks, but due to the nature of concurrent block download, the parent may not have been retrieved and written by the time the node was previously shutdown. ACKs for top commit: LarryRuane: tested code-review ACK bcb0cacac28e98a39dc856c574a0872fe17059e9 ryanofsky: Code review ACK bcb0cacac28e98a39dc856c574a0872fe17059e9. This is a disturbing bug with an easy fix which seems well-worth merging. mzumsande: ACK bcb0cacac28e98a39dc856c574a0872fe17059e9 (reviewed code and did some testing, I agree that it fixes the bug). w0xlt: tACK https://github.com/bitcoin/bitcoin/pull/24858/commits/bcb0cacac28e98a39dc856c574a0872fe17059e9 Tree-SHA512: acc97927ea712916506772550451136b0f1e5404e92df24cc05e405bb09eb6fe7c3011af3dd34a7723c3db17fda657ae85fa314387e43833791e9169c0febe51
2022-10-12Merge bitcoin/bitcoin#26280: rpc: Return coinbase flag in scantxoutsetfanquake
fa08663344eb4cea335804d149b46ff4ff081b1c rpc: Return coinbase flag in scantxoutset (MacroFake) Pull request description: I guess it can't hurt to return this for someone that wants to know it ACKs for top commit: aureleoules: ACK fa08663344eb4cea335804d149b46ff4ff081b1c shaavan: ACK fa08663344eb4cea335804d149b46ff4ff081b1c Tree-SHA512: 04c554b3ed9877bab93ffcf0c1a4430cd41b30c5f4f3bf462a518fc8b3d68832dd85a29e81bd805eaa16e987856933d7a888a8c126f670bb2844bbd5ca1bf902
2022-10-12Merge bitcoin/bitcoin#22087: Validate port-optionsfanquake
04526787b5f6613d1f1ad78434e1dd24ab88dd76 Validate `port` options (amadeuszpawlik) f8387c42343867779170a0f96ef64e6acff5c481 Validate port value in `SplitHostPort` (amadeuszpawlik) Pull request description: Validate `port`-options, so that invalid values are rejected early in the startup. Ports are `uint16_t`s, which effectively limits a port's value to <=65535. As discussed in https://github.com/bitcoin/bitcoin/pull/24116 and https://github.com/bitcoin/bitcoin/pull/24344, port "0" is considered invalid too. Proposed in https://github.com/bitcoin/bitcoin/issues/21893#issuecomment-835784223 The `SplitHostPort(std::string in, uint16_t& portOut, std::string& hostOut)` now returns a bool that indicates whether the port value was set and within the allowed range. This is an improvement that can be used not only for port validation of options at startup, but also in rpc calls, etc, ACKs for top commit: luke-jr: utACK 04526787b5f6613d1f1ad78434e1dd24ab88dd76 ryanofsky: Code review ACK 04526787b5f6613d1f1ad78434e1dd24ab88dd76. Just suggested changes since last review: reverting some SplitHostPort changes, adding release notes, avoiding 'GetArgs[0]` problem. Tree-SHA512: f1ac80bf98520b287a6413ceadb41bc3a93c491955de9b9319ee1298ac0ab982751905762a287e748997ead6198a8bb7a3bc8817ac9e3d2468e11ab4a0f8496d
2022-10-11test: move SyncWithValidationInterfaceQueue() before Stop() in txindex_testsVasil Dimov
So that the call order is the same as in coinstatsindex_tests.
2022-10-11test: silence TSAN false positive in coinstatsindex_initial_syncVasil Dimov
Fixes https://github.com/bitcoin/bitcoin/issues/25365
2022-10-11Merge bitcoin/bitcoin#25676: sync: simplify and remove unused code from sync.hfanquake
75c3f9f8806259ac7ac02e725d2f2f48e5a1d954 sync: rename AnnotatedMixin::UniqueLock to AnnotatedMixin::unique_lock (Vasil Dimov) 8d9ee8efe80edeb04824b0daee236ed1a3f53a87 sync: remove DebugLock alias template (Vasil Dimov) 4b2e16763fbe70e7cdcf82b438d415e9d96f1674 sync: avoid confusing name overlap (Mutex) (Vasil Dimov) 9d7ae4b66c9ce202d51286daac9be7e599d6a629 sync: remove unused template parameter from ::UniqueLock (Vasil Dimov) 11c190e3f18b43ecb120a5f3e81243fb6fd97261 sync: simplify MaybeCheckNotHeld() definitions by using a template (Vasil Dimov) Pull request description: Summary: * Reduce 4 of the `MaybeCheckNotHeld()` definitions to 2 by using a template. * Remove unused template parameter from `::UniqueLock`. * Use `MutexType` instead of `Mutex` for a template parameter name to avoid overlap/confusion with the `Mutex` class. * Rename `AnnotatedMixin::UniqueLock` to `AnnotatedMixin::unique_lock` to avoid overlap/confusion with the global `UniqueLock` and for consistency with `UniqueLock::reverse_lock`. The first commit `sync: simplify MaybeCheckNotHeld() definitions by using a template` is also part of https://github.com/bitcoin/bitcoin/pull/25390 ACKs for top commit: aureleoules: ACK 75c3f9f8806259ac7ac02e725d2f2f48e5a1d954 - LGTM ryanofsky: Code review ACK 75c3f9f8806259ac7ac02e725d2f2f48e5a1d954. Nice cleanups! Just suggested changes since last review: keeping UniqueLock name and fixing a missed rename in a code comment Tree-SHA512: ec261f6a444bdfe4f06e844b57b3606fdd9b2f842647cae15266d9729970d87585c808d482fbba0b31c33a4aa03527c36e282c92b28d9052711f75a7048c96f1
2022-10-10Merge bitcoin/bitcoin#26254: iwyu: Add zmq source filesMacroFake
13afcc0cd4c2975852924d2d9be5e96096147716 iwyu: Add zmq source files (Hennadii Stepanov) Pull request description: ACKs for top commit: fanquake: ACK 13afcc0cd4c2975852924d2d9be5e96096147716 Tree-SHA512: 7af95e991fc2782aeba2edfef0a2f75f9c361058295586adb062087aa31c47cfcce2425aee9dd5153e18e018cf1f9272c9617c671b7262db55f241526c3fcb15
2022-10-10refactor: Replace m_params with chainman.GetParams()Aurèle Oulès
Fixes a TODO introduced in #24595.
2022-10-10iwyu: Add zmq source filesHennadii Stepanov
2022-10-10Merge bitcoin/bitcoin#26118: log: Use steady clock for bench loggingMacroFake
fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 Use steady clock for bench logging (MacroFake) faed342a2338d6a1a26cf977671a736662debae4 scripted-diff: Rename time symbols (MacroFake) Pull request description: Instead of using `0.001` and similar constants to "convert" an int64_t to milliseconds, use the type-safe `Ticks<>` helper. Also, use steady clock instead of system clock, since the durations are used for benchmarking. ACKs for top commit: fanquake: ACK fabf1cdb206e368a9433abf99a5ea2762a5ed2c0 - validation bench output still looks sane. Tree-SHA512: e6525b5fdad6045ca500c56014897d7428ad288aaf375933d3b5939feddf257f6910d562eb66ebcde9186bef9a604ee8d763a318253838318d59df2a285be7c2
2022-10-10Merge bitcoin/bitcoin#26196: kernel: move RunCommandParseJSON to its own filefanquake
43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b refactor: move run_command from util to common (Cory Fields) 192325a77d593e404e74ef5e204aed8801b4e66f kernel: move RunCommandParseJSON to its own file (Cory Fields) Pull request description: Because libbitcoinkernel does not include this new object, this has the side-effect of eliminating its unnecessary `boost::process` dependency. This leaves libbitcoinkernel with 3 remaining boost dependencies: - `boost::date_time` for `util/time.cpp`, which I'll separate out next. Exactly like this PR. - `boost::signals2` for which I have a POC re-implementation here: https://github.com/theuni/bitcoin/commits/replace-boost-signals - `boost::multi_index` which I'm not sure about yet. ACKs for top commit: ryanofsky: Code review ACK 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b. Could consider squashing the two commits, so the code just moves once instead of twice. fanquake: ACK 43b8777dc3e63f4a1b20a3cb23e44c1b9e32862b Tree-SHA512: f2a46cac34aaadfb8a1442316152ad354f6990021b82c78d80cae9fd43cd026209ffd62132eaa99d5d0f8cf34e996b6737d318a9d9a3f1d2ff8d17d697abf26d
2022-10-10Merge bitcoin/bitcoin#26282: wallet: have prune error take precedence over ↵fanquake
assumedvalid 1c36bafc5f7db268546dcc86c793071a7e9d35e0 wallet: have prune error take precedence over assumedvalid (James O'Beirne) Pull request description: Fixes https://github.com/bitcoin/bitcoin/pull/23997#discussion_r891412739. From Russ Yanofsky: > Agree with all of Marco's points here and think this should be updated > > If havePrune and hasAssumedValidChain are both true, better to show havePrune error message. Assumed-valid error message is vague and not very actionable. Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}" ACKs for top commit: MarcoFalke: ACK 1c36bafc5f7db268546dcc86c793071a7e9d35e0 aureleoules: ACK 1c36bafc5f7db268546dcc86c793071a7e9d35e0 Tree-SHA512: bfb0024bb962525cbbd392ade3c0331a8b0525e7f2f2ab52b2dbb9b6dd6311070d85ecb762a7689db84a30991971865698ab6fec187206e6a92133790c5a91dc
2022-10-10Merge bitcoin/bitcoin#25073: test: Cleanup miner_testsfanquake
faa15527d7e0c7923ff9c0fad7bab4648705ed0f test: Use dedicated mempool in TestBasicMining (MacroFake) fafab384a0a5f6d80195307b7bbeb00515da432b test: Use dedicated mempool in TestPackageSelection (MacroFake) fa4055d79c7ea1d4c3b694e39cafa98a1c7ba8bb test: Use dedicated mempool in TestPrioritisedMining (MacroFake) fa2921828511816d0420c567386e1da0391b3ad7 test: Pass mempool reference to AssemblerForTest (MacroFake) Pull request description: This cleans up the miner tests: * Removes duplicate/redundant and thus confusing chainparams object. * Uses a fresh mempool for each subtest instead of using the "global" one from the testing setup. This makes it easier to follow the tests in smaller scopes. Also it makes sure the mempool is truly cleared by reconstructing it. Finally, this removes calls to `clear`, see https://github.com/bitcoin/bitcoin/pull/19909 ACKs for top commit: glozow: utACK faa15527d7e0c7923ff9c0fad7bab4648705ed0f Tree-SHA512: ced1260f6ab70fba74b0fac7ff4fc7adfddcd2f3bee785249d2a4a9055ac253eff9090edbda7a17e72a71a81b56ff708d5ff64e1f57ebc7b7747d6c88fec51e3
2022-10-10Merge bitcoin/bitcoin#26284: Fix comment typosMacroFake
adb1714426365fb72f73097610ba217ac94ea560 Fix comment typos in scriptpubkeyman.cpp, wallet.cpp, wallet.h (Dimitris Tsapakidis) Pull request description: Fixes a number of comment typos found in the code. Top commit has no ACKs. Tree-SHA512: c2c996b66d33ecf0ee734b76303a0f2444e184d2f3ff6931768712ca51011ad51e54336c33a2ff55133766d20ae6adcbb14ddc754dde58b1fe9167d68f54fec5
2022-10-10sync: rename AnnotatedMixin::UniqueLock to AnnotatedMixin::unique_lockVasil Dimov
This avoids confusion with the global `UniqueLock` and the snake case is consistent with `UniqueLock::reverse_lock.
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-30 21:40:22 +0000