aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
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-13Merge bitcoin/bitcoin#26299: ci: Use Multi-ToolTask in "Win64 native" taskMacroFake
98c389ccff2d808524fa6d8b8091465e7b86ea1c ci: Use Multi-ToolTask in "Win64 native" task (Hennadii Stepanov) Pull request description: See https://devblogs.microsoft.com/cppblog/improved-parallelism-in-msbuild/. Build time (`ccache` cache has been _invalidated_ to ensure equal initial conditions): - on the [master](https://cirrus-ci.com/task/4900469905555456) branch: ![image](https://user-images.githubusercontent.com/32963518/195401896-c714cb08-2a41-4eed-afb1-14992f5a9152.png) - this [PR](https://cirrus-ci.com/task/5840011785404416) changes: ![image](https://user-images.githubusercontent.com/32963518/195412308-3dd823bb-0ecd-481f-8ab8-05643fd4f7c0.png) Also consider "CPU Usage" charts provided by CI. ACKs for top commit: aureleoules: ACK 98c389ccff2d808524fa6d8b8091465e7b86ea1c :tada: jarolrod: ACK 98c389ccff2d808524fa6d8b8091465e7b86ea1c Tree-SHA512: 03b7ff48322ab8ab74ca1646df994fb6766ad4dd656f3b89e5ef5bc4756c185354689f44e5e1aa3b193aab895402b4d032b53fa9aa437455d9800e0294d86836
2022-10-13Merge bitcoin/bitcoin#25964: build: fix mingw miniupnpc cflagsfanquake
859644b3c8550bb596fc8823747859e772dd493a build: set D_WIN32_WINNT=0x0601 for mingw miniupnpc (fanquake) 8e2d93ff0fee36baf7fe88662c83f5b4f40f0add build: fix cflags passing for mingw miniupnpc (fanquake) Pull request description: Pulls in a patch I've upstreamed to miniupnpc so that we properly pass our cflags when building it for mingw. See https://github.com/miniupnp/miniupnp/pull/619. Also set `D_WIN32_WINNT` to `0x0601` to match libevent, configure etc. Previously it was being set to `0X501`. Guix Build (x86_64 / arm64): ```bash 39a66c473a45b83ca85500b32ccf8f30d4ae80f965ca064566ee9fd84a51964b guix-build-859644b3c855/output/aarch64-linux-gnu/SHA256SUMS.part 7b0515e422f350cb23f4f0b2f87eaa1b30d1c80389da6f1cbe700794902c88e9 guix-build-859644b3c855/output/aarch64-linux-gnu/bitcoin-859644b3c855-aarch64-linux-gnu-debug.tar.gz 192253fb387a2216b6d63d47a18e34bfa284874488d7ebc6ba656ca76a905519 guix-build-859644b3c855/output/aarch64-linux-gnu/bitcoin-859644b3c855-aarch64-linux-gnu.tar.gz 0a8b5c77928a46e62dff85cf73ca9e932560533b99ef1ec374be00516f9e1183 guix-build-859644b3c855/output/arm-linux-gnueabihf/SHA256SUMS.part d1b01b36d7092d63ab84877e05a973d915d177dbc618fe00eeaff86295032750 guix-build-859644b3c855/output/arm-linux-gnueabihf/bitcoin-859644b3c855-arm-linux-gnueabihf-debug.tar.gz 50d9cd81a4a37fbd5c22ee8f1b8398a836879bda1b514a9ed3d0bcd6fd3de41f guix-build-859644b3c855/output/arm-linux-gnueabihf/bitcoin-859644b3c855-arm-linux-gnueabihf.tar.gz e590d6dc6687c744b4067af330a7fe44110da4972c46f0262c39ce03e2aa6ac5 guix-build-859644b3c855/output/arm64-apple-darwin/SHA256SUMS.part 6b9b97a1f6ead6d2b70d706ba39b11f36c2929962afc0d52404f2341d412d4e7 guix-build-859644b3c855/output/arm64-apple-darwin/bitcoin-859644b3c855-arm64-apple-darwin-unsigned.dmg de6be985a4e1b11c6450c388b54be4fff3dc3a78e528cb628623ee4a8ea249f4 guix-build-859644b3c855/output/arm64-apple-darwin/bitcoin-859644b3c855-arm64-apple-darwin-unsigned.tar.gz 940024658b9387040ceb26535dbd1ed7edb3709106f6e25d5d3720ed90bbbb2e guix-build-859644b3c855/output/arm64-apple-darwin/bitcoin-859644b3c855-arm64-apple-darwin.tar.gz 67c8fcdd31dca595e5c6b72e597a135718dd50ce1f062cb18d181520c13d3013 guix-build-859644b3c855/output/dist-archive/bitcoin-859644b3c855.tar.gz 4b1954953913d1387589873a8e9dc9765f0f300c125270046da95a23c43aa069 guix-build-859644b3c855/output/powerpc64-linux-gnu/SHA256SUMS.part 5b12ab7e3a7fc162912e67e026646ee5d4c92ef804525504f188d9a569af7d67 guix-build-859644b3c855/output/powerpc64-linux-gnu/bitcoin-859644b3c855-powerpc64-linux-gnu-debug.tar.gz fa4debb24dbb4c9c515ad3bfa2e5cb9bd686f1062157bd4480d076ef35d7ea9d guix-build-859644b3c855/output/powerpc64-linux-gnu/bitcoin-859644b3c855-powerpc64-linux-gnu.tar.gz bafad2f56aad4edd25e49e4cc658811cf87f32a2a2e013672659d410d7a8cac8 guix-build-859644b3c855/output/powerpc64le-linux-gnu/SHA256SUMS.part 96318cac800dc7ee86229754a76047dececd62ecd962362e6dbd1d694bc8c17a guix-build-859644b3c855/output/powerpc64le-linux-gnu/bitcoin-859644b3c855-powerpc64le-linux-gnu-debug.tar.gz 461415ee7bf67e2c59bfce3a09736b3b0ecdc7c81751ce2bed19369237450154 guix-build-859644b3c855/output/powerpc64le-linux-gnu/bitcoin-859644b3c855-powerpc64le-linux-gnu.tar.gz 5c9d19f6af5d1fab7de7496bbdcd1cd266abd6c0e2c226b362dab2582c94a33a guix-build-859644b3c855/output/riscv64-linux-gnu/SHA256SUMS.part 04492f5ce121ba09672119cc8861ee30ecdb814b34726c9c5d7971c528209a55 guix-build-859644b3c855/output/riscv64-linux-gnu/bitcoin-859644b3c855-riscv64-linux-gnu-debug.tar.gz 5fb28111b49e73fc53db2805500c275a2e6321c3e180695eb813675629dcd64c guix-build-859644b3c855/output/riscv64-linux-gnu/bitcoin-859644b3c855-riscv64-linux-gnu.tar.gz 3f48c1c2ba77d4fda725225f6c9b5ab7f3aae244c8abb354407cc73d6547d983 guix-build-859644b3c855/output/x86_64-apple-darwin/SHA256SUMS.part 6dcfb5a4af350466fb7f9319e02fd4bcef66e015116c9eda8aff03b3ac53d109 guix-build-859644b3c855/output/x86_64-apple-darwin/bitcoin-859644b3c855-x86_64-apple-darwin-unsigned.dmg 87112cc5f2d02c16614f6d8df41af6f3ea3c765eb6d196d68d2b514b6bd317bc guix-build-859644b3c855/output/x86_64-apple-darwin/bitcoin-859644b3c855-x86_64-apple-darwin-unsigned.tar.gz b245a4d4881a679f2c91e0c7fd5466d6b93313289e609dbce4e1009b7591332e guix-build-859644b3c855/output/x86_64-apple-darwin/bitcoin-859644b3c855-x86_64-apple-darwin.tar.gz a4c3449aa9d8a8e1c8b0532f6e74845c40f90ad4186f480d5bb2750e184efc10 guix-build-859644b3c855/output/x86_64-linux-gnu/SHA256SUMS.part 12a32f25d6ef7f60023dd3053b21e31131350b6afa01aeab25e53d1928b1a0f5 guix-build-859644b3c855/output/x86_64-linux-gnu/bitcoin-859644b3c855-x86_64-linux-gnu-debug.tar.gz 93d603d89c9251e93992e3c0e176b094d981b020ef1d4bbfbff21806e50b962d guix-build-859644b3c855/output/x86_64-linux-gnu/bitcoin-859644b3c855-x86_64-linux-gnu.tar.gz 79df55f4ca8f972665fd55b43550b2ab9ffafa8fc8de1335b324f4af047c6788 guix-build-859644b3c855/output/x86_64-w64-mingw32/SHA256SUMS.part 8d837e96595ab7337736da334d940ffc9d15215141f176804a528fe9e21f490a guix-build-859644b3c855/output/x86_64-w64-mingw32/bitcoin-859644b3c855-win64-debug.zip 6c9541524f1d54eceb3265c6e79d62502fdc0c2e5263719a0ca357988d7ed718 guix-build-859644b3c855/output/x86_64-w64-mingw32/bitcoin-859644b3c855-win64-setup-unsigned.exe 7566ab4ee53092e81c3079db955d85c8d574cbde2be21526d45619076ffcd264 guix-build-859644b3c855/output/x86_64-w64-mingw32/bitcoin-859644b3c855-win64-unsigned.tar.gz 32164cfa7c06ead63305485653f37d74c6ada82d28b79f58e66faf6e72e130bb guix-build-859644b3c855/output/x86_64-w64-mingw32/bitcoin-859644b3c855-win64.zip ``` ACKs for top commit: jarolrod: re-ACK 859644b hebasto: ACK 859644b3c8550bb596fc8823747859e772dd493a, I've verified introduced changes in compiler flags, including the case with `DEBUG=1`. Tree-SHA512: 6e181ced7e474a80aa191663b08dc594179a0593b8e2d1e4b7c8683794fd7de8d37faedb9a36997645ce6a2a6151e1461678b4db95170fc9b1fcadd6e1bddbe5
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#24467: doc: minor improvements in getutxos REST ↵fanquake
endpoint synopsis c456302d4258e3abc4b8afde20fba808632771b2 doc: minor improvements in getutxos REST endpoint synopsis (Sebastian Falbesoner) Pull request description: Describing an optional sub-path as `<checkmempool>` in the synopsis could be misleading as the angle brackets normally indicate that the field has to be replaced a custom value. Clarify that by showing two variants instead, similar to the `block` endpoint with the `notxdetails` option: ``` #### Blocks `GET /rest/block/<BLOCK-HASH>.<bin|hex|json>` `GET /rest/block/notxdetails/<BLOCK-HASH>.<bin|hex|json>` ``` Further improvements: - uppercase `<TXID>` and `<N>`, to match the description of the other endpoints - s/getutxo command/getutxos endpoint/ - describe what the `checkmempool` option does - s/serialisation/serialization/ (the US spelling is more dominant than the UK spelling in the project, and there is indeed no other instance of the string "serialis*" in the source tree, except once in a release note) - link to BIP64 within the text instead of only showing bare URL - mention that BIP64 is only relevant for `bin` and `hex` output formats - show two endpoint formats of the block section as list ACKs for top commit: stickies-v: ACK c456302d4258e3abc4b8afde20fba808632771b2 - also checked that current master (cc12b8947) doesn't have any other lines changes that would require updates as per the outlined improvement points. Tree-SHA512: b025aac0812397f5fbf78c805c13aeb5afa6862a049d13c0b101178799cdaff1ccd3abc368a5c103ea6ebf17cdff76584c54638d0f8d303d81ade2d71443d305
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-12ci: Use Multi-ToolTask in "Win64 native" taskHennadii Stepanov
See https://devblogs.microsoft.com/cppblog/improved-parallelism-in-msbuild/
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-11ci: Use all available CPUs for functional tests in "Win64 native" taskHennadii Stepanov
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#25322: build: Fix `capnp` package build for Androidfanquake
8b8edc25c13a3e613770bf38b21a2556192e6315 build: Specify native binaries explicitly when building `capnp` package (Hennadii Stepanov) a413595c37f51557f9506e0a279cd80fc9a6fb36 build: Fix `capnp` package build for Android (Hennadii Stepanov) Pull request description: On master (e3c08eb620a2f317fc09fdf20969fa26f02afb91): ``` $ make -C depends capnp MULTIPROCESS=1 HOST=aarch64-linux-android ANDROID_SDK=$ANDROID_HOME ANDROID_NDK=$ANDROID_HOME/ndk/23.2.8568313 ANDROID_API_LEVEL=28 ANDROID_TOOLCHAIN_BIN=$ANDROID_HOME/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/bin ... ld: error: unable to find library -lkj ... ``` This PR fixes this error, and also improves configuring according to the docs. ACKs for top commit: ryanofsky: Code review ACK 8b8edc25c13a3e613770bf38b21a2556192e6315. I'd be a little curious to know what causes the error and how `--disable-shared` fixes it, but these changes all look good Tree-SHA512: 1b07b75f2a83932d8dc1f007e42a67d8327bd5fe4566f554dab4599e2a1e04b0144648790a1fd2ab1c295dba728586035aa0ebdbe5cf49df048ec87736895aaf
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-10Merge bitcoin/bitcoin#26183: build: split ARM crc & crypto extension checksfanquake
20adaeaef5fa190c6c996e010683e9728b947ac9 build: split ARM crc & crypto extension checks (fanquake) Pull request description: We currently perform the same check twice, to put the same set of flags in two different variables. Split the checks so we test for the `crc` and `crypto` extensions independently. If we don't want to split, we should just delete the second `AX_CHECK_COMPILE_FLAG` check, and set `ARM_CRC_CXXFLAGS` & `ARM_SHANI_CXXFLAGS` at the same time. Guix Build: ```bash 045392a6a4f538723b7759c67eeafd832735de7294b72b3a7f488d05a13711f7 guix-build-20adaeaef5fa/output/aarch64-linux-gnu/SHA256SUMS.part 054fda86577d757788a1c87508268402535fcbe869240309a2c91997234389cf guix-build-20adaeaef5fa/output/aarch64-linux-gnu/bitcoin-20adaeaef5fa-aarch64-linux-gnu-debug.tar.gz 92dc2513b2b6d87c0869ae18493fd9d0e2690b5b02bfd4310d54f4d394cfccdf guix-build-20adaeaef5fa/output/aarch64-linux-gnu/bitcoin-20adaeaef5fa-aarch64-linux-gnu.tar.gz 2515cfc708cc6ce0e650ca00c49de8dad856b54741ddc0c195845fc6ce2d67db guix-build-20adaeaef5fa/output/arm-linux-gnueabihf/SHA256SUMS.part fa0a956365e62b484f66dcf9763a02858db5c7e99317861819a54a15589ced80 guix-build-20adaeaef5fa/output/arm-linux-gnueabihf/bitcoin-20adaeaef5fa-arm-linux-gnueabihf-debug.tar.gz 1b3ddf2b1bbdc7632696ca78908e69b4fd156ccf7afa8078b5541d2ac10ab931 guix-build-20adaeaef5fa/output/arm-linux-gnueabihf/bitcoin-20adaeaef5fa-arm-linux-gnueabihf.tar.gz f87d8e23df60b208a631f6642f6c2cc0fc8e4e5e9563b36d1de9d371f22a69d9 guix-build-20adaeaef5fa/output/arm64-apple-darwin/SHA256SUMS.part c24ac07bfa935fd40358823d95ef01128a03b80deec6b2cb8bed122994e8adc2 guix-build-20adaeaef5fa/output/arm64-apple-darwin/bitcoin-20adaeaef5fa-arm64-apple-darwin-unsigned.dmg 696660e030accadc27901dfb4e120aa2fefefa8cc2a33ae887e3c98e5d4795f5 guix-build-20adaeaef5fa/output/arm64-apple-darwin/bitcoin-20adaeaef5fa-arm64-apple-darwin-unsigned.tar.gz 30dcd3f543781ac0e07e36336c2901a25a0829e0d1425c25b3c7aba1d0e5420e guix-build-20adaeaef5fa/output/arm64-apple-darwin/bitcoin-20adaeaef5fa-arm64-apple-darwin.tar.gz 4d63db45f28fcb99aa8f3b30cf06afef80dd308a8d2fdf874752accb3f341258 guix-build-20adaeaef5fa/output/dist-archive/bitcoin-20adaeaef5fa.tar.gz eb208b98b3118e9f8240aab91c7ecb2f9b778109bc19d81d0ba73b3e35aa1123 guix-build-20adaeaef5fa/output/powerpc64-linux-gnu/SHA256SUMS.part 8b0de7008b1932ed18d3ab71ca309dc4919096e226e0a7197bd192e1ba96da82 guix-build-20adaeaef5fa/output/powerpc64-linux-gnu/bitcoin-20adaeaef5fa-powerpc64-linux-gnu-debug.tar.gz bcbc269cc4b5883397c516ef3ef6df564f4a81c240d5afcf912a2bf9554ff148 guix-build-20adaeaef5fa/output/powerpc64-linux-gnu/bitcoin-20adaeaef5fa-powerpc64-linux-gnu.tar.gz e5f7fd823056449a495a68d18fe941b472479bc59d9d4d11a041a4e2cc4044ec guix-build-20adaeaef5fa/output/powerpc64le-linux-gnu/SHA256SUMS.part 73ee7e786372b32ab840f0c00ca0479ddd022b3d37219cd929cb49f744c174e3 guix-build-20adaeaef5fa/output/powerpc64le-linux-gnu/bitcoin-20adaeaef5fa-powerpc64le-linux-gnu-debug.tar.gz 08f64c9aae4d9beef88d8fbae8ad0152517de74bedc88540775c4f757c8b6b9a guix-build-20adaeaef5fa/output/powerpc64le-linux-gnu/bitcoin-20adaeaef5fa-powerpc64le-linux-gnu.tar.gz fe3c28fdb1ee9d5b6ca3ba4510d61c052567edb3b93fdde929ed197072c0fd66 guix-build-20adaeaef5fa/output/riscv64-linux-gnu/SHA256SUMS.part 890d6b96edcc431620eede6239dec51368aff917010e03dabeb29d6a672d7a28 guix-build-20adaeaef5fa/output/riscv64-linux-gnu/bitcoin-20adaeaef5fa-riscv64-linux-gnu-debug.tar.gz df1fc0c9af4799cfe170444e21965f2a600aa193fdd0da542fedceeb3081b194 guix-build-20adaeaef5fa/output/riscv64-linux-gnu/bitcoin-20adaeaef5fa-riscv64-linux-gnu.tar.gz f69cae0b2d0eadb336cf314a888b1e0bed241f38954fe58ca9c9c2d00e49b74e guix-build-20adaeaef5fa/output/x86_64-apple-darwin/SHA256SUMS.part acc5fa9725bba738d10bb8b1e7df2d8a7b0e648015e1c046f67451d343f68224 guix-build-20adaeaef5fa/output/x86_64-apple-darwin/bitcoin-20adaeaef5fa-x86_64-apple-darwin-unsigned.dmg 7e4d8cb6d74434ba9084f487187c49cd5a4138c9ae03a6c2236cdffadb236bc8 guix-build-20adaeaef5fa/output/x86_64-apple-darwin/bitcoin-20adaeaef5fa-x86_64-apple-darwin-unsigned.tar.gz 8d93add28b20dfb2a556d3867cfbf218db336d7eefee6ab6f76a1bb4dd4ae20b guix-build-20adaeaef5fa/output/x86_64-apple-darwin/bitcoin-20adaeaef5fa-x86_64-apple-darwin.tar.gz ba0863eda963db706d2880daa8bc526e6332097010fa7227f513a2d715b6cd6c guix-build-20adaeaef5fa/output/x86_64-linux-gnu/SHA256SUMS.part 6915794f3cdc8ad9b305b6baa58f89f7493097b88c0af190d52d93457a17e8d8 guix-build-20adaeaef5fa/output/x86_64-linux-gnu/bitcoin-20adaeaef5fa-x86_64-linux-gnu-debug.tar.gz 467b05298058ec507c3b247c423f3ea7e027ecf62e45d7ae4b81160118bc0d02 guix-build-20adaeaef5fa/output/x86_64-linux-gnu/bitcoin-20adaeaef5fa-x86_64-linux-gnu.tar.gz 51a534803deaabcbba27d82359ef46e4d5b9e7b121ab71e1975c2a0d1c4c6f45 guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/SHA256SUMS.part c9c5496f20bac01ed6439746aff9ca3dd55708718902c898e99f3d5741b167a3 guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64-debug.zip cfaac54be36789927f83172c0af44c50648f63df7cdc9d81774a170e5ab6e3e5 guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64-setup-unsigned.exe 759b79660c291dcc7da88088de3bb666162fed5c9d94bb24f10cef6e781c565f guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64-unsigned.tar.gz f0124333d384ff6962e2131c7b2814bf5c968e77b63ff1b2c7d19cb4c571757c guix-build-20adaeaef5fa/output/x86_64-w64-mingw32/bitcoin-20adaeaef5fa-win64.zip ``` ACKs for top commit: jarolrod: ACK 20adaeaef5fa190c6c996e010683e9728b947ac9 Tree-SHA512: 8b515b95ba4d41ca2ce91448339841dcfb80feb028e9e3bc67a72e72d93669e1257534c11286489a60ae240f6ad6e68f56615818fefd1c09a07a1bee4976fa6e
2022-10-10Merge bitcoin/bitcoin#26277: test: Remove confusing DUMMY_P2WPKH_SCRIPTfanquake
fa8a305ddde15abc0df03a3af0a03b92405d68f6 test: Remove confusing DUMMY_P2WPKH_SCRIPT (MacroFake) Pull request description: It is confusing because, it is *not* a P2WPKH script, and it is nonstandard. See also https://github.com/bitcoin/bitcoin/pull/26265/files#r989827855 Fix all issues by removing it, and also remove the no longer needed `-acceptnonstdtxn` setting from the test. ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/26277/commits/fa8a305ddde15abc0df03a3af0a03b92405d68f6 theStack: Code-review ACK fa8a305ddde15abc0df03a3af0a03b92405d68f6 📜 Tree-SHA512: 64f3e0009b055e4fd4428b20f3e85582e1608e9b06e500b8fbfeb91fc35ce510e69d051e8f48ce35d0320067793e12f4423b214cc1f68c217a5872e0ad97d211
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.
2022-10-10sync: remove DebugLock alias templateVasil Dimov
Use `UniqueLock` directly. Type deduction works just fine from the first argument to the constructor of `UniqueLock`, so there is no need to repeat ```cpp UniqueLock<typename std::remove_reference<typename std::remove_pointer<decltype(cs)>::type>::type> ``` five times in the `LOCK` macros. Just `UniqueLock` suffices.
2022-10-10sync: avoid confusing name overlap (Mutex)Vasil Dimov
Use `MutexType` instead of `Mutex` for the template parameter of `UniqueLock` because there is already a class named `Mutex` and the naming overlap is confusing. `MutexType` is used elsewhere in `sync.h`.
2022-10-10Merge bitcoin/bitcoin#26215: index: Improve ↵fanquake
BaseIndex::BlockUntilSyncedToCurrentChain reliability 8891949bdcb25093d3a6703ae8228c3c3687d3a4 index: Improve BaseIndex::BlockUntilSyncedToCurrentChain reliability (Ryan Ofsky) Pull request description: Since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb from PR https://github.com/bitcoin/bitcoin/pull/21726, index `BlockUntilSyncedToCurrentChain` behavior has been less reliable, and there has also been a race condition in the `coinstatsindex_initial_sync` unit test. It seems better for `BlockUntilSyncedToCurrentChain` to actually wait for the last connected block to be fully processed, than to be able to return before prune locks are set, so this switches the order of `m_best_block_index = block;` and `UpdatePruneLock` statements in `SetBestBlockIndex` to make it more reliable. Also since commit f08c9fb0c6a799e3cb75ca5f763a746471625beb, there has been a race condition in the `coinstatsindex_initial_sync` test. Before that commit, the atomic index best block pointer `m_best_block_index` was updated as the last step of `BaseIndex::BlockConnected`, so `BlockUntilSyncedToCurrentChain` could safely be used in tests to wait for the last `BlockConnected` notification to be finished before stopping and destroying the index. But after that commit, calling `BlockUntilSyncedToCurrentChain` is no longer sufficient, and there is a race between the test shutdown code which destroys the index object and the new code introduced in that commit calling `AllowPrune()` and `GetName()` on the index object. Reproducibility instructions for this are in https://github.com/bitcoin/bitcoin/issues/25365#issuecomment-1259744133 This commit fixes the `coinstatsindex_initial_sync` race condition, even though it will require an additional change to silence TSAN false positives, https://github.com/bitcoin/bitcoin/pull/26188, after it is fixed. So this partially addresses but does not resolve the bug reporting TSAN errors https://github.com/bitcoin/bitcoin/issues/25365. There is no known race condition outside of test code currently, because the bitcoind `Shutdown` function calls `FlushBackgroundCallbacks` not `BlockUntilSyncedToCurrentChain` to safely shut down. Co-authored-by: vasild Co-authored-by: MarcoFalke ACKs for top commit: mzumsande: re-ACK 8891949bdcb25093d3a6703ae8228c3c3687d3a4 Tree-SHA512: 52e29e3772a0c92873c54e5ffb31dd66a909b68a2031b7585713cd1d976811289c98bd9bb41679a8689062f03be4f97bb8368696e789caa4607c2fd8b1fe289b
2022-10-10Merge bitcoin/bitcoin#26258: refactor: Remove unused CDataStream::rdbuf methodfanquake
fabbbe32ee09430d356fd1843f7d5c716b5f46cc Remove unused CDataStream::rdbuf method (MacroFake) Pull request description: It is unused and seems unlikely to be ever used. ACKs for top commit: theStack: Code-review ACK fabbbe32ee09430d356fd1843f7d5c716b5f46cc aureleoules: ACK fabbbe32ee09430d356fd1843f7d5c716b5f46cc Tree-SHA512: 5804642658f96a0fb51482ebf3a062bb0f997c1e0527455afa4aceeeb6c1ad139a98b14a7c8a0909daba733a83bdc24fcadad45060ead4be6eb3dc3e66c129e2
2022-10-09Merge bitcoin/bitcoin#26103: refactor: mempool: use CTxMemPool::Limitsglozow
33b12e5df6aa14344dd084e0c6f2d34158fca383 docs: improve docs where MemPoolLimits is used (stickies-v) 6945853c0bf3b1941bfd18b5ffbd5a81be0e56da test: use NoLimits() in MempoolIndexingTest (stickies-v) 3a86f24a4c1e4e985b1d90eddc135b8dd17049a4 refactor: mempool: use CTxMempool::Limits (stickies-v) b85af25f8770974bae4ef3fae64e75ef6dd2d3c2 refactor: mempool: add MemPoolLimits::NoLimits() (stickies-v) Pull request description: Mempool currently considers 4 limits regarding ancestor and descendant count and size, which get passed around between functions quite a bit. This PR uses `CTxMemPool::Limits` introduced in https://github.com/bitcoin/bitcoin/pull/25290 to simplify those signatures and callsites. The purpose of this PR is to improve readability and maintenance, without behaviour change. As noted in the first commit "refactor: mempool: change MemPoolLimits members to uint", we currently have an underflow issue where a user could pass a negative `-limitancestorsize`, which is eventually cast to an unsigned integer. This behaviour already exists. Because it's orthogonal and to minimize scope, I think this should be fixed in a separate PR. ACKs for top commit: hebasto: ACK 33b12e5df6aa14344dd084e0c6f2d34158fca383, I have reviewed the code and it looks OK, I agree it can be merged. glozow: reACK 33b12e5df6aa14344dd084e0c6f2d34158fca383 Tree-SHA512: 591c6dcee1894f1c3ca28b34a680eeadcf0d40cda92451b4a422c03087b27d682b5e30ba4367abd75a99b5ccb115b7884b0026958d3c7dddab030549db5a4056
2022-10-09Fix comment typos in scriptpubkeyman.cpp, wallet.cpp, wallet.hDimitris Tsapakidis
Fix comment typos: sigature -> signature ponter -> pointer it's key -> its key
2022-10-09Merge bitcoin/bitcoin#26281: docs: fix m_children to be a member of ↵glozow
CTxMemPoolEntry 01bf4af4f2e67420b0180ffd39098758b1fc5e1b docs: fix m_children to be a member of CTxMemPoolEntry (stickies-v) Pull request description: Small documentation fix to reflect that `m_children` [is a member](https://github.com/bitcoin/bitcoin/blob/73b61717a977fc9d23f1bae3f8620641a9dee1f3/src/txmempool.h#L99) of `CTxMemPoolEntry`, not `CTxMemPool` ACKs for top commit: hebasto: ACK 01bf4af4f2e67420b0180ffd39098758b1fc5e1b, wrong wording was introduced in bitcoin/bitcoin#19478. glozow: ACK 01bf4af4f2 Tree-SHA512: b66c43b92fda44682b1f67c43073ca9e133a6dc03cd28253e571e67170531138c20b22ffdb08f312fb2d47a1f869b876611646b54325c8b614d12049befad578
2022-10-07wallet: have prune error take precedence over assumedvalidJames O'Beirne
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}" Co-authored-by: MacroFake <MarcoFalke@gmail.com> Co-authored-by: Russ Yanofsky <russ@yanofsky.org>
2022-10-07docs: fix m_children to be a member of CTxMemPoolEntrystickies-v
2022-10-07rpc: Return coinbase flag in scantxoutsetMacroFake
2022-10-07test: Remove confusing DUMMY_P2WPKH_SCRIPTMacroFake
2022-10-06tests: Test that PSBT_OUT_TAP_TREE is included correctlyAndrew Chow
2022-10-06psbt: Only include m_tap_tree if it has scriptsAndrew Chow
2022-10-06psbt: Change m_tap_tree to store just the tuplesAndrew Chow
Instead of having an entire TaprootBuilder which may or may not be complete, and could potentially have future changes that interact oddly with taproot tree tuples, have m_tap_tree be just the tuples. When needed in other a TaprootBuilder for actual use, the tuples will be added to a a TaprootBuilder that, in the future, can take in whatever other data is needed as well.
2022-10-06tests: Test that PSBT_OUT_TAP_TREE is combined correctlyAndrew Chow
2022-10-06psbt: Fix merging of m_tap_treeAndrew Chow
Merging should be checking that the current PSBTOutput doesn't have a taptree and the other one's is copied over. The original merging had this inverted and would remove m_tap_tree if the other did not have it.
2022-10-06Merge bitcoin/bitcoin#26272: test: Prevent UB in `minisketch_tests.cpp`MacroFake
97007e2b9b7a578315df2b1549cd6075130e8f05 test: Prevent UB in `minisketch_tests.cpp` (Hennadii Stepanov) Pull request description: [`std::optional::operator*`](https://en.cppreference.com/w/cpp/utility/optional/operator*), which follows after the changed line, can cause UB. This PR addresses https://github.com/bitcoin/bitcoin/issues/26262#issuecomment-1268855418 ACKs for top commit: stickies-v: ACK 97007e2b9b7a578315df2b1549cd6075130e8f05 Tree-SHA512: a7dde8dac0cbdfa362fa1158b4564eccff9405852612227d581690c9a34084b3467ae6d4c0269262688d75339dcea90aaa38fccbba9be92d2643c2113860f3d6
2022-10-06test: Prevent UB in `minisketch_tests.cpp`Hennadii Stepanov
2022-10-06Merge bitcoin/bitcoin#24364: refactor: remove duplicate code from BlockAssemblerglozow
0f40d653218789aa176ca2f844e3222d2ad890a3 refactor: remove duplicate code from BlockAssembler (James O'Beirne) Pull request description: Found while reminding myself how transactions are chosen for blocks. Take it or leave it! ACKs for top commit: glozow: ACK 0f40d653218789aa176ca2f844e3222d2ad890a3 theStack: Concept and code-review ACK 0f40d653218789aa176ca2f844e3222d2ad890a3 Tree-SHA512: 8a2694e670ce3fe897ab8f64f64c8df5f8487fc1264527a3abbcba0e5b921fb693416497ccd62508295bc33f202c65556b91b6af463acb91aab43138d2492c14
2022-10-05Validate `port` optionsamadeuszpawlik
Check `port` options for invalid values (ports are parsed as uint16, so in practice values >65535 are invalid; port 0 is undefined and therefore considered invalid too). This allows for an early rejection of faulty values and an supplying an informative message to the user. Splits tests in `feature_proxy.py` to cover both invalid `hostname` and `port` values. Adds a release-note as previously valid `-port` and `-rpcport` values can now result in errors.
2022-10-05Validate port value in `SplitHostPort`amadeuszpawlik
Forward the validation of the port from `ParseUInt16(...)`. Consider port 0 as invalid. Add suitable test for the `SplitHostPort` function. Add doxygen description to the `SplitHostPort` function.
generated by cgit v1.2.3 (git 2.26.2) at 2024-10-13 03:27:02 +0000