aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-07-10doc: Use precise permission flags where possibleMarcoFalke
2020-07-09Merge #19191: net: Extract download permission from nobanMarcoFalke
fa0540cd46eaf44d9e1a9f91c3a937986826c4fa net: Extract download permission from noban (MarcoFalke) Pull request description: It should be possible to grant nodes in a local network (e.g. home, university, enterprise, ...) permission to download blocks even after the maxuploadtarget is hit. Currently this is only possible by setting the `noban` permission, which has some adverse effects, especially if the peers can't be fully trusted. Fix this by extracting a `download` permission from `noban`. ACKs for top commit: jonatack: ACK fa0540c Sjors: re-utACK fa0540cd46eaf44d9e1a9f91c3a937986826c4fa Tree-SHA512: 255566baa43ae925d93f5d0a3aa66b475a556d1590f662a88278a4872f16a1a05739a6119ae48a293011868042e05cb264cffe5822a50fb80db7333bf44376d9
2020-07-09Merge #19314: refactor: Use uint16_t instead of unsigned shortWladimir J. van der Laan
1cabbddbca615b26aa4510c75f459c28d6fe0afd refactor: Use uint16_t instead of unsigned short (Aaron Hook) Pull request description: I wanted to see if the `up for grabs` label works and looked at PR #17822 originally opend by ahook I saw it had many acks for example by jonatack and practicalswift but needed rebasing. So I checked out the remote branch rebased it resolved three conflicts and continued the rebase. Hope everything is as expected (: ACKs for top commit: sipsorcery: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd. practicalswift: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd -- patch looks correct :) laanwj: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd hebasto: ACK 1cabbddbca615b26aa4510c75f459c28d6fe0afd, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 0e6bf64f274aae5dacb188358b4d5f65ccb207d4f70922f039bc4ed7934709418ddad19f8bfb7462517427837c3d2bb3f86ef284bb40e87119aad2a1e148d9d6
2020-07-09Merge #18993: qt: increase console command max lengthfanquake
fc6a637a013daeb14b2f93652d7f494f3b8462aa qt: increase console command max length (10xcryptodev) Pull request description: fix #17618 Tested the examples https://github.com/bitcoin/bitcoin/issues/17618#issuecomment-559538070 and works ACKs for top commit: MarcoFalke: Approach ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa hebasto: ACK fc6a637a013daeb14b2f93652d7f494f3b8462aa, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 4975d7fa4c13a6b0f50f5754c3e04eb5a42b1411c385dc883d9948b6fc0dee38900ba2a418218a9a30ce39988a27d22f3ff3a02f0fa44f4136f01eef473efeca
2020-07-09Merge #19317: Add a left-justified width field to log2_work component for a ↵Wladimir J. van der Laan
uniform debug.log output c8583022800410afeb75e0154df7290d080d581d Change format of log2_work for uniform output (zero-padded) (jmorgan) Pull request description: Motivation: It's jarring to watch the output of `tail -f ~/btcdata/debug.log` scroll by and very frequently see columns not lining up correctly because `log2_work` somtimes has less precision than 8 digits. Current display: ``` 2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo) 2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868 tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo) ``` Display with this commit: ``` 2020-06-18T02:54:42Z UpdateTip: new best=0000000000000000107f877e4920643f9fb06090fa7551cd1cdd83b857f520aa height=382038 version=0x00000003 log2_work=83.558653 tx=90953616 date='2015-11-04T17:11:44Z' progress=0.166675 cache=117.6MiB(966410txo) 2020-06-18T02:54:51Z UpdateTip: new best=0000000000000000019a4de585d30d1a8cc13c7a1972d11b4945635c9556acb5 height=382039 version=0x00000003 log2_work=83.55868 tx=90955936 date='2015-11-04T17:19:39Z' progress=0.166679 cache=117.9MiB(968799txo) ``` ACKs for top commit: practicalswift: ACK c8583022800410afeb75e0154df7290d080d581d -- patch looks great :) achow101: ACK c8583022800410afeb75e0154df7290d080d581d laanwj: Tested ACK c8583022800410afeb75e0154df7290d080d581d Tree-SHA512: 16cbe419c4993ad51019c676e8ca409ef1025b803cc598437c780dd7ca003d7e4ad421f451e9a374e0070ee9b3ee601b7aba849e1f346798f9321d1bce5c4401
2020-07-09Merge #19468: refactor: Drop unused CDBWrapper methodsWladimir J. van der Laan
4b5ac258812a1e8848862689ff333587cf274892 Drop unused CDBWrapper methods (Hennadii Stepanov) Pull request description: `CDBWrapper::Flush()` and `CDBWrapper::Sync()` are not used in the code. ACKs for top commit: promag: ACK 4b5ac258812a1e8848862689ff333587cf274892. laanwj: ACK 4b5ac258812a1e8848862689ff333587cf274892 Tree-SHA512: 06115c59e75995d496173a64ceea1b9bb1b4fe3eac8bf4f59df68b87b112b5b3e8065298dcd5c4c7408544f76ee62922325acc2208619d830fd5dbb420cdda5c
2020-07-09Merge #18896: qt: Reset toolbar after all wallets are closedfanquake
1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 qt: Reset toolbar after all wallets are closed (Hennadii Stepanov) Pull request description: If the last open wallet is closed from the non-"Overview" tab, that tab remains active when a new wallet is opened: ![Screenshot from 2020-05-06 09-00-26](https://user-images.githubusercontent.com/32963518/81142394-46821880-8f78-11ea-84b5-1d02f2b7f3f1.png) This PR fixes this bug. ACKs for top commit: promag: Code review ACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0. luke-jr: utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 jonasschnelli: utACK 1e9bfd4926a3cbb9db877c773a60b0bbbbe8bde0 Tree-SHA512: a8dfd7591267e9544ad40c87581d554e5cfaad4b2a5bbfdbaf2596dc6869d2ac6cf7877adfef3d528fc61b081d40c6e30d787bbd7280ef7946aa7f7d9bc8b18e
2020-07-09Merge #19470: banlist: log post-swept banlist size at startupfanquake
0b8ba84659b56b12d74587ea31b6062fce887ba3 banlist: log post-swept banlist size at startup (fanquake) Pull request description: We are currently logging the size of the banlist from before `SweepBanned()` has been called, meaning the value may be incorrect. i.e banlist.dat had `1`ban. That ban is swept on startup. We log "loaded 1 banned node..". Actual banlist size is `0`. ACKs for top commit: jonatack: Code review ACK 0b8ba84659b `m_banned` is set in SetBanned and is updated by SweepBanned before the logging. laanwj: Code review ACK 0b8ba84659b56b12d74587ea31b6062fce887ba3 jnewbery: Code review ACK 0b8ba84659b56b12d74587ea31b6062fce887ba3 Tree-SHA512: 1d6e363d6c68d7cc214dd685df3d2d27572f6a58a4c0e43c03cfbb03bc01badb6a10ecae403d137094bb316d27f33feb6be15b4e23ef1e9496cd0b3c23c21698
2020-07-09Merge #19454: tools: `.clang-format` compat with clang versions < 9fanquake
b9253c7d2089d3d159dcc10118ce5a219d9a6881 tools: clang-format 6 compatibility (Jon Atack) Pull request description: Our `.clang-format` settings inadvertently lost compatibility with Clang versions < 9 in #19095, including for Debian stable. This patch returns compatibility in the interim until the distros update. See discussion from https://github.com/bitcoin/bitcoin/pull/19095#issuecomment-651926138. ACKs for top commit: MarcoFalke: Approach ACK b9253c7d2089d3d159dcc10118ce5a219d9a6881 , haven't tested Tree-SHA512: 4af541a195f48d84ffb80e23aaefb624c66bc78f087c8d92b4af5a654420b69fedf25272c6e4fde2688ff88412d306b7a990ce1e15d8b24180374c625a253fb6
2020-07-09net: Extract download permission from nobanMarcoFalke
2020-07-09refactor: clean up PeriodicFlush()John Newbery
2020-07-09Merge #19320: wallet: Replace CDataStream& with CDataStream&& where appropriateMarcoFalke
fa8a341b88cabfd7f8d702db7cb9972b0804bf2a wallet: Replace CDataStream& with CDataStream&& where appropriate (MarcoFalke) fa021e9a5b7e930a3db0febb416942dea3a90a8f wallet: Remove confusing double return value ret+success (MarcoFalke) Pull request description: The keys and values are only to be used once because their memory is set to zero. Make that explicit by moving the bytes into the lower level methods. ACKs for top commit: sipa: utACK fa8a341b88cabfd7f8d702db7cb9972b0804bf2a ryanofsky: Code review ACK fa8a341b88cabfd7f8d702db7cb9972b0804bf2a. Nice changes. Tree-SHA512: 5c0218bae0f3cd2a07346f1bbf4ad232e5dde7ef2f807d82cc6cfd208d11fe60c8b0f37e7986087b52fbfc79cdfd33c3c8a5822b3d4d9a44d1c6b09e354fc424
2020-07-08Merge #19347: [net] Make cs_inventory nonrecursiveMarcoFalke
e8a2822119233ade0de84f791a9e92918a3d6896 [net] Don't try to take cs_inventory before deleting CNode (John Newbery) 3556227ddd3365cfac43b307204d73058b2943f0 [net] Make cs_inventory a non-recursive mutex (John Newbery) 344e831de54f7b864f03a90f6cb19692eafcd463 [net processing] Remove PushBlockInventory and PushBlockHash (John Newbery) Pull request description: - Remove PushBlockInventory() and PushBlockHash(). These are one-line functions that can easy be inlined into the calling code. Doing so also allows us to eliminate the one place that cs_inventory is recursively locked. - Make cs_inventory a nonrecursive mutex - Remove a redundant TRY_LOCK of cs_inventory when deleting CNode. ACKs for top commit: sipa: utACK e8a2822119233ade0de84f791a9e92918a3d6896 MarcoFalke: ACK e8a2822119233ade0de84f791a9e92918a3d6896 🍬 hebasto: re-ACK e8a2822119233ade0de84f791a9e92918a3d6896 Tree-SHA512: dbc721d102cdef7b5827a8f2549daf8b54f543050266999a7ea56c9f36618565b71e31ce0beb1209ba2db43d15388be173355a03fb6db8ad24e2475b145050bd
2020-07-08banlist: log post-swept banlist size at startupfanquake
We are currently logging the size of the banlist before SweepBanned() has been called, meaning the value may be incorrect.
2020-07-08Drop unused CDBWrapper methodsHennadii Stepanov
2020-07-07Merge #19219: Replace automatic bans with discouragement filterPieter Wuille
2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Clean up separated ban/discourage interface (Pieter Wuille) b691f2df5f7d443c0c9ee056ab94aa0fc19566d5 Replace automatic bans with discouragement filter (Pieter Wuille) Pull request description: This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned since #14929, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to "discouraged" to better reflect reality. ACKs for top commit: naumenkogs: utACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8 amitiuttarwar: code review ACK 2ad58381ff jonatack: ACK 2ad5838 per changes since last review `git range-diff 3276c14 1f7e0ca 2ad5838` jnewbery: Code review ACK 2ad58381fffb33d611abf900b73d9e6b5a4e35f8 Tree-SHA512: 5dedef401d9cbfa026812651303e6286223563dbeed7a10766ed536ac9e3f29ed4bd0df29cc6deadceeb35cbe9f066346add14ef0833958ca9f93d123fe7aab5
2020-07-06Merge #19328: Add gettxoutsetinfo hash_type optionMarcoFalke
40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a test: Test gettxouttsetinfo hash_type option (Fabian Jahr) f17a4d1c4ddce6935a353004898fb4e8618a213e rpc: Add hash_type NONE to gettxoutsetinfo (Fabian Jahr) a712cf6f6801157667fcf36d1c498b6fff6d328a rpc: gettxoutsetinfo can specify hash_type (only legacy option for now) (Fabian Jahr) 605884ef21318fc3f326dbdf4901cb353ba63fab refactor: Extract GetBogoSize function (Fabian Jahr) Pull request description: This is another intermediate part of the Coinstats Index (tracked in #18000). Sjors suggested [here](https://github.com/bitcoin/bitcoin/pull/18000#issuecomment-641423019) that the part of the changes in #19145 that don't rely on the new `hash_type` muhash, i.e. that are for `hash_type=none`, could be merged separately from everything involving muhash. So these changes are extracted from #19145 here and can be merged without any other requirements. Building the index with no UTXO set hash is still valuable because `gettxoutsetinfo` can still be used to audit the `total_amount` for example. By itself this PR is not a huge improvement, `hash_type=none` is speeding up `gettxoutsetinfo` by about 10%, but it enables the implementation of an index on top of it in a follow-up and that means large parts of the index code of Coinstats Index can be merged while reviews for the hashing algorithm might take longer. ACKs for top commit: MarcoFalke: ACK 40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a 🖨 Sjors: tACK 40506bf93f955adfbc446c4d5fee4fa8bcfd7d9a Tree-SHA512: 3964c2b8eed427511b1aa9b2ef285dff27dc4d1537d72c3911e435b6e6b40912232da4acb3a09bd19a0372ddffa44103388d8a650169d95a4a727b970d210add
2020-07-06tools: clang-format 6 compatibilityJon Atack
2020-07-05Merge #19324: wallet: Move BerkeleyBatch static functions to BerkeleyDatabaseMarcoFalke
d8e9ca66d119d80acfb2bb3c8940c386ce0fc226 walletdb: Move Rewrite into BerkeleyDatabase (Andrew Chow) 91d109156d63ff81cda534bd7bec8369af0027dd walletdb: Move PeriodicFlush into WalletDatabase (Andrew Chow) 8f1bcf8b7b6e47c05f2e43dd98ec3505b888d8b3 walletdb: Combine VerifyDatabaseFile and VerifyEnvironment (Andrew Chow) Pull request description: The `BerkeleyBatch` class has 4 static functions that operate on `BerkeleyDatabase` or `BerkeleyEnvironment`. It doesn't make sense for these to be standalone nor for them to be static functions. So instead, move them from `BerkeleyBatch` into `BerkeleyDatabase` and make them member functions instead of static. `BerkeleyBatch::VerifyEnvironment` and `BerkeleyBatch::VerifyDatabaseFile` are combined into a single `BerkeleyDatabase::Verify` function that operates on that `BerkeleyDatabase` object. `BerkeleyBatch::Rewrite` and `BerkeleyBatch::PeriodicFlush` both took a `BerkeleyDatabase` as an argument and did stuff on it. So we just make it a member function so it doesn't need to take a database as an argument. Part of #18971 ACKs for top commit: MarcoFalke: re-ACK d8e9ca66d1 only change is test fixup 🤞 promag: Code review ACK d8e9ca66d119d80acfb2bb3c8940c386ce0fc226, good stuff. Tree-SHA512: 9847e55b13d98bf4e5636cc14bc3f5351d56737f7e320fafffaed128606240765599e5400382c5aecac06690f7e36265ca3e1031f3f6d8a9688f6d5cb1bacd2a
2020-07-04Merge #19277: util: Add Assert identity functionMarcoFalke
fab80fef61ddd4afeff6e497c7e76bffcd05e8a4 refactor: Remove unused EnsureChainman (MarcoFalke) fa34587f1c811d99200453b0936219c473f514b0 scripted-diff: Replace EnsureChainman with Assert in unit tests (MarcoFalke) fa6ef701adba1cb48535cac25fd43c742a82e40d util: Add Assert identity function (MarcoFalke) fa457fbd3387661e1973a8f4e5cc2def79e0c625 move-only: Move NDEBUG compile time check to util/check (MarcoFalke) Pull request description: The utility function is primarily useful to dereference pointer types, which are known to be not null at that time. For example, the ArgsManager is known to exist when the wallets are started: https://github.com/bitcoin/bitcoin/pull/18923/files#diff-fdb2a1a1d8bc790fcddeb6cf5a42ac55R503 . Instead of silently relying on that assumption, `Assert` can be used to abort the program and avoid UB should the assumption ever be violated. ACKs for top commit: promag: Tested ACK fab80fef61ddd4afeff6e497c7e76bffcd05e8a4. ryanofsky: Code review ACK fab80fef61ddd4afeff6e497c7e76bffcd05e8a4 Tree-SHA512: 830fba10152ba17d47c4dd42809c7e26f9fe6d38e17a2d5b3f054fd644a5c4c9841286ac421ec9bb28cea9f5faeb659740fcf00de6cc589d423fee7694c42d16
2020-07-03Clean up separated ban/discourage interfacePieter Wuille
2020-07-03Replace automatic bans with discouragement filterPieter Wuille
This patch improves performance and resource usage around IP addresses that are banned for misbehavior. They're already not actually banned, as connections from them are still allowed, but they are preferred for eviction if the inbound connection slots are full. Stop treating these like manually banned IP ranges, and instead just keep them in a rolling Bloom filter of misbehaving nodes, which isn't persisted to disk or exposed through the ban framework. The effect remains the same: preferred for eviction, avoided for outgoing connections, and not relayed to other peers. Also change the name of this mechanism to better reflect reality; they're not banned, just discouraged. Contains release notes and several interface improvements by John Newbery.
2020-07-03Merge #19413: refactor: Remove confusing BlockIndex globalMarcoFalke
fa0dfdf447d5b84a1849dc823d8508463600136a refactor: Remove confusing BlockIndex global (MarcoFalke) Pull request description: The global `::BlockIndex()` is problematic for several reasons: * It returns a mutable reference to the block tree, without the appropriate lock annotation (`m_block_index` is guarded by `cs_main`). The current code is fine, but in the future this might lead to accidental races and data corruption. * The rpc server shouldn't rely on node globals, but rather a context that is passed in to the RPC method. * Tests might want to spin up their own block tree, and thus should also not rely on a single global. Fix all issues by removing the global ACKs for top commit: promag: Code review ACK fa0dfdf447d5b84a1849dc823d8508463600136a. jonatack: re-ACK fa0dfdf Tree-SHA512: 8f158fc5e1c67e73588a21c25677b3fa0fe442313b13ec24b87054806c59607d6ba0c062a865ce3e0ee568706bd0d1faa84febda21aff5bcd65dab172f74c52f
2020-07-03Merge #19215: psbt: Include and allow both non_witness_utxo and witness_utxo ↵Samuel Dobson
for segwit inputs 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 tests: Check that segwit inputs in psbt have both UTXO types (Andrew Chow) 46004790588c24174a0bec49b540d158ce163ffd psbt: always put a non_witness_utxo and don't remove it (Andrew Chow) 5279d8bc07d601fe6a67ad665fbc7591fe73c7de psbt: Allow both non_witness_utxo and witness_utxo (Andrew Chow) 72f6bec1da198764d4648a10a61c485e7ab65e9e rpc: show both UTXOs in decodepsbt (Andrew Chow) Pull request description: Due to recent changes to hardware wallets, the full previous transaction will need to be provided for segwit inputs. Since some software may be checking for the existence of a `witness_utxo` to determine whether to produce a segwit signature, we keep that field to ease the transition. Because all of the sanity checks implemented by the `IsSane` functions were related to having mixed segwit and non-segwit data in a PSBT, those functions are removed as those checks are no longer proper. Some tests are updated/removed to accommodate this and a simple test added to check that both UTXOs are being added to segwit inputs. As discussed in the wallet IRC meeting, our own signer will not require `non_witness_utxo` for segwit inputs. ACKs for top commit: Sjors: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 (didn't retest compared to 836d6fc, but fortunately HWI's CI tracks our master branch, with a bunch of hardware wallet simulators) ryanofsky: Code review re-ACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3. No changes since last review, but now I understand the context better. I think it would good to improve the comments as suggested https://github.com/bitcoin/bitcoin/pull/19215#discussion_r447889473 and maybe refer to meshcollider: utACK 84d295e51341a126a6c3cbeea7a8caa04c7b5bc3 Tree-SHA512: ccc1fd3c16ac3859f5aca4fa489bd40f68be0b81bbdc4dd51188bbf28827a8642dc8b605a37318e5f16cf40f1c4910052dace2f27eca21bb58435f02a443e940
2020-07-02Merge #19396: refactor: Remove confusing OutputType::CHANGE_AUTOWladimir J. van der Laan
fa927ff884ae373c676deed63180a8d238872cdc Enable Wswitch for OutputType (MarcoFalke) faddad71f648ed99734f4f8811bd4bc7232ca670 Remove confusing OutputType::CHANGE_AUTO (MarcoFalke) fa2eb383522249a5f4d48726c520cec5de496614 interfaces: Remove unused getDefaultChangeType (MarcoFalke) Pull request description: `OutputType::CHANGE_AUTO` is problematic for several reasons: * An output that is not change must never be described by `CHANGE_AUTO`. Simply allowing that option makes the code confusing and review harder than it needs to be. * To make review even harder, `CHANGE_AUTO` requires `-Wswitch` to be disabled for `OutputType` Fix both issues by removing `CHANGE_AUTO` and then enabling `-Wswitch` for `OutputType` ACKs for top commit: promag: Code review ACK fa927ff884ae373c676deed63180a8d238872cdc. laanwj: Code review ACK fa927ff884ae373c676deed63180a8d238872cdc Tree-SHA512: 24fd809757aa343866c94dafe9a7130b50cda4f77c97666d407f99b813f75b115a7d8e688a6bc2a737e87cba64ddd4e43f2b3c5538fd35fabb5845807bb39134
2020-07-01Merge bitcoin-core/gui#17: doc: Remove outdated comment in TransactionTablePrivMarcoFalke
faebb60b8d009e52d585cffd0f124a0f2fd1a66d doc: Remove outdated comment in TransactionTablePriv (MarcoFalke) Pull request description: Locks are no longer taken upfront, so remove the outdated comment ACKs for top commit: hebasto: ACK faebb60b8d009e52d585cffd0f124a0f2fd1a66d, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: cd6df24d49d17e58049ac9b261c5e07c8e85ed1aacb547b13c0e55139339d7fcc3b1f766ea2e27d758ea77deadc01f7e28781be1515323c82b9012cee8fd488b
2020-07-01Enable Wswitch for OutputTypeMarcoFalke
2020-07-01Remove confusing OutputType::CHANGE_AUTOMarcoFalke
2020-07-01wallet: Replace boost::none with nulloptMarcoFalke
2020-07-01script: Fix boost/C++17 compile failureMarcoFalke
script/standard.cpp:297:48: error: temporary of type 'boost::static_visitor<CScript>' has protected destructor return boost::apply_visitor(CScriptVisitor{}, dest); ^ /usr/include/boost/variant/static_visitor.hpp:53:5: note: declared protected here ~static_visitor() = default; ^ 1 error generated.
2020-07-01walletdb: Move Rewrite into BerkeleyDatabaseAndrew Chow
Make Rewrite actually a member of BerkeleyDatabase instead of a static function in BerkeleyBatch
2020-07-01walletdb: Move PeriodicFlush into WalletDatabaseAndrew Chow
Make PeriodicFlush a non-static member of WalletDatabase instead of WalletBatch.
2020-07-01walletdb: Combine VerifyDatabaseFile and VerifyEnvironmentAndrew Chow
Combine these two functions into a single Verify function that is a member of WalletDatabase. Additionally, these are no longer static.
2020-07-01Merge #19028: test: Set -logthreadnames in unit testsWladimir J. van der Laan
99993489da9bc003b823bcab10e5f5297b369431 test: Set -logthreadnames in unit tests (MarcoFalke) fa4ea997b4da1ae0afafba223fff9efbeefaf555 init: Setup scheduler in tests and init in exactly the same way (MarcoFalke) Pull request description: Generally the unit tests are single threaded, with the exception of the script check threads, the schedule, and optionally indexer threads. Like the functional tests, the thread name can serve additional debug information, so set `-logthreadnames` in unit tests. Can be tested with ``` ./src/test/test_bitcoin -l test_suite -t validation_tests/test_combiner_all -- DEBUG_LOG_OUT ACKs for top commit: laanwj: ACK 99993489da9bc003b823bcab10e5f5297b369431 Tree-SHA512: 3bdbfc211da146da64b50b0826246aff5c611a84b69ab896a55b3c9d1adc92c5975da36ab92aee577df82e229c4326b477f4105bfdd1a5df4c9a0b018cf61602
2020-07-01Merge #19256: gui: change combiner for signals to optional_last_valueWladimir J. van der Laan
f1a0314c537791f202dfb7c1209f0e04ba7988c3 gui: change combiner for signals to optional_last_value (Cory Fields) Pull request description: [`optional_last_value`](https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html), which does not throw, has replaced `last_value` as Boosts default combiner. Besides being better supported, it also doesn't trigger gcc's `-Wmaybe-unitialized` warning, presumably because exceptions no longer bubble-up out of signals: ```bash In file included from ui_interface.cpp:9: /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]': /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ^ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here optional<T> value; ^~~~~ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp: In member function 'boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::result_type boost::signals2::detail::signal_impl<R(Args ...), Combiner, Group, GroupCompare, SlotFunction, ExtendedSlotFunction, Mutex>::operator()(Args ...) [with Combiner = boost::signals2::last_value<bool>; Group = int; GroupCompare = std::less<int>; SlotFunction = boost::function<bool(const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; ExtendedSlotFunction = boost::function<bool(const boost::signals2::connection&, const bilingual_str&, const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&, unsigned int)>; Mutex = boost::signals2::mutex; R = bool; Args = {const bilingual_str&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, unsigned int}]': /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ^ /bitcoin/depends/x86_64-pc-linux-gnu/share/../include/boost/signals2/last_value.hpp:43:21: note: '*((void*)& value +1)' was declared here optional<T> value; ^~~~~ ``` The change in default happened in [Boost 1.39.0](https://www.boost.org/users/history/version_1_39_0.html) (along with the introduction of the Signals2 library. More information is also available here https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4: > The default combiner for Boost.Signals2 has changed from the last_value combiner used by default in the original Boost.Signals library. > This is because last_value requires that at least 1 slot be connected to the signal when it is invoked (except for the last_value<void> specialization). > In a multi-threaded environment where signal invocations and slot connections and disconnections may be happening concurrently, it is difficult to fulfill this requirement. When using optional_last_value, there is no requirement for slots to be connected when a signal is invoked, since in that case the combiner may simply return an empty boost::optional. ACKs for top commit: laanwj: ACK f1a0314c537791f202dfb7c1209f0e04ba7988c3 Tree-SHA512: 3600f85019a3591b141dc9207f8a7e66d16d9996cf97fdf08f5133a212d55c591955ab835ffbdca20b5d62711578bc305d5525c75546fa957f180192e2a80c1e
2020-07-01Merge #19308: wallet: BerkeleyBatch Handle cursor internallyWladimir J. van der Laan
ca24edfbc1941ed0a3c9586416dae4e84794eb66 walletdb: Handle cursor internally (Andrew Chow) Pull request description: Instead of returning a Dbc (BDB cursor object) and having the caller deal with the cursor, make BerkeleyBatch handle the cursor internally. Split from #18971 ACKs for top commit: ryanofsky: Code review ACK ca24edfbc1941ed0a3c9586416dae4e84794eb66. Changes since last review: StartCursor rename, moving CloseCursor calls near returns promag: Code review ACK ca24edfbc1941ed0a3c9586416dae4e84794eb66. Tree-SHA512: f029b498c7f275aedca53ce7ade7cb99c82975fd6cad17346a4990fb3bcc54e2a5309b32053bd13def9ee464d331b036ac79abb8fc4fa561170c6cfc85283447
2020-07-01gui: change combiner for signals to optional_last_valueCory Fields
optional_last_value, which does not throw, has replaced optional_value as boost's default combiner. Besides being better supported, it also doesn't trigger gcc's -Wmaybe-unitialized warning, presumably because exceptions no longer bubble-up out of signals: ```bash boost/signals2/last_value.hpp:54:36: warning: '*((void*)& value +1)' may be used uninitialized in this function [-Wmaybe-uninitialized] if(value) return value.get(); ``` The change in default happened in Boost 1.39.0 (along with the introduction of the signals 2 library. More information is available here: https://www.boost.org/doc/libs/1_73_0/doc/html/signals2/rationale.html#id-1.3.36.9.4 and here: https://www.boost.org/doc/libs/1_73_0/doc/html/boost/signals2/optional_last_value.html Co-authored-by: fanquake <fanquake@gmail.com>
2020-07-01Merge #19331: build: Do not include server symbols in walletWladimir J. van der Laan
faca73000fa8975c28f6be8be01957c1ae94ea62 ci: Install fixed version of clang-format for linters (MarcoFalke) fa4695da4c69646b58a8fa0b6b30146bb234deb8 build: Sort Makefile.am after renaming file (MarcoFalke) cccc2784a3bb10fa8e43be7e68207cafb12bd915 scripted-diff: Move ui_interface to the node lib (MarcoFalke) fa72ca6a9d90d66012765b0043fd819698b94ba8 qt: Remove unused includes (MarcoFalke) fac96e6450d595fe67168cb7afa7692da6cc9973 wallet: Do not include server symbols (MarcoFalke) fa0f6c58c1c6d10f04c4e65a424cc51ebca50a8c Revert "Fix link error with --enable-debug" (MarcoFalke) Pull request description: This reverts a hacky workaround from commit b83cc0f, which only happens to work due to compiler optimizations. Then, it actually fixes the linker error. The underlying problem is that the wallet includes symbols from the server (ui_interface), which usually results in linker failures. Though, in this specific case the linker failures have not been observed (unless `-O0`) because our compilers were smart enough to strip unused symbols. Fix the underlying problem by creating a new header-only with the needed symbol and move ui_interface to node to clarify that this is part of libbitcoin_server. ACKs for top commit: Sjors: ACK faca730 laanwj: ACK faca73000fa8975c28f6be8be01957c1ae94ea62 hebasto: re-ACK faca73000fa8975c28f6be8be01957c1ae94ea62, since the [previous](https://github.com/bitcoin/bitcoin/pull/19331#pullrequestreview-434420539) review: Tree-SHA512: e9731f249425aaea50b6db5fc7622e10078cf006721bb87989cac190a2ff224412f6f8a7dd83efd018835302337611f5839e29e15bef366047ed591cef58dfb4
2020-06-30doc: Remove outdated comment in TransactionTablePrivMarcoFalke
2020-06-29refactor: Remove confusing BlockIndex globalMarcoFalke
2020-06-29Merge #19399: refactor: Replace RecursiveMutex with Mutex in rpc/server.cppWladimir J. van der Laan
6fdfeebcc79df62c8bf1cf4b6e9e97d6aefb3eb3 refactor: Replace RecursiveMutex with Mutex in rpc/server.cpp (Hennadii Stepanov) Pull request description: The functions that could lock this mutex, i.e., `SetRPCWarmupStatus()`, `SetRPCWarmupFinished()`, `RPCIsInWarmup()`, `CRPCTable::execute()`, do not call itself recursively, and do not call each other either directly or indirectly. Therefore, the `g_rpc_warmup_mutex` could be a non-recursive mutex. Related to #19303. ACKs for top commit: laanwj: ACK 6fdfeebcc79df62c8bf1cf4b6e9e97d6aefb3eb3 MarcoFalke: ACK 6fdfeebcc79df62c8bf1cf4b6e9e97d6aefb3eb3 Tree-SHA512: 05a8ac58c0cd6a3c9afad9e06ad78059642e3e97715e129f379c0bf6dccdb58e70d05d965f23e7432fd3f02d7f97967a778ffb8e424837891d9d785a9e98964c
2020-06-29Merge #19300: wallet: Handle concurrent wallet loadingMarcoFalke
9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7 qa: Test concurrent wallet loading (João Barbosa) b9971ae5853c1d62e09d976a8705f4f731290d85 wallet: Handle concurrent wallet loading (João Barbosa) Pull request description: This PR handles concurrent wallet loading. This can be tested by running in parallel the following script a couple of times: ```sh for i in {1..10} do src/bitcoin-cli -regtest loadwallet foo src/bitcoin-cli -regtest unloadwallet foo done ``` Eventually the error occurs: ``` error code: -4 error message: Wallet already being loading. ``` For reference, loading and already loaded wallet gives: ``` error code: -4 error message: Wallet file verification failed. Error loading wallet w1. Duplicate -wallet filename specified. ``` Fixes #19232. ACKs for top commit: MarcoFalke: Concept ACK 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7 I have not reviewed the code hebasto: ACK 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7, tested on Linux Mint 20 (x86_64): ryanofsky: Code review good-but-not-ideal ACK 9b009fae6e2eb0ab2ee7ce7882c3556a9ac363a7 Tree-SHA512: 0ccd77b03c0926e4c4e51efb31e193b93cb4b9ffe8bac6bb018f7344c55dfd939b873b8cf5e657dca73e6202eb75aa672de2acb787cc133184b0b3b51e47b972
2020-06-29Merge #19333: refactor: Fix clang compile failureWladimir J. van der Laan
fa3b35a189c4a4fd9667ef0af1c7059471ac8b01 ci: Add test for clang-3.8 C++11 support (MarcoFalke) faa7431fee45b26f7ac2f5fd0b8874cb6afafbd4 refactor: Fix clang compile failure (MarcoFalke) Pull request description: Fix ``` script/standard.cpp:278:22: error: default initialization of an object of const type 'const (anonymous namespace)::CScriptVisitor' without a user-provided default constructor const CScriptVisitor g_script_visitor; ^ {} 1 error generated. ACKs for top commit: laanwj: ACK fa3b35a189c4a4fd9667ef0af1c7059471ac8b01 Tree-SHA512: b3251208945b44530224aadbc10fef1260b479c0b43a5e345501fbfd3579a9fe354b946090e023232852bbb99759da4429b58b137b7b286ddac6bd7960851f7f
2020-06-29Merge #19403: build: improve __builtin_clz* detectionWladimir J. van der Laan
9952242c03fe587b5dff46a9f770e319146103bf build: improve builtin_clz* detection (fanquake) Pull request description: Fixes #19402. The way we currently test for `__builtin_clz*` support with `AC_CHECK_DECLS` does not work with Clang: ```bash configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5 conftest.cpp:100:10: error: builtin functions must be directly called (void) __builtin_clz; ^ 1 error generated. ``` This also removes the `__builtin_clz()` check, as we don't actually use it anywhere, and it's trvial to re-add detection if we do start using it at some point. If this is controversial then I'll add a test for it as well. ACKs for top commit: sipa: ACK 9952242c03fe587b5dff46a9f770e319146103bf laanwj: ACK 9952242c03fe587b5dff46a9f770e319146103bf Tree-SHA512: 695abb1a694a01a25aaa483b4fffa7d598842f2ba4fe8630fbed9ce5450b915c33bf34bb16ad16a16b702dd7c91ebf49fe509a2498b9e28254fe0ec5177bbac0
2020-06-29Merge #19204: p2p: Reduce inv traffic during IBDMarcoFalke
fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 net: Avoid wasting inv traffic during IBD (MarcoFalke) fa06d7e93489e61078cfb95ab767c001536a6e10 refactor: block import implies IsInitialBlockDownload (MarcoFalke) faba65e696a88e5626e587f4e63fa15500cbe4d0 Add ChainstateManager::ActiveChainstate (MarcoFalke) fabf3d64ff2bd14f762810316144bb9fd69c517c test: Add FeeFilterRounder test (MarcoFalke) Pull request description: Tx-inv messages are ignored during IBD, so it would be nice if we told peers to not send them in the first place. Do that by sending two `feefilter` messages: One when the connection is made (and the node is in IBD), and another one when the node leaves IBD. ACKs for top commit: jamesob: ACK fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 ([`jamesob/ackr/19204.1.MarcoFalke.p2p_reduce_inv_traffic_d`](https://github.com/jamesob/bitcoin/tree/ackr/19204.1.MarcoFalke.p2p_reduce_inv_traffic_d)) naumenkogs: utACK fa525e4 gzhao408: ACK https://github.com/bitcoin/bitcoin/commit/fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72 jonatack: re-ACK fa525e4 checked diff `git range-diff 19612ca fa8a66c fa525e4`, re-reviewed, ran tests, ran a custom p2p IBD behavior test at https://github.com/jonatack/bitcoin/commit/9321e0f223ea87d21f6fa42b61bcc8d40a0943de. hebasto: re-ACK fa525e4d1cfda8c1924d2c69f43bd7ae3b98fb72, only rebased since the [previous](https://github.com/bitcoin/bitcoin/pull/19204#pullrequestreview-429519667) review (verified with `git range-diff`). Tree-SHA512: 2c22a5def9822396fca45d808b165b636f1143c4bdb2eaa5c7e977f1f18e8b10c86d4c180da488def38416cf3076a26de15014dfd4d86b2a7e5af88c74afb8eb
2020-06-29Merge #19394: build: Remove unused RES_IMAGESMarcoFalke
53361ddc7591f35df36cc71d11c24f826f209e11 [build] Remove unused RES_IMAGES (Bushstar) Pull request description: Remove RES_IMAGES. Seems to be unused since 2015 in the commit below. https://github.com/bitcoin/bitcoin/commit/98c222b5aa94543fce683b989356b0d8ad1f1d22#diff-9a4f3a253de77bf90b107bdf5283ebc3R317 The src/qt/res/images to which it was used with is no longer present either. ACKs for top commit: hebasto: ACK 53361ddc7591f35df36cc71d11c24f826f209e11, tested on Linux Mint 20 (x86_64). Tree-SHA512: d2f09ae225a4c6c171e1aae4c4a444064dc0502e96130e04ccb718f9fcf611a287c56630dec3e9a8937b5e29040d931a237da36180d2343c23cef30359e46323
2020-06-29Merge #19367: doc: Span pitfallsWladimir J. van der Laan
fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 doc: Mention Span in developer-notes.md (Pieter Wuille) 3502a60418858a8281ddf2f9cd59daa8f01d2fa8 doc: Document Span pitfalls (Pieter Wuille) Pull request description: This is an attempt to document pitfalls with the use of `Span`, following up on comments like https://github.com/bitcoin/bitcoin/pull/18468#issuecomment-622846597 and https://github.com/bitcoin/bitcoin/pull/18468#discussion_r442998211 ACKs for top commit: laanwj: ACK fab57e2b9bc4577fcfcd9fbddbc35d96046c5d88 Tree-SHA512: 8f6f277d6d88921852334853c2b7ced97e83d3222ce40c9fe12dfef508945f26269b90ae091439ebffddf03f939797cb28126b2387f77959069ef8909c25ab53
2020-06-29[build] Remove unused RES_IMAGESBushstar
2020-06-29build: improve builtin_clz* detectionfanquake
The way we currently test with AC_CHECK_DECLS do not work with Clang: ```bash configure:21492: clang++-10 -std=c++11 -c -g -O2 -DHAVE_BUILD_INFO -D__STDC_FORMAT_MACROS conftest.cpp >&5 conftest.cpp:100:10: error: builtin functions must be directly called (void) __builtin_clz; ^ 1 error generated. ``` This also removes the __builtin_clz() check, as we don't actually use it anywhere, and it's trvial to re-add detection if we do start using it at some point.
2020-06-28Merge #19114: scripted-diff: TxoutType C++11 scoped enum classMarcoFalke
fa32adf9dc25540ad27f5b82654c7057d7738627 scripted-diff: TxoutType C++11 scoped enum class (MarcoFalke) fa95a694c492b267e4038674fd3f338dd215ab48 doc: Update outdated txnouttype documentation (MarcoFalke) fa58469c770d8c935a86462634e4e8cd806aa6e3 rpc: Properly use underlying type in GetAllOutputTypes (MarcoFalke) fa41c657022b8f99c8e6718a0e33c5838c412a0b rpc: Simplify GetAllOutputTypes with the Join helper (MarcoFalke) Pull request description: Non-scoped enums can accidentally and silently decay into an integral type. Also, the symbol names of the keys are exported to the surrounding (usually global) namespace. Fix both issues by switching to an `enum class TxoutType` in a (mostly) scripted-diff. ACKs for top commit: practicalswift: ACK fa32adf9dc25540ad27f5b82654c7057d7738627 -- patch looks correct hebasto: re-ACK fa32adf9dc25540ad27f5b82654c7057d7738627, since fa5997bd6fc82e16b597ea96e3c5c665f1f174ab (https://github.com/bitcoin/bitcoin/pull/19114#pullrequestreview-421425198) rebased only (verified with `git range-diff`). Tree-SHA512: f42a9db47f9be89fa4bdd8d2fb05a16726286d8b12e3d87327b67d723f91c7d5a57deb4b2ddae9e1d16fee7a5f8c00828b6dc8909c5db680fc5e0a3cf07cd465