aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2021-07-29Merge bitcoin/bitcoin#21882: build: Fix undefined reference to __mulodi4fanquake
e4c8bb62e4a6873c45f42d0d2a24927cb241a0ea build: Fix undefined reference to __mulodi4 (Hennadii Stepanov) Pull request description: When compiling with clang on 32-bit systems the `__mulodi4` symbol is defined in compiler-rt only. Fixes #21294. See more: - https://bugs.llvm.org/show_bug.cgi?id=16404 - https://bugs.llvm.org/show_bug.cgi?id=28629 ACKs for top commit: MarcoFalke: tested-only ACK e4c8bb62e4a6873c45f42d0d2a24927cb241a0ea luke-jr: utACK e4c8bb62e4a6873c45f42d0d2a24927cb241a0ea fanquake: ACK e4c8bb62e4a6873c45f42d0d2a24927cb241a0ea - it's a bit of an awkward workaround to carry, but at-least it's contained to the fuzzers. Tree-SHA512: 93edb4ed568027702b1b9aba953ad50889b834ef97fde3cb99d1ce70076d9c00aa13f95c86b12d6f59b24fa90108d93742f920e15119901a2848fb337ab859a1
2021-07-28Merge bitcoin/bitcoin#22528: refactor: move GetTransaction to ↵MarcoFalke
node/transaction.cpp f685a13bef0418663015ea6d8f448f075510c0ec doc: GetTransaction()/getrawtransaction follow-ups to #22383 (John Newbery) abc57e1f0882a1a2bb20474648419979af6e383d refactor: move `GetTransaction(...)` to node/transaction.cpp (Sebastian Falbesoner) Pull request description: ~This PR is based on #22383, which should be reviewed first~ (merged by now). In [yesterday's PR review club session to PR 22383](https://bitcoincore.reviews/22383), the idea of moving the function `GetTransaction(...)` from src/validation.cpp to src/node/transaction.cpp came up. With this, the circular dependency "index/txindex -> validation -> index/txindex" is removed (see change in `lint-circular-dependencies.sh`). Thanks to jnewbery for suggesting and to sipa for providing historical background. Relevant IRC log: ``` 17:52 <jnewbery> Was anyone surprised that GetTransaction() is in validation.cpp? It seems to me that node/transaction.cpp would be a more appropriate place for it. 17:53 <raj_> jnewbery, +1 17:53 <stickies-v> agreed! 17:54 <glozow> jnewbery ya 17:54 <jnewbery> seems weird that validation would call into txindex. I wonder if we remove this function, then validation would no longer need to #include txindex 17:54 <sipa> GetTransaction predates node/transaction.cpp, and even the generic index framework itself :) 17:55 <sipa> (before 0.8, validation itself used the txindex) 17:55 <jnewbery> (and GetTransaction() seems like a natural sibling to BroadcastTransaction(), which is already in node/transaction.cpp) 17:55 <jnewbery> sipa: right, this is not meant as a criticism of course. Just wondering if we can organize things a bit more rationally now that we have better separation between things. 17:55 <sipa> jnewbery: sure, just providing background 17:56 <sipa> seems very reasonable to move it elsewhere now ``` The commit should be trivial to review with `--color-moved`. ACKs for top commit: jnewbery: Code review ACK f685a13bef0418663015ea6d8f448f075510c0ec rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/22528/commits/f685a13bef0418663015ea6d8f448f075510c0ec mjdietzx: crACK f685a13bef0418663015ea6d8f448f075510c0ec LarryRuane: Code review, test ACK f685a13bef0418663015ea6d8f448f075510c0ec Tree-SHA512: 0e844a6ecb1be04c638b55bc4478c2949549a4fcae01c984eee078de74d176fb19d508fc09360a62ad130677bfa7daf703b67870800e55942838d7313246248c
2021-07-28Merge bitcoin/bitcoin#21562: [net processing] Various tidying up of ↵MarcoFalke
PeerManagerImpl ctor fde1bf4f6136638e84cdf9806eedaae08e841bbf [net processing] Default initialize m_recent_confirmed_transactions (John Newbery) 37dcd12d539e4a875581fa049aa0f7fafeb932a4 scripted-diff: Rename recentRejects (John Newbery) cd9902ac5054c01228d52616bf85f7196364d4ff [net processing] Default initialize recentRejects (John Newbery) a28bfd1d4cfa523a6abf3832dbfd6183cd546944 [net processing] Default initialize m_stale_tip_check_time (John Newbery) 9190b01d8dcf03b74e9b9e1653688a97ac171b37 [net processing] Add Orphanage empty consistency check (John Newbery) Pull request description: - Use default initialization of PeerManagerImpl members where possible - Remove unique_ptr indirection where it's not needed ACKs for top commit: MarcoFalke: ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf 👞 theStack: re-ACK fde1bf4f6136638e84cdf9806eedaae08e841bbf Tree-SHA512: 7ddedcc972df8e933e1fbe5c88b8ea17df89e1e58fc769518512c5540e49dc8eddb3f47e78d1329a6fc5644d2c1d11c981f681fd633f5218bfa4b3e6a86f3d7b
2021-07-28Merge bitcoin/bitcoin#22047: index, rpc: Coinstatsindex follow-upsW. J. van der Laan
779e638ca9b2b37c247577d225b93ac762b0602f coinstats: Add comments for new coinstatsindex values (Fabian Jahr) 5b3d4e724f377834e24b1f014787cc7aa7fc30fe Index: Improve logging in coinstatsindex (Fabian Jahr) d4356d4e48f59c63894b68691cc21ed4892ee716 rpc: Block until synced if coinstatsindex is used in gettxoutsetinfo (Fabian Jahr) a5f6791139554936d13f367660283899a37ff5c7 rpc: Add missing gettxoutsetinfo help docs (Fabian Jahr) 01386bfd88019397237256cb16f91de346eb66f2 Index: Return early from failed coinstatsindex init (Fabian Jahr) 1e3842385b8c0d15086c7cd8736f8c67e6c0c285 index: Use batch writing in coinstatsindex WriteBlock (Fabian Jahr) fb65dde147f63422c4148b089c2f5be0bf5ba80f scripted-diff: Fix coinstats data member names (Fabian Jahr) 8ea8c927ac05980d6a81252e40b7444e9abb74f9 index: Avoid unnecessary type casts in coinstatsindex (Fabian Jahr) Pull request description: This is a collection of smaller follow-ups to #19521, addressing several post-merge review comments. ACKs for top commit: Sjors: re-utACK 779e638ca9b2b37c247577d225b93ac762b0602f jonatack: re-ACK 779e638ca9b2b37c247577d225b93ac762b0602f diff since last review involves doc changes only; rebased to current master and verified clean debug build/no silent conflicts, unit tests, and feature_coinstatsindex functional test laanwj: Code review ACK 779e638ca9b2b37c247577d225b93ac762b0602f Talkless: re-utACK 779e638ca9b2b37c247577d225b93ac762b0602f after cosmetic changes. Tree-SHA512: cb0d038d230c582d7fe3041c89b1e04d39971fab3739d540c609cf826754c6c513b12ded08ac92180aec7a9d7a70114ece50357bd1a902de4adaae9f30b8d699
2021-07-28Merge bitcoin/bitcoin#22530: log: sort logging categories alphabeticallyMarcoFalke
d596dba9877e7ead3fb5426cbe7e608fbcbfe3eb test: assert logging categories are sorted in rpc and help (Jon Atack) 17bbff3b88132c0c95b29b59100456b85e26df75 log, refactor: use guard clause in LogCategoriesList() (Jon Atack) 7c57297319bc386afaf06528778384fe58576ef9 log: sort LogCategoriesList and LogCategoriesString alphabetically (Jon Atack) f720cfa824f1be863349e7016080f8fb1c3c76c2 test: verify number of categories returned by logging RPC (Jon Atack) Pull request description: Sorting the logging categories seems more user-friendly with the number of categories we now have, allowing CLI users to more quickly find a particular category. before ``` $ bitcoin-cli help logging ... The valid logging categories are: net, tor, mempool, http, bench, zmq, walletdb, rpc, estimatefee, addrman, selectcoins, reindex, cmpctblock, rand, prune, proxy, mempoolrej, libevent, coindb, qt, leveldb, validation, i2p, ipc $ bitcoind -h | grep -A8 "debug=<category>" -debug=<category> ... output all debugging information. <category> can be: net, tor, mempool, http, bench, zmq, walletdb, rpc, estimatefee, addrman, selectcoins, reindex, cmpctblock, rand, prune, proxy, mempoolrej, libevent, coindb, qt, leveldb, validation, i2p, ipc. $ bitcoin-cli logging [] '["addrman"]' { "net": false, "tor": true, "mempool": false, "http": false, "bench": false, "zmq": false, "walletdb": false, "rpc": false, "estimatefee": false, "addrman": false, "selectcoins": false, "reindex": false, "cmpctblock": false, "rand": false, "prune": false, "proxy": true, "mempoolrej": false, "libevent": false, "coindb": false, "qt": false, "leveldb": false, "validation": false, "i2p": true, "ipc": false } ``` after ``` $ bitcoin-cli help logging ... The valid logging categories are: addrman, bench, cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb, libevent, mempool, mempoolrej, net, proxy, prune, qt, rand, reindex, rpc, selectcoins, tor, validation, walletdb, zmq $ bitcoind -h | grep -A8 "debug=<category>" -debug=<category> ... output all debugging information. <category> can be: addrman, bench, cmpctblock, coindb, estimatefee, http, i2p, ipc, leveldb, libevent, mempool, mempoolrej, net, proxy, prune, qt, rand, reindex, rpc, selectcoins, tor, validation, walletdb, zmq. $ bitcoin-cli logging [] '["addrman"]' { "addrman": false, "bench": false, "cmpctblock": false, "coindb": false, "estimatefee": false, "http": false, "i2p": false, "ipc": false, "leveldb": false, "libevent": false, "mempool": false, "mempoolrej": false, "net": false, "proxy": false, "prune": false, "qt": false, "rand": false, "reindex": false, "rpc": false, "selectcoins": false, "tor": false, "validation": false, "walletdb": false, "zmq": false } ``` ACKs for top commit: theStack: re-ACK d596dba9877e7ead3fb5426cbe7e608fbcbfe3eb Tree-SHA512: d546257f562b0a288d1b19a028f1a510aaf21bd21da058e7c84653d305ea8662ecb4647ebefd2b97411f845fe5b0b841d40d3fe6814eefcb8ce82df341dfce22
2021-07-28Merge bitcoin/bitcoin#22557: fuzz: silence a compiler warning about unused ↵fanquake
CBanEntry comparator 787296eb6744b15ab693c053e4030ff68dfc95e0 fuzz: silence a compiler warning about unused CBanEntry comparator (Vasil Dimov) Pull request description: ``` test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function] static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs) ^ 1 warning generated. ``` See https://github.com/bitcoin/bitcoin/pull/22517#issuecomment-886177699 ACKs for top commit: MarcoFalke: cr ACK 787296eb6744b15ab693c053e4030ff68dfc95e0 practicalswift: cr ACK 787296eb6744b15ab693c053e4030ff68dfc95e0 hebasto: ACK 787296eb6744b15ab693c053e4030ff68dfc95e0 Tree-SHA512: 72e483cef249170160879cf4b69b787fb6c539d61dda423f618e2c5f130bee8c42897487751e5b58e7679cdb0153eb80efcb104e8a85095daa60d47e39ce78b8
2021-07-27tracing: Tracepoint for connected blocks0xb10c
Can, for example, be used to benchmark block connections.
2021-07-27tracing: Tracepoints for in- and outbound P2P msgs0xb10c
Can be used to monitor in- and outbound node traffic. Based on ealier work by jb55. Co-authored-by: William Casarin <jb55@jb55.com>
2021-07-27fuzz: silence a compiler warning about unused CBanEntry comparatorVasil Dimov
``` test/fuzz/banman.cpp:35:13: warning: unused function 'operator==' [-Wunused-function] static bool operator==(const CBanEntry& lhs, const CBanEntry& rhs) ^ 1 warning generated. ```
2021-07-27build: Fix undefined reference to __mulodi4Hennadii Stepanov
When compiling with clang on 32-bit systems the __mulodi4 symbol is defined in compiler-rt only.
2021-07-27Merge bitcoin/bitcoin#22495: p2p: refactor: tidy up ↵MarcoFalke
`PeerManagerImpl::Misbehaving(...)` 8858e88c840197cdcabea07dd1380ef2aa4ece02 p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)` (Sebastian Falbesoner) Pull request description: This simple refactoring PR has the goal to improve the readability of the `Misbehaving` method by - introducing constant variables `score_before` and `score_now` (to avoid repeatedly calculating the former) - deduplicating calls to LogPrint(), eliminates else-branch ACKs for top commit: jnewbery: utACK 8858e88c840197cdcabea07dd1380ef2aa4ece02 rajarshimaitra: tACK https://github.com/bitcoin/bitcoin/pull/22495/commits/8858e88c840197cdcabea07dd1380ef2aa4ece02 Tree-SHA512: 1d4dd5ac1d16ee9595edf4fa46e4960915a203641d74e6c33cffaba62ea71328834309a4451256fb45daf759f0cf6f4f199c46815afff6c89c0746e2ad4d4092
2021-07-27Merge bitcoin/bitcoin#22155: wallet test: Add test for subtract fee from ↵MarcoFalke
recipient behavior fe6dc76b7c9c5405f37464a3b19fcf82aaf22861 wallet test: Add test for subtract fee from recipient behavior (Russell Yanofsky) 2565478c813fb7278153b113de4b9338fc186872 wallet test refactor: add CreateSyncedWallet function (Russell Yanofsky) Pull request description: This adds test coverage for wallet subtract from recipient behavior without changing it. Behavior seems to have changed recently in a minor way in #17331 without being noticed. ACKs for top commit: achow101: ACK fe6dc76b7c9c5405f37464a3b19fcf82aaf22861 glozow: ACK fe6dc76b7c9c5405f37464a3b19fcf82aaf22861 promag: Code review ACK fe6dc76b7c9c5405f37464a3b19fcf82aaf22861. Tree-SHA512: e00c5dfe467e4ccef5edb0dd4fff6c53f35a37828a4327bea2e166751e5ef971d519ffca7b8f735b12912bb4a547980626356bc1855981005aed1a6c2a57be0b
2021-07-26p2p: refactor: tidy up `PeerManagerImpl::Misbehaving(...)`Sebastian Falbesoner
- introduce constant variables `score_before` and `score_after` in order to improve readability - deduplicate calls to LogPrint(), eliminates else-branch
2021-07-25coinstats: Add comments for new coinstatsindex valuesFabian Jahr
2021-07-25Index: Improve logging in coinstatsindexFabian Jahr
More accurate logging of a warning should make clear if the recovery condition was hit while catching the results of the previous block.
2021-07-25rpc: Block until synced if coinstatsindex is used in gettxoutsetinfoFabian Jahr
During initial sync after startup the gettxoutsetinfo RPC will still return an error while catching up. However, after the initial sync the index will not error immediately anymore when it's in the process of syncing to the tip while being called. Instead it will block until synced and then return the response.
2021-07-25rpc: Add missing gettxoutsetinfo help docsFabian Jahr
2021-07-25Merge bitcoin/bitcoin#22454: fuzz: Limit max ops in tx_pool fuzz targetsMarcoFalke
fa33ed4b3fe422d6a6949cec04d2e14efc9ba3ca fuzz: Limit max ops in tx_pool fuzz targets (MarcoFalke) Pull request description: Without a size limit on the input data, the runtime is unbounded. Fix this by picking an upper bound on the maximum number of fuzz operations. Reproducer from OSS-Fuzz (without bug report): [clusterfuzz-testcase-tx_pool_standard-5963992253202432.log](https://github.com/bitcoin/bitcoin/files/6822465/clusterfuzz-testcase-tx_pool_standard-5963992253202432.log) ACKs for top commit: practicalswift: cr ACK fa33ed4b3fe422d6a6949cec04d2e14efc9ba3ca Tree-SHA512: 32098d573880afba12d510ac83519dc886a6c65d5207edb810f92c7c61edf5e2fc9c57e7b7a1ae656c02ce14e3595707dd6b93caf7956beb2bc817609e14d23d
2021-07-25Merge bitcoin/bitcoin#22453: fuzz: Limit max ops in rolling_bloom_filter ↵MarcoFalke
fuzz target faa86b71acefc8f2e366746a1c251888e6e686dd fuzz: Use ConsumeUInt256 helper to simplify rolling_bloom_filter fuzz test (MarcoFalke) aaaa61fd306e25379e6222e31bf160a6eb04f74e fuzz: Speed up rolling_bloom_filter fuzz test (MarcoFalke) Pull request description: Without a size limit on the input data, the runtime is unbounded. Fix this by picking an upper bound on the maximum number of fuzz operations. Reproducer from OSS-Fuzz (without bug report): [clusterfuzz-testcase-rolling_bloom_filter-5980807721254912.log](https://github.com/bitcoin/bitcoin/files/6822159/clusterfuzz-testcase-rolling_bloom_filter-5980807721254912.log) ACKs for top commit: practicalswift: cr ACK faa86b71acefc8f2e366746a1c251888e6e686dd theStack: Concept and code review ACK faa86b71acefc8f2e366746a1c251888e6e686dd Tree-SHA512: eace588509dfddb2ba97baf86379fa713fa6eb758184abff676cb95807ff8ff36905eeaddeba05665b8464c35c57e2138f88caec71cbfb255e546bbe76558da0
2021-07-25Merge bitcoin/bitcoin#22444: fuzz: Limit max ops in prevector fuzz targetMarcoFalke
faafda232e1d4f79ee64dbfee699a8018f25b0bc fuzz: Speed up prevector fuzz target (MarcoFalke) Pull request description: Without a size limit on the input data, the runtime is unbounded. Fix this by picking an upper bound on the maximum number of fuzz operations. Should fix https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=35981 ACKs for top commit: practicalswift: cr ACK faafda232e1d4f79ee64dbfee699a8018f25b0bc Tree-SHA512: 1bf166c4a99a8ce88bdc030cd6a32ce1da5251b73873772e0e9c001ec2bacafebb183f7c8c88806d0ab633aada2cff8b78791f5c9c0c6f2cc8ef5f0875c4b2ef
2021-07-25Merge bitcoin/bitcoin#22517: fuzz: Temporarily disable failing assert in ↵MarcoFalke
banman fuzz test fa8bed6a47c88f769ae05b04b93eeaf2e1011478 fuzz: Temporarily disable failing assert in banman fuzz test (MarcoFalke) Pull request description: Otherwise the remainder of the fuzz test can't be fuzzed without running into crashes ACKs for top commit: practicalswift: cr ACK fa8bed6a47c88f769ae05b04b93eeaf2e1011478 Tree-SHA512: ec6606292e2cfd26484c7f6caf1c418c377da54111b332990fce68373f0438defda71d931a42ca34431527fbc172dd2fdf29b260afca15b34910ee137de1c365
2021-07-24Merge bitcoin/bitcoin#13533: [tests] Reduced number of validations in ↵MarcoFalke
tx_validationcache_tests c3e111a7daf5800026dda4455c737de0412528f1 Reduced number of validations in `tx_validationcache_tests` to keep the run time reasonable. (lucash-dev) Pull request description: Following a suggestion in the comments, changed `ValidateCheckInputsForAllFlags` from testing all possible flag combinations to testing a random subset. Also created a new enum constant for the highest flag, so that this test doesn’t keep testing an incomplete subset in case a new flag is added. Timing for `checkinputs_test`: ``` Before: 6.8s After: 3.7s ---------------- Saved: 3.1s (45%) ``` This PR was split from #13050. Also see #10026. ACKs for top commit: leonardojobim: tACK https://github.com/bitcoin/bitcoin/pull/13533/commits/c3e111a7daf5800026dda4455c737de0412528f1. kallewoof: ACK c3e111a7daf5800026dda4455c737de0412528f1 theStack: re-ACK c3e111a7daf5800026dda4455c737de0412528f1 Tree-SHA512: bef49645bdd4f61ec73cc77a9f028b95d9856db9446d2e7fc9a48867a6f0e94c2c9f150cb771a30fe852db0efb0a1bd15d38b00d712651793ccb59ff6157a7b4
2021-07-22log, refactor: use guard clause in LogCategoriesList()Jon Atack
and minor formatting fixups
2021-07-22log: sort LogCategoriesList and LogCategoriesString alphabeticallyJon Atack
2021-07-22doc: GetTransaction()/getrawtransaction follow-ups to #22383John Newbery
2021-07-22Merge bitcoin/bitcoin#22383: rpc: Prefer to use txindex if available for ↵MarcoFalke
GetTransaction 78f4c8b98eada337346ffb206339c3ebae4ff43b prefer to use txindex if available for GetTransaction (Jameson Lopp) Pull request description: Fixes #22382 Motivation: prevent excessive disk reads if txindex is enabled. Worth noting that this could be argued to be less of a bug and more of an issue of undefined behavior. If a user calls GetTransaction with the wrong block hash, what should happen? ACKs for top commit: jonatack: ACK 78f4c8b98eada337346ffb206339c3ebae4ff43b theStack: Code review ACK 78f4c8b98eada337346ffb206339c3ebae4ff43b LarryRuane: tACK 78f4c8b98eada337346ffb206339c3ebae4ff43b luke-jr: utACK 78f4c8b98eada337346ffb206339c3ebae4ff43b jnewbery: utACK 78f4c8b98eada337346ffb206339c3ebae4ff43b rajarshimaitra: Code review ACK https://github.com/bitcoin/bitcoin/commit/78f4c8b98eada337346ffb206339c3ebae4ff43b lsilva01: Code Review ACK and Tested ACK https://github.com/bitcoin/bitcoin/pull/22383/commits/78f4c8b98eada337346ffb206339c3ebae4ff43b on Ubuntu 20.04 Tree-SHA512: af7db5b98cb2ae4897b28476b2fa243bf7e6f850750d9347062fe8013c5720986d1a3c808f80098e5289bd84b085de03c81a44e584dc28982f721c223651bfe0
2021-07-22Merge bitcoin/bitcoin#21090: Default to NODE_WITNESS in nLocalServicesW. J. van der Laan
a806647d260132a00cd633160040625c7dd17803 [validation] Always include merkle root in coinbase commitment (Dhruv Mehta) 189128c220190a588500b8e74ee7ae47671b9558 [validation] Set witness script flag with p2sh for blocks (Dhruv Mehta) ac82b99db77ec843af82dcdf040dfdbc98c8ff26 [p2p] remove redundant NODE_WITNESS checks (Dhruv Mehta) 6f8b198b8256a6703a6f5e592dfa77fa024a7035 [p2p] remove unused segwitheight=-1 option (Dhruv Mehta) eba5b1cd6460c98e75d0422bd394e12af7f11e4c [test] remove or move tests using `-segwitheight=-1` (Dhruv Mehta) Pull request description: Builds on #21009 and makes progress on remaining items in #17862 Removing `RewindBlockIndex()` in #21009 allows the following: - removal of tests using `segwitheight=-1` in `p2p_segwit.py`. - move `test_upgrade_after_activation()` out of `p2p_segwit.py` reducing runtime - in turn, that allows us to drop support for `-segwitheight=-1`, which is only supported for that test. - that allows us to always set `NODE_WITNESS` in our local services. The only reason we don't do that is to support `-segwitheight=-1`. - that in turn allows us to drop all of the `GetLocalServices() & NODE_WITNESS` checks inside `net_processing.cpp`, since our local services would always include `NODE_WITNESS` ACKs for top commit: mzumsande: Code-Review ACK a806647d260132a00cd633160040625c7dd17803 laanwj: Code review ACK a806647d260132a00cd633160040625c7dd17803, nice cleanup jnewbery: utACK a806647d260132a00cd633160040625c7dd17803 theStack: ACK a806647d260132a00cd633160040625c7dd17803 Tree-SHA512: 73e1a69d1d7eca1f5c38558ec6672decd0b60b16c2ef6134df6f6af71bb159e6eea160f9bb5ab0eb6723c6632d29509811e29469d0d87abbe9b69a2890fbc73e
2021-07-22Merge bitcoin/bitcoin#22493: fuzz: Extend addrman fuzz test with deserializeMarcoFalke
aaaa9c6019790a1a21a7b4ef01693ac9390ae6d0 fuzz: Extend addrman fuzz test with deserialize (MarcoFalke) Pull request description: Requested on IRC: ``` [18:01] <vasild> => I think there is a good chance fuzzing addrman unserialize will find more bugs [18:04] <sipa> definitely ACKs for top commit: jonatack: ACK aaaa9c6019790a1a21a7b4ef01693ac9390ae6d0 per `git diff fa74025 aaaa9c6` vasild: ACK aaaa9c6019790a1a21a7b4ef01693ac9390ae6d0 Tree-SHA512: f57d0aecf22a933e48d3181d7398218949588dd0de31218d1d28c825649e55fd60b0de6fbc92d2497cf5639a4adc2061c9bf8216546a2be916feac4f03f16e8f
2021-07-22refactor: move `GetTransaction(...)` to node/transaction.cppSebastian Falbesoner
can be reviewed with --color-moved
2021-07-22Merge bitcoin/bitcoin#22481: mempool: apply rule of 5 to epochguard.h, fix ↵W. J. van der Laan
compiler warnings 7b3a20b2602f902c344a615f23f8f0280b6f6bcc mempool: apply rule of 5 to epochguard.h, fix compiler warnings (Jon Atack) Pull request description: Apply the rule of five to `src/util/epochguard.h::{Epoch, Marker}` for safety, which also nicely fixes the `-Wdeprecated-copy` compiler warnings with Clang 13. References: - https://en.cppreference.com/w/cpp/language/rule_of_three - https://www.stroustrup.com/C++11FAQ.html#default - https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-five <details><summary>Compiler warnings fixed</summary><p> ```bash In file included from policy/rbf.cpp:5: In file included from ./policy/rbf.h:8: In file included from ./txmempool.h:24: ./util/epochguard.h:53:17: warning: definition of implicit copy constructor for 'Marker' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy] Marker& operator=(const Marker&) = delete; ^ ./txmempool.h:81:7: note: in implicit copy constructor for 'Epoch::Marker' first required here class CTxMemPoolEntry ^ policy/rbf.cpp:29:29: note: in implicit copy constructor for 'CTxMemPoolEntry' first required here CTxMemPoolEntry entry = *pool.mapTx.find(tx.GetHash()); ``` ```bash In file included from txmempool.cpp:6: In file included from ./txmempool.h:24: ./util/epochguard.h:53:17: warning: definition of implicit copy constructor for 'Marker' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy] Marker& operator=(const Marker&) = delete; ^ ./txmempool.h:81:7: note: in implicit copy constructor for 'Epoch::Marker' first required here class CTxMemPoolEntry ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:150:23: note: in implicit copy constructor for 'CTxMemPoolEntry' first required here { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:512:8: note: in instantiation of function template specialization '__gnu_cxx::new_allocator<boost::multi_index::detail::hashed_index_node<boost::multi_index::detail::hashed_index_node<boost::multi_index::detail::ordered_index_node<boost::multi_index::detail::null_augment_policy, boost::multi_index::detail::ordered_index_node<boost::multi_index::detail::null_augment_policy, boost::multi_index::detail::ordered_index_node<boost::multi_index::detail::null_augment_policy, boost::multi_index::detail::index_node_base<CTxMemPoolEntry, std::allocator<CTxMemPoolEntry>>>>>>>>::construct<CTxMemPoolEntry, const CTxMemPoolEntry &>' requested here __a.construct(__p, std::forward<_Args>(__args)...); ^ /usr/include/boost/multi_index_container.hpp:655:24: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<boost::multi_index::detail::hashed_index_node<boost::multi_index::detail::hashed_index_node<boost::multi_index::detail::ordered_index_node<boost::multi_index::detail::null_augment_policy, boost::multi_index::detail::ordered_index_node<boost::multi_index::detail::null_augment_policy, boost::multi_index::detail::ordered_index_node<boost::multi_index::detail::null_augment_policy, boost::multi_index::detail::index_node_base<CTxMemPoolEntry, std::allocator<CTxMemPoolEntry>>>>>>>>>::construct<CTxMemPoolEntry, const CTxMemPoolEntry &>' requested here node_alloc_traits::construct( ^ /usr/include/boost/multi_index/detail/index_base.hpp:108:15: note: in instantiation of member function 'boost::multi_index::multi_index_container<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>>>::construct_value' requested here final().construct_value(x,v); ^ /usr/include/boost/multi_index/detail/ord_index_impl.hpp:770:33: note: (skipping 5 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all) final_node_type* res=super::insert_(v,x,variant); ^ /usr/include/boost/multi_index_container.hpp:693:33: note: in instantiation of function template specialization 'boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>>, std::allocator<CTxMemPoolEntry>>, boost::mpl::vector0<>, boost::multi_index::detail::hashed_unique_tag>::insert_<boost::multi_index::detail::lvalue_tag>' requested here final_node_type* res=super::insert_(v,x,variant); ^ /usr/include/boost/multi_index_container.hpp:705:12: note: in instantiation of function template specialization 'boost::multi_index::multi_index_container<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>>>::insert_<boost::multi_index::detail::lvalue_tag>' requested here return insert_(v,detail::lvalue_tag()); ^ /usr/include/boost/multi_index/detail/index_base.hpp:213:21: note: in instantiation of member function 'boost::multi_index::multi_index_container<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>>>::insert_' requested here {return final().insert_(x);} ^ /usr/include/boost/multi_index/hashed_index.hpp:284:46: note: in instantiation of member function 'boost::multi_index::detail::index_base<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>>, std::allocator<CTxMemPoolEntry>>::final_insert_' requested here std::pair<final_node_type*,bool> p=this->final_insert_(x); ^ txmempool.cpp:363:53: note: in instantiation of member function 'boost::multi_index::detail::hashed_index<mempoolentry_txid, SaltedTxidHasher, std::equal_to<uint256>, boost::multi_index::detail::nth_layer<1, CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<descendant_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByDescendantScore>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<entry_time>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByEntryTime>, boost::multi_index::ordered_non_unique<boost::multi_index::tag<ancestor_score>, boost::multi_index::identity<CTxMemPoolEntry>, CompareTxMemPoolEntryByAncestorFee>>, std::allocator<CTxMemPoolEntry>>, boost::mpl::vector0<>, boost::multi_index::detail::hashed_unique_tag>::insert' requested here indexed_transaction_set::iterator newit = mapTx.insert(entry).first; ``` ```bash In file included from test/fuzz/policy_estimator.cpp:9: In file included from ./test/fuzz/util.h:27: In file included from ./txmempool.h:24: ./util/epochguard.h:53:17: warning: definition of implicit copy constructor for 'Marker' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy] Marker& operator=(const Marker&) = delete; ^ ./txmempool.h:81:7: note: in implicit copy constructor for 'Epoch::Marker' first required here class CTxMemPoolEntry ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/ext/new_allocator.h:150:23: note: in implicit move constructor for 'CTxMemPoolEntry' first required here { ::new((void *)__p) _Up(std::forward<_Args>(__args)...); } ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/alloc_traits.h:512:8: note: in instantiation of function template specialization '__gnu_cxx::new_allocator<CTxMemPoolEntry>::construct<CTxMemPoolEntry, CTxMemPoolEntry>' requested here __a.construct(__p, std::forward<_Args>(__args)...); ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/vector.tcc:115:21: note: in instantiation of function template specialization 'std::allocator_traits<std::allocator<CTxMemPoolEntry>>::construct<CTxMemPoolEntry, CTxMemPoolEntry>' requested here _Alloc_traits::construct(this->_M_impl, this->_M_impl._M_finish, ^ /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_vector.h:1204:9: note: in instantiation of function template specialization 'std::vector<CTxMemPoolEntry>::emplace_back<CTxMemPoolEntry>' requested here { emplace_back(std::move(__x)); } ^ test/fuzz/policy_estimator.cpp:49:37: note: in instantiation of member function 'std::vector<CTxMemPoolEntry>::push_back' requested here mempool_entries.push_back(ConsumeTxMemPoolEntry(fuzzed_data_provider, tx)); ``` </p></details> ACKs for top commit: laanwj: Code review ACK 7b3a20b2602f902c344a615f23f8f0280b6f6bcc vasild: ACK 7b3a20b2602f902c344a615f23f8f0280b6f6bcc Tree-SHA512: 0406dfcec180152d4f9ed07cbc2f406ad739b41f9c9cb38f8c75159c15d9d8a9a5c7526765966e40d695d265c178f6a80152e7edf82da344a65e55938dddb63d
2021-07-22Merge bitcoin-core/gui#381: refactor: Make BitcoinCore class reusableW. J. van der Laan
8169fc4e73a87331e02502fc24e293831765c8b1 qt, refactor: Fix code styling of moved InitExecutor class (Hennadii Stepanov) c82165a55701fe4ff604d7f30163051cd47c2363 qt, refactor: Move InitExecutor class into its own module (Hennadii Stepanov) dbcf56b6c6e939923673b3f07bed7bb3632dbeb1 scripted-diff: Rename BitcoinCore class to InitExecutor (Hennadii Stepanov) 19a1d008310f250b69b7aa764a9f26384d5a4a85 qt: Add BitcoinCore::m_thread member (Hennadii Stepanov) Pull request description: This PR makes the `BitcoinCore` class reusable, i.e., it can be used by the widget-based GUI or by the [QML-based](https://github.com/bitcoin-core/gui-qml/tree/main/src/qml) one, and it makes the divergence between these two repos minimal. The small benefit to the current branch is more structured code. Actually, this PR is ported from https://github.com/bitcoin-core/gui-qml/pull/10. The example of the re-using of the `BitcoinCore` class is https://github.com/bitcoin-core/gui-qml/pull/11. ACKs for top commit: laanwj: ACK 8169fc4e73a87331e02502fc24e293831765c8b1 ryanofsky: Code review ACK 8169fc4e73a87331e02502fc24e293831765c8b1. Only change is switching from `m_executor` from pointer to optional type (thanks for update!) Tree-SHA512: a0552c32d26d9acf42921eb12bcdf68f02d52f7183c688c43257b1a58679f64e45f193ee2d316850c7f0f516561e17abe989fe545bfa05e158ad3f4c66d19bca
2021-07-22Merge bitcoin/bitcoin#22218: multiprocess: Add ipc::Context and ↵MarcoFalke
ipc::capnp::Context structs 3e33d170cc0a8f386791777f3cc597e2bd0bf2ee Add ipc::Context and ipc::capnp::Context structs (Russell Yanofsky) Pull request description: These are currently empty structs but they will be used to pass some function and object pointers from bitcoin application code to IPC hooks that run, for example, when a remote object is created or destroyed, or a new process is created. --- This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/projects/10). The commit was first part of larger PR #10102. ACKs for top commit: ariard: Code Review ACK 3e33d170 Tree-SHA512: fd949fae5f1a973d39cb97f2745821ab2f62b98e166e53bc2801f97dcde988e18faaaaa0ffc2a82c170938b3a18078b6162fa35460e6e7c635e681b3c9e5b0a6
2021-07-21fuzz: Temporarily disable failing assert in banman fuzz testMarcoFalke
2021-07-21Merge bitcoin/bitcoin#22428: [Refactor] Rename scriptPubKey -> exec_scriptMarcoFalke
007910388b54abc97057e44a7a8f7241e83c203b [Refactor] Rename scriptPubKey -> exec_script (sanket1729) Pull request description: Rename scriptPubKey to witness_script in ExecuteWitnessScript() function to correctly reflect which script is being executed. For example in segwitv0, this scriptPubKey refers to the script of the form `OP_0 <script_hash>`, but witness_script refers to the script that actually hashes to the `script_hash`. If there is a reason why it's named this way, I would love to know ACKs for top commit: MarcoFalke: review ACK 007910388b54abc97057e44a7a8f7241e83c203b 🖖 theStack: ACK 007910388b54abc97057e44a7a8f7241e83c203b lsilva01: Code Review https://github.com/bitcoin/bitcoin/pull/22428/commits/007910388b54abc97057e44a7a8f7241e83c203b ACK Tree-SHA512: 768e10e656b60b1293beb560fb7adbc2c1495e6db1f54f0c2c63109692ae0c579c856b194b33f72afd0d332159a9796c0e2bd99b79ea5c4b1803469a81301fd6
2021-07-21cli: Implement human readable -getinfo.Klement Tan
2021-07-21Merge bitcoin/bitcoin#22288: Resolve Tor control plane addressW. J. van der Laan
cdd51e8ee156f3bb3135be8aa51530a53734153e torcontrol: Resolve Tor control plane address (Adrian-Stefan Mares) Pull request description: Closes https://github.com/bitcoin/bitcoin/issues/22236 This PR forces the Tor control plane address to be resolved before a connection attempt is made, similar to how the `-proxy` / `-onion` address is resolved. The use case for this change is that the control plane may not have a stable address - in a containerized environment perhaps. ACKs for top commit: jonatack: ACK cdd51e8ee156f3bb3135be8aa51530a53734153e tested various configurations on signet with this branch versus master laanwj: LGTM ACK cdd51e8ee156f3bb3135be8aa51530a53734153e theStack: ACK cdd51e8ee156f3bb3135be8aa51530a53734153e 🪐 prayank23: ACK https://github.com/bitcoin/bitcoin/pull/22288/commits/cdd51e8ee156f3bb3135be8aa51530a53734153e Tree-SHA512: 5335cfcb89089a2acd6d02b88c2022dec60bb74388a99187c901c1c35d32896814d5f81df55c053953276c51fcec263c6ddadd068316f8e428b841bd599fc21e
2021-07-21Merge bitcoin/bitcoin#21934: RPC/blockchain: getblockchaininfo: Include ↵MarcoFalke
versionbits signalling details during LOCKED_IN 2b19f3443efc9e7868746ea1c603b1027d822f32 RPC/blockchain: getblockchaininfo: Include versionbits signalling details during LOCKED_IN (Luke Dashjr) Pull request description: While the signal has no effect during `LOCKED_IN`, the bit is still defined and recommended for measuring uptake. Makes sense to expose statistics too. ACKs for top commit: prayank23: ACK https://github.com/bitcoin/bitcoin/pull/21934/commits/2b19f3443efc9e7868746ea1c603b1027d822f32 Sjors: tACK 2b19f34 theStack: Tested ACK 2b19f3443efc9e7868746ea1c603b1027d822f32 MarcoFalke: review-only ACK 2b19f3443efc9e7868746ea1c603b1027d822f32 Tree-SHA512: a9bb5adb21992586119cbb5f87e5348eabcab11d5a3bf769b00b69e466589a669846e503f8384fa8927fd77da0c2d64a54f13a7a55a62980046d70f8255ddf47
2021-07-21Merge bitcoin/bitcoin#22407: rpc: Return block time in getblockchaininfoMarcoFalke
20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 rpc: Return block time in getblockchaininfo (João Barbosa) Pull request description: Return tip time in `getblockchaininfo`, for some use cases this can save a call to `getblock`. ACKs for top commit: naumenkogs: ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 theStack: re-ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 0xB10C: ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 kristapsk: ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 Zero-1729: re-ACK 20edf4bcf61e9fa310c3d7f3cac0c80a04df5364 Tree-SHA512: 29a920cfff1ef53e0af601c3f93f8f9171f3be47fc84b0fa293cb865b824976e8c1510b17b27d17daf0b8e658dd77d9dc388373395f0919fc4a23cd5019642d5
2021-07-21Merge bitcoin/bitcoin#22505: addrman: Remove unused test_before_evict ↵fanquake
argument from Good() f036dfbb692c4d44d0f59194d089ed0aa1096347 [addrman] Remove unused test_before_evict argument from Good() (John Newbery) Pull request description: This has never been used in the public interface method since it was introduced in #9037. ACKs for top commit: lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/22505/commits/f036dfbb692c4d44d0f59194d089ed0aa1096347 on Ubuntu 20.04. theStack: Code-review ACK f036dfbb692c4d44d0f59194d089ed0aa1096347 Tree-SHA512: 98145d9596b4ae1f354cfa561be1a54c6b8057c920e0ac3d4c1d42c9326b2dad2d44320f4171bb701d97088b216760cca8017b84c8b5dcd2b1dc8f158f28066d
2021-07-20Merge bitcoin/bitcoin#22371: Move pblocktree global to BlockManagerMarcoFalke
faa54e375782b21cbc2761c763128131c569e903 Move pblocktree global to BlockManager (MarcoFalke) fa27f03b4943540aa2eab283d4cf50ad4a1a01f8 Move LoadBlockIndexDB to BlockManager (MarcoFalke) Pull request description: The block tree db is used within BlockManager to write and read the block index, so make the db global a member variable of BlockManager. ACKs for top commit: jamesob: ACK faa54e375782b21cbc2761c763128131c569e903 ([`jamesob/ackr/22371.1.MarcoFalke.move_pblocktree_global_t`](https://github.com/jamesob/bitcoin/tree/ackr/22371.1.MarcoFalke.move_pblocktree_global_t)) theStack: re-ACK faa54e375782b21cbc2761c763128131c569e903 🥧 ryanofsky: Code review ACK faa54e375782b21cbc2761c763128131c569e903. I was thinking this looked like a change Carl would like, so no surprised he [Mega-acked](https://github.com/bitcoin/bitcoin/pull/22371#pullrequestreview-696450475) Tree-SHA512: 1b7badbf503d53f5d4dbd9ed8f2e5c1ebfe48102665197048cc9e37bc87b5cec5f2277f3aae9f73a1095bfe879b19d288286ca3daa28031f5f1b64b1184439a9
2021-07-20[addrman] Remove unused test_before_evict argument from Good()John Newbery
This has never been used in the public interface method since it was introduced in #9037.
2021-07-20Merge bitcoin/bitcoin#22232: refactor: Pass interpreter flags as uint32_t ↵MarcoFalke
instead of signed int fa621ededdfe31a200b77a8787de7e3d2e667aec refactor: Pass script verify flags as uint32_t (MarcoFalke) Pull request description: The flags are cast to unsigned in the interpreter anyway, so avoid the confusion (and fuzz crashes) by just passing them as unsigned from the beginning. Also, the flags are often inverted bit-wise with the `~` operator, which also works on signed integers, but might cause confusion as the sign bit is flipped. Fixes #22233 ACKs for top commit: theStack: Concept and code review ACK fa621ededdfe31a200b77a8787de7e3d2e667aec kristapsk: ACK fa621ededdfe31a200b77a8787de7e3d2e667aec jonatack: ACK fa621ededdfe31a200b77a8787de7e3d2e667aec Tree-SHA512: ea0720f32f823fa7f075309978672aa39773c6019d12b6c1c9d611fc1983a76115b7fe2a28d50814673bb6415c311ccc05b99d6e871575fb6900faf75ed17769
2021-07-20Merge bitcoin/bitcoin#22499: Update assumed chain paramsfanquake
eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 Update assumed chain params (Sriram) Pull request description: Update the relevant variables in `src/chainparams.cpp` for `mainnet`, `testnet`, and `signet` as given [here](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md#before-branch-off). To review this PR, check out [this guide](https://github.com/fanquake/core-review/blob/master/update-assumevalid.md). Note: added a 10% overhead to the base value of `mainnet` in `m_assumed_blockchain_size` ACKs for top commit: MarcoFalke: ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415, checked against my node 🌮 bfolkens: ACK eeddd1c - checked against `mainnet` achow101: Code Review ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 0xB10C: ACK mainnet, testnet, and signet eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 jamesob: ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 ([`jamesob/ackr/22499.1.sriramdvt.update_assumed_chain_par`](https://github.com/jamesob/bitcoin/tree/ackr/22499.1.sriramdvt.update_assumed_chain_par)) darosior: ACK eeddd1c8fa96cf546b0bf92063cefa4fd8c6b415 mainnet and testnet Tree-SHA512: 0ab19d2acc6a854c6aa38fba199d61c68cec40f005d1d54341ea32b59aae9b7d1aabfd21d7c0bc79f54be99d3e71d1d727196cab88f370259fd2c6e002d3e43c
2021-07-20Merge bitcoin/bitcoin#22492: wallet: Reorder locks in dumpwallet to avoid ↵MarcoFalke
lock order assertion 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 tests: Test for dumpwallet lock order issue (Andrew Chow) 25d99e6511d8c43b2025a89bcd8295de755346a7 Reorder dumpwallet so that cs_main functions go first (Andrew Chow) Pull request description: When a wallet is loaded which has an unconfirmed transaction in the mempool, it will end up establishing the lock order of cs_wallet -> cs_main -> cs_KeyStore. If `dumpwallet` is used on this wallet, then a lock order of cs_wallet -> cs_KeyStore -> cs_main will be used, which causes a lock order assertion. This PR fixes this by reordering `dumpwallet` and `GetKeyBirthTimes` (only used by `dumpwallet`). Specifically, in both functions, the function calls which lock cs_main are done prior to locking cs_KeyStore. This avoids the lock order issue. Additionally, I have added a test case to `wallet_dump.py`. Of course testing this requires `--enable-debug`. Fixes #22489 ACKs for top commit: MarcoFalke: review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 🎰 ryanofsky: Code review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4. Nice to reduce lock scope, and good test! prayank23: tACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 lsilva01: Tested ACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 under the same conditions reported in issue #22489 and the `dumpwallet` command completed successfully. Tree-SHA512: d370a8f415ad64ee6a538ff419155837bcdbb167e3831b06572562289239028c6b46d80b23d227286afe875d9351f3377574ed831549ea426fb926af0e19c755
2021-07-20Merge bitcoin/bitcoin#22261: [p2p/mempool] Two small fixes to node broadcast ↵fanquake
logic 5a77abd4e657458852875a07692898982f4b1db5 [style] Clean up BroadcastTransaction() (John Newbery) 7282d4c0363ab5152baa34af626cb49afbfddc32 [test] Allow rebroadcast for same-txid-different-wtxid transactions (glozow) cd48372b67d961fe661990a2c6d3cc3d91478924 [mempool] Allow rebroadcast for same-txid-different-wtxid transactions (John Newbery) 847b6ed48d7bacec9024618922e9b339d2d97676 [test] Test transactions are not re-added to unbroadcast set (Duncan Dean) 2837a9f1eaa2c6bf402d1d9891d9aa84c4a56033 [mempool] Only add a transaction to the unbroadcast set when it's added to the mempool (John Newbery) Pull request description: 1. Only add a transaction to the unbroadcast set when it's added to the mempool Currently, if BroadcastTransaction() is called to rebroadcast a transaction (e.g. by ResendWalletTransactions()), then we add the transaction to the unbroadcast set. That transaction has already been broadcast in the past, so peers are unlikely to request it again, meaning RemoveUnbroadcastTx() won't be called and it won't be removed from m_unbroadcast_txids. Net processing will therefore continue to attempt rebroadcast for the transaction every 10-15 minutes. This will most likely continue until the node connects to a new peer which hasn't yet seen the transaction (or perhaps indefinitely). Fix by only adding the transaction to the broadcast set when it's added to the mempool. 2. Allow rebroadcast for same-txid-different-wtxid transactions There is some slightly unexpected behaviour when: - there is already transaction in the mempool (the "mempool tx") - BroadcastTransaction() is called for a transaction with the same txid as the mempool transaction but a different witness (the "new tx") Prior to this commit, if BroadcastTransaction() is called with relay=true, then it'll call RelayTransaction() using the txid/wtxid of the new tx, not the txid/wtxid of the mempool tx. For wtxid relay peers, in SendMessages(), the wtxid of the new tx will be taken from setInventoryTxToSend, but will then be filtered out from the vector of wtxids to announce, since m_mempool.info() won't find the transaction (the mempool contains the mempool tx, which has a different wtxid from the new tx). Fix this by calling RelayTransaction() with the wtxid of the mempool transaction in this case. The third commit is a comment/whitespace only change to tidy up the BroadcastTransaction() function. ACKs for top commit: duncandean: reACK 5a77abd naumenkogs: ACK 5a77abd4e657458852875a07692898982f4b1db5 theStack: re-ACK 5a77abd4e657458852875a07692898982f4b1db5 lsilva01: re-ACK https://github.com/bitcoin/bitcoin/pull/22261/commits/5a77abd4e657458852875a07692898982f4b1db5 Tree-SHA512: d1a46d32a9f975220e5b432ff6633fac9be01ea41925b4958395b8d641680500dc44476b12d18852e5b674d2d87e4d0160b4483e45d3d149176bdff9f4dc8516
2021-07-20Merge bitcoin/bitcoin#22096: p2p: AddrFetch - don't disconnect on ↵fanquake
self-announcements 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 test: Add functional test for AddrFetch connections (Martin Zumsande) c34ad3309f93979b274a37de013502b05d25fad8 net, rpc: Enable AddrFetch connections for functional testing (Martin Zumsande) 533500d9072b7d5a36a6491784bdeb9247e91fb0 p2p: Add timeout for AddrFetch peers (Martin Zumsande) b6c5d1e450dde6a54bd785504c923adfb45c7060 p2p: AddrFetch - don't disconnect on self-announcements (Martin Zumsande) Pull request description: AddrFetch connections (old name: oneshots) are intended to be short-lived connections on which we ask a peer for addresses via `getaddr` and disconnect after receiving them. This is done by disconnecting after receiving the first `addr`. However, it is no longer working as intended, because nowadays, the first `addr` a typical bitcoin core node sends is its self-announcement. So we'll disconnect before the peer gets a chance to answer our `getaddr`. I checked that this affects both `-seednode` peers specified manually, and DNS seeds when AddrFetch is used as a fallback if DNS doesn't work for us. The current behavior of getting peers via AddrFetch when starting with an empty addrman would be to connect to the peer, receive its self-announcement and add it to addrman, disconnect, reconnect to the same peer again as a full outbound (no other addresses in addrman) and then receive more `addr`. This is silly and not in line with AddrFetch peer being intended to be short-lived peers.  Fix this by only disconnecting after receiving an `addr` message of size > 1. [Edit] As per review discussion, this PR now also adds a timeout after which we disconnect if we haven't received any suitable `addr`, and a functional test. ACKs for top commit: amitiuttarwar: reACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 naumenkogs: ACK 5730a43703f7e5a5ca26245ba3b55fbdd027d0b6 jnewbery: ACK 5730a43703 Tree-SHA512: 8a81234f37e827705138eb254223f7f3b3bf44a06cb02126fc7990b0d231b9bd8f07d38d185cc30d55bf35548a6fdc286b69602498d875b937e7c58332158bf9
2021-07-20[net processing] Default initialize m_recent_confirmed_transactionsJohn Newbery
Now that m_recent_confirmed_transactions is owned by PeerManagerImpl, and PeerManagerImpl's lifetime is managed by the node context, we can just default initialize m_recent_confirmed_transactions during object initialization. We can also remove the unique_ptr indirection.
2021-07-20scripted-diff: Rename recentRejectsJohn Newbery
-BEGIN VERIFY SCRIPT- ren() { sed -i "s:\<$1\>:$2:g" $(git grep -l "\<$1\>" ./src ./test); } ren recentRejects m_recent_rejects -END VERIFY SCRIPT-
2021-07-20[net processing] Default initialize recentRejectsJohn Newbery
Now that recentRejects is owned by PeerManagerImpl, and PeerManagerImpl's lifetime is managed by the node context, we can just default initialize recentRejects during object initialization. We can also remove the unique_ptr indirection.
2021-07-20[net processing] Default initialize m_stale_tip_check_timeJohn Newbery