aboutsummaryrefslogtreecommitdiff
path: root/src
AgeCommit message (Collapse)Author
2020-09-17net: CNetAddr: add support to (un)serialize as ADDRv2Vasil Dimov
Co-authored-by: Carl Dong <contact@carldong.me>
2020-09-17test: move HasReason so it can be reusedVasil Dimov
Move the class `HasReason` from `miner_tests.cpp` to `setup_common.h` so that it can be reused by other tests.
2020-09-11util: move HasPrefix() so it can be reusedVasil Dimov
Move the function `HasPrefix()` from `netaddress.cpp` to `util/string.h` so it can be reused by `CNetAddr` methods (and possibly others).
2020-09-10Merge #19841: Implement Keccak and SHA3_256Wladimir J. van der Laan
ab654c7d587b33d62230394663020439f80cee28 Unroll Keccak-f implementation (Pieter Wuille) 3f01ddb01bfffd49dfa131898d1c674ac5d0ac99 Add SHA3 benchmark (Pieter Wuille) 2ac8bf95834c8a43ebf365f09fb610829733134b Implement keccak-f[1600] and SHA3-256 (Pieter Wuille) Pull request description: Add a simple (and initially unoptimized) Keccak/SHA3 implementation based on https://github.com/mjosaarinen/tiny_sha3/blob/master/sha3.c, as one will be needed for TORv3 support (the conversion from BIP155 encoding to .onion notation uses a SHA3-based checksum). In follow-up commits, a benchmark is added, and the Keccakf function is unrolled for a (for me) 4.9x speedup. Test vectors are taken from https://csrc.nist.gov/projects/cryptographic-algorithm-validation-program/secure-hashing#sha3vsha3vss. ACKs for top commit: practicalswift: ACK ab654c7d587b33d62230394663020439f80cee28 -- patch looks correct and no sanitizer complaints when doing some basic fuzz testing of the added code (remember: **don't trust: fuzz!**) :) laanwj: re-ACK ab654c7d587b33d62230394663020439f80cee28 vasild: ACK ab654c7 Tree-SHA512: 8a91b18c46e8fb178b7ff82046cff626180362337e515b92fbbd771876e795da2ed4e3995eb4849773040287f6e687237f469a90474ac53f521fc12e0f5031d9
2020-09-08Do not pass chain params to CheckForStaleTipAndEvictPeers twiceMarcoFalke
2020-09-07Merge #19791: [net processing] Move Misbehaving() to PeerManagerMarcoFalke
bb6a32ce9983c72afa90f41a43a47ffd703ca006 [net processing] Move Misbehaving() to PeerManager (John Newbery) aa114b1c9b06c2bd3ed936bbb9fb32b31f75bdb2 [net_processing] Move SendBlockTransactions into PeerManager (John Newbery) 3115e00f75b41d9765dcbb376e367b25f61a1d58 [net processing] Move MaybePunishPeerForTx to PeerManager (John Newbery) e662e2d42afaf9c67c898634a0f3bc200255b6ea [net processing] Move ProcessOrphanTx to PeerManager (John Newbery) b70cd890e375e904b7f36b3d959e5656f5a5cbcd [net processing] Move MaybePunishNodeForBlock into PeerManager (John Newbery) d7778351bf60a21925a97b7fc4e9df5541b6d995 [net processing] Move ProcessHeadersMessage to PeerManager (John Newbery) 64f6162651420be2f4aa1498f0378a86780bc089 [whitespace] tidy up indentation after scripted diff (John Newbery) 58bd369b0ddd3383f7bdf7840912d18b96545f91 scripted-diff: [net processing] Rename PeerLogicValidation to PeerManager (John Newbery) 2297b26b3ce95e935c0ebb8c38dabf19965054a5 [net_processing] Pass chainparams to PeerLogicValidation constructor (John Newbery) 824bbd1ffba3df7ffa6f5bfaa31298cd484473b1 [move only] Collect all private members of PeerLogicValidation together (John Newbery) Pull request description: Continues the work of moving net_processing logic into PeerLogicValidation. See https://github.com/bitcoin/bitcoin/pull/19704 and https://github.com/bitcoin/bitcoin/pull/19607#discussion_r462032894 for motivation. This PR also renames `PeerLogicValidation` to `PeerManager` as suggested in https://github.com/bitcoin/bitcoin/pull/10756#pullrequestreview-53892618. ACKs for top commit: MarcoFalke: re-ACK bb6a32ce99 only change is rebase due to conflict in struct NodeContext and variable rename 🤸 hebasto: re-ACK bb6a32ce9983c72afa90f41a43a47ffd703ca006, only rebased, and added renaming `s/peer_logic/peerman/` into scripted-diff since my [previous](https://github.com/bitcoin/bitcoin/pull/19791#pullrequestreview-483118079) review (verified with `git range-diff`). Tree-SHA512: a2de4a521688fd25125b401e5575402c52b328a0fa27b3010567008d4f596b960aabbd02b2d81f42658f88f4365443fadb1008150a62fbcea123fb42d85a2c21
2020-09-07[net processing] Move Misbehaving() to PeerManagerJohn Newbery
2020-09-07[net_processing] Move SendBlockTransactions into PeerManagerJohn Newbery
2020-09-07[net processing] Move MaybePunishPeerForTx to PeerManagerJohn Newbery
2020-09-07[net processing] Move ProcessOrphanTx to PeerManagerJohn Newbery
2020-09-07[net processing] Move MaybePunishNodeForBlock into PeerManagerJohn Newbery
2020-09-07[net processing] Move ProcessHeadersMessage to PeerManagerJohn Newbery
2020-09-07[whitespace] tidy up indentation after scripted diffJohn Newbery
2020-09-07scripted-diff: [net processing] Rename PeerLogicValidation to PeerManagerJohn Newbery
-BEGIN VERIFY SCRIPT- sed -i 's/PeerLogicValidation/PeerManager/g' $(git grep -l PeerLogicValidation ./src ./test) sed -i 's/peer_logic/peerman/g' $(git grep -l peer_logic ./src ./test) -END VERIFY SCRIPT- PeerLogicValidation was originally net_processing's implementation to the validation interface. It has since grown to contain much of net_processing's logic. Therefore rename it to reflect its responsibilities. Suggested in https://github.com/bitcoin/bitcoin/pull/10756#pullrequestreview-53892618.
2020-09-07[net_processing] Pass chainparams to PeerLogicValidation constructorJohn Newbery
Keep a references to chainparams, rather than calling the global Params() function every time it's needed. This is fine, since globalChainParams does not get updated once it's been set, and it's available at the point of constructing the PeerLogicValidation object.
2020-09-07[move only] Collect all private members of PeerLogicValidation togetherJohn Newbery
We don't have a project style for ordering class members, but it always makes sense to have no more than one of each public/protected/private specifier. Also move documentation for MaybeDiscourageAndDisconnect to the header.
2020-09-07Merge #19478: Remove CTxMempool::mapLinks data structure memberMarcoFalke
296be8f58e02b39a58f017c52294aceed22c3ffd Get rid of unused functions CTxMemPool::GetMemPoolChildren, CTxMemPool::GetMemPoolParents (Jeremy Rubin) 46d955d196043cc297834baeebce31ff778dff80 Remove mapLinks in favor of entry inlined structs with iterator type erasure (Jeremy Rubin) Pull request description: Currently we have a peculiar data structure in the mempool called maplinks. Maplinks job is to track the in-pool children and parents of each transaction. This PR can be primarily understood and reviewed as a simple refactoring to remove this extra data structure, although it comes with a nice memory and performance improvement for free. Maplinks is particularly peculiar because removing it is not as simple as just moving it's inner structure to the owning CTxMempoolEntry. Because TxLinks (the class storing the setEntries for parents and children) store txiters to each entry in the mempool corresponding to the parent or child, it means that the TxLinks type is "aware" of the boost multiindex (mapTx) it's coming from, which is in turn, aware of the entry type stored in mapTx. Thus we used maplinks to store this entry associated data we in an entirely separate data structure just to avoid a circular type reference caused by storing a txiter inside a CTxMempoolEntry. It turns out, we can kill this circular reference by making use of iterator_to multiindex function and std::reference_wrapper. This allows us to get rid of the maplinks data structure and move the ownership of the parents/child sets to the entries themselves. The benefit of this good all around, for any of the reasons given below the change would be acceptable, and it doesn't make the code harder to reason about or worse in any respect (as far as I can tell, there's no tradeoff). ### Simpler ownership model No longer having to consistency check that mapLinks did have records for our CTxMempoolEntry, impossible to have a mapLinks entry outlive or incorrectly die before a CTxMempoolEntry. ### Memory Usage We get rid of a O(Transactions) sized map in the mempool, which is a long lived data structure. ### Performance If you have a CTxMemPoolEntry, you immediately know the address of it's children/parents, rather than having to do a O(log(Transactions)) lookup via maplinks (which we do very often). We do it in *so many* places that a true benchmark has to look at a full running node, but it is easy enough to show an improvement in this case. The ComplexMemPool shows a good coherence check that we see the expected result of it being 12.5% faster / 1.14x faster. ``` Before: # Benchmark, evals, iterations, total, min, max, median ComplexMemPool, 5, 1, 1.40462, 0.277222, 0.285339, 0.279793 After: # Benchmark, evals, iterations, total, min, max, median ComplexMemPool, 5, 1, 1.22586, 0.243831, 0.247076, 0.244596 ``` The ComplexMemPool benchmark only checks doing addUnchecked and TrimToSize for 800 transactions. While this bench does a good job of hammering the relevant types of function, it doesn't test everything. Subbing in 5000 transactions shows a that the advantage isn't completely wiped out by other asymptotic factors (this isn't the only bottleneck in growing the mempool), but it's only a bit proportionally slower (10.8%, 1.12x), which adds evidence that this will be a good change for performance minded users. ``` # Benchmark, evals, iterations, total, min, max, median ComplexMemPool, 5, 1, 59.1321, 11.5919, 12.235, 11.7068 # Benchmark, evals, iterations, total, min, max, median ComplexMemPool, 5, 1, 52.1307, 10.2641, 10.5206, 10.4306 ``` I don't think it's possible to come up with an example of where a maplinks based design would have better performance, but it's something for reviewers to consider. # Discussion ## Why maplinks in the first place? I spoke with the author of mapLinks (sdaftuar) a while back, and my recollection from our conversation was that it was implemented because he did not know how to resolve the circular dependency at the time, and there was no other reason for making it a separate map. ## Is iterator_to weird? iterator_to is expressly for this purpose, see https://www.boost.org/doc/libs/1_51_0/libs/multi_index/doc/tutorial/indices.html#iterator_to > iterator_to provides a way to retrieve an iterator to an element from a pointer to the element, thus making iterators and pointers interchangeable for the purposes of element pointing (not so for traversal) in many situations. This notwithstanding, it is not the aim of iterator_to to promote the usage of pointers as substitutes for real iterators: the latter are specifically designed for handling the elements of a container, and not only benefit from the iterator orientation of container interfaces, but are also capable of exposing many more programming bugs than raw pointers, both at compile and run time. iterator_to is thus meant to be used in scenarios where access via iterators is not suitable or desireable: > > - Interoperability with preexisting APIs based on pointers or references. > - Publication of pointer-based interfaces (for instance, when designing a C-compatible library). > - The exposure of pointers in place of iterators can act as a type erasure barrier effectively decoupling the user of the code from the implementation detail of which particular container is being used. Similar techniques, like the famous Pimpl idiom, are used in large projects to reduce dependencies and build times. > - Self-referencing contexts where an element acts upon its owner container and no iterator to itself is available. In other words, iterator_to is the perfect tool for the job by the last reason given. Under the hood it should just be a simple pointer cast and have no major runtime overhead (depending on if the function call is inlined). Edit by laanwj: removed at sign from the description ACKs for top commit: jonatack: re-ACK 296be8f per `git range-diff ab338a19 3ba1665 296be8f`, sanity check gcc 10.2 debug build is clean. hebasto: re-ACK 296be8f58e02b39a58f017c52294aceed22c3ffd, only rebased since my [previous](https://github.com/bitcoin/bitcoin/pull/19478#pullrequestreview-482400727) review (verified with `git range-diff`). Tree-SHA512: f5c30a4936fcde6ae32a02823c303b3568a747c2681d11f87df88a149f984a6d3b4c81f391859afbeb68864ef7f6a3d8779f74a58e3de701b3d51f78e498682e
2020-09-07Merge #19556: Remove mempool globalMarcoFalke
fafb381af8279b2d2ca768df0bf68d7eb036a2f9 Remove mempool global (MarcoFalke) fa0359c5b30730744aa8a7cd9ffab79ded91041f Remove mempool global from p2p (MarcoFalke) eeee1104d78eb59a582ee1709ff4ac2c33ee1190 Remove mempool global from init (MarcoFalke) Pull request description: This refactor unlocks some nice potential features, such as, but not limited to: * Removing the fee estimates global (would avoid slightly fragile workarounds such as #18766) * Making the mempool optional for a "blocksonly" operation mode Even absent those features, the new code without the global should be easier to maintain, read and write tests for. ACKs for top commit: jnewbery: utACK fafb381af8279b2d2ca768df0bf68d7eb036a2f9 hebasto: ACK fafb381af8279b2d2ca768df0bf68d7eb036a2f9, I have reviewed the code and it looks OK, I agree it can be merged. darosior: ACK fafb381af8279b2d2ca768df0bf68d7eb036a2f9 Tree-SHA512: a2e696dc377e2e81eaf9c389e6d13dde4a48d81f3538df88f4da502d3012dd61078495140ab5a5854f360a06249fe0e1f6a094c4e006d8b5cc2552a946becf26
2020-09-07Merge #19738: wallet: Avoid multiple BerkeleyBatch in DelAddressBookSamuel Dobson
abac4367607d8d2b628e4db6a9663c960bacdacc wallet: Avoid multiple BerkeleyBatch in DelAddressBook (João Barbosa) Pull request description: ACKs for top commit: achow101: ACK abac4367607d8d2b628e4db6a9663c960bacdacc jonatack: ACK abac4367607d8d2b628e4db6a9663c960bacdacc meshcollider: re-utACK abac4367607d8d2b628e4db6a9663c960bacdacc Tree-SHA512: 92309fb74c48694160807326c0fe9793044a75cd77ed19400cceab54a7eefeb54ffc9334535e6021b3af7b9a364dbbeda3a9173540fff8144dfd437e96d76b5c
2020-09-06Unroll Keccak-f implementationPieter Wuille
2020-09-06Add SHA3 benchmarkPieter Wuille
2020-09-06Implement keccak-f[1600] and SHA3-256Pieter Wuille
2020-09-07Merge #19619: Remove wallet.dat path handling from wallet.cpp, rpcwallet.cppSamuel Dobson
7bf6dfbb484adfda3b8df26ee3e2ebda239dd263 wallet: Remove path checking code from bitcoin-wallet tool (Russell Yanofsky) 77d5bb72b8722ec7a6c7c33479a532cbd5870ba4 wallet: Remove path checking code from createwallet RPC (Russell Yanofsky) a987438e9d9cad0b5530e218a447928485f3fd93 wallet: Remove path checking code from loadwallet RPC (Russell Yanofsky) 8b5e7297c02f3100a9cb27bfe206e3fc617ec173 refactor: Pass wallet database into CWallet::Create (Russell Yanofsky) 3c815cfe54087fd139169161d2fd175e99840e6a wallet: Remove Verify and IsLoaded methods (Russell Yanofsky) 0d94e6062547f288a75921d2433458a44a5f2297 refactor: Use DatabaseStatus and DatabaseOptions types (Russell Yanofsky) b5b414151af32e5a07b5757b64482d77519d77c0 wallet: Add MakeDatabase function (Russell Yanofsky) 288b4ffb6b291f0466d513ff3c40af6758ca7c88 Remove WalletLocation class (Russell Yanofsky) Pull request description: Get rid of file path handling in wallet application code and move it down to database layer. There is no change in behavior except for some changed error messages. Motivation for this change is to make code more understandable, but also to prepare for adding SQLite support in #19077 so SQLite implementation can be contained at the database layer and wallet loading code does not need to become more complicated. ACKs for top commit: achow101: ACK 7bf6dfbb484adfda3b8df26ee3e2ebda239dd263 meshcollider: Code re-review and functional test run ACK 7bf6dfbb484adfda3b8df26ee3e2ebda239dd263 Tree-SHA512: 23ad18324c9e8947f0cf88a3734c2e9fb25536b2cb4d552cf5d1a4ade320fbffb73bb2d1b3a99585c11630aa7092e0fcfc2dd4fe65b91e3a54161433a5cd13cb
2020-09-06wallet: Avoid multiple BerkeleyBatch in DelAddressBookJoão Barbosa
2020-09-06refactor: remove unused header <arpa/inet.h> in protocol.cppSebastian Falbesoner
2020-09-05Remove mempool globalMarcoFalke
2020-09-05Remove mempool global from p2pMarcoFalke
2020-09-05Remove mempool global from initMarcoFalke
Can be reviewed with the git diff options --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space --ignore-all-space
2020-09-05Merge #19848: Remove mempool global from interfacesMarcoFalke
fa9ee52556f493e4a896e2570ca1a3102d777d9a doc: Add doxygen comment to IsRBFOptIn (MarcoFalke) faef4fc9b4990e563022b6ab595cb02c4060c216 Remove mempool global from interfaces (MarcoFalke) fa831684e54783f6b40533ca218eb7636bdae667 refactor: Add IsRBFOptInEmptyMempool (MarcoFalke) Pull request description: The chain interface has an `m_node` member, which has a pointer to the mempool global. Use the pointer instead of the global to prepare the removal of the mempool global. See #19556 ACKs for top commit: jnewbery: utACK fa9ee52556 darosior: ACK fa9ee52 hebasto: re-ACK fa9ee52556f493e4a896e2570ca1a3102d777d9a, since my [previous](https://github.com/bitcoin/bitcoin/pull/19848#pullrequestreview-482403942) review: Tree-SHA512: 11b4c1446f0860a743fdaa67f95c52bf0262d0a4f888be0eaf07ee497448965d32be414111bf016bd568f2989cde923430e3a3889e224057b73c499f06de7199
2020-09-05Merge #19728: Increase the ip address relay branching factor for unreachable ↵Wladimir J. van der Laan
networks 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e Increase the ip address relay branching factor for unreachable networks (Pieter Wuille) Pull request description: Onion addresses propagate very badly among the IPv4/IPv6 network, resulting in difficulty for those to find each other. The branching factor 1 is probably so low that propagations die out before they reach another onion peer. Increase it to 1.5 on average. ACKs for top commit: practicalswift: ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e -- patch looks correct naumenkogs: ACK 86d4cf4 jonatack: ACK 86d4cf42d97abf4c436d1eabf29e2ed150f69c1e. Code review, built and running with some sanity check logging. `RelayAddress()` is called by `ProcessMessage() ADDR` msg handling, from within the loop while processing each new address to relay it to a limited number of other nodes. According to git blame, the line setting `nRelayNodes` hasn't been touched since 2016 in e736772c56a *Move network-msg-processing code out of main to its own file*, which moved the line but otherwise did not change it. Running a mixed clearnet/onion node with this patch and the logging below, I'm only seeing values of `fReachable 1, nRelayNodes 2`. IIUC, I need to use the settings in `init.cpp` that call `SetReachable(*, false)`. *Edit:* with `onlynet=onion` am now seeing entries of `fReachable 0` with `nRelayNodes` values of 1 and 2. vasild: ACK 86d4cf42d Tree-SHA512: 22391e16d60bcfdec9a9336728da39d68a24a183b3d1b0e8fbc038d265ca6ddf71d16db018f3678745fd9f3e9281049e42197fa0a29124833c50a9170ed6f793
2020-09-05Merge #19852: refactor: Avoid duplicate map lookup in ScriptToAsmStrMarcoFalke
ac2ff4fb1e06270cf17727f90599c9f3a55ddd5a refactor: Avoid duplicate map lookup in ScriptToAsmStr (João Barbosa) Pull request description: Simple change that avoids a duplicate (unnecessary) `mapSigHashTypes` lookup. ACKs for top commit: laanwj: re-ACK ac2ff4fb1e06270cf17727f90599c9f3a55ddd5a Tree-SHA512: 7e7f5af51c1acd7a42af273e5ee5e2faddd250ba8b8f63ccb3172d95f153ae391b2816b79564b856571af52dc2a767b5736a5d10ffb5cd2c540cd9832bf86419
2020-09-05doc: Add doxygen comment to IsRBFOptInMarcoFalke
2020-09-05Remove mempool global from interfacesMarcoFalke
2020-09-05refactor: Add IsRBFOptInEmptyMempoolMarcoFalke
Co-authored-by: John Newbery <jonnynewbs@gmail.com>
2020-09-04rawtransaction: fix argument in combinerawtransaction help messageMatthew Zipkin
2020-09-04Get rid of unused functions CTxMemPool::GetMemPoolChildren, ↵Jeremy Rubin
CTxMemPool::GetMemPoolParents
2020-09-04Remove mapLinks in favor of entry inlined structs with iterator type erasureJeremy Rubin
2020-09-04Merge #19405: rpc, cli: add network in/out connections to `getnetworkinfo` ↵Wladimir J. van der Laan
and `-getinfo` 581b343d5bf517510ab0236583ca96628751177d Add in/out connections to cli -getinfo (Jon Atack) d9cc13e88d096c1a171159c01cbb96444f7f8d7f UNIX_EPOCH_TIME fixup in rpc getnettotals (Jon Atack) 1ab49b81cf32b6ef9e312a0a8ac45c68a3262f0d Add in/out connections to rpc getnetworkinfo (Jon Atack) Pull request description: This is basic info that is present in the GUI that I've been wishing to have exposed via the RPC and CLI without needing a bash workaround or script. For human users it would also be useful to have it in `-getinfo`. `bitcoin-cli getnetworkinfo` ``` "connections": 15, "connections_in": 6, "connections_out": 9, ``` `bitcoin-cli -getinfo` ``` "connections": { "in": 6, "out": 9, "total": 15 }, ``` Update the tests, RPC help, and release notes for the changes. Also fixup the `getnettotals` timemillis help while touching `rpc/net.cpp`. ----- Reviewers can manually test this PR by [building from source](https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests), launching bitcoind, and then running `bitcoin-cli -getinfo`, `bitcoin-cli getnetworkinfo`, `bitcoin-cli help getnetworkinfo`, and `bitcoin-cli help getnettotals` (for the UNIX epoch time change). ACKs for top commit: eriknylund: > tACK [581b343](https://github.com/bitcoin/bitcoin/commit/581b343d5bf517510ab0236583ca96628751177d) on master at [a0a422c](https://github.com/bitcoin/bitcoin/commit/a0a422c34cfd6514d0cc445bd784d3ee1a2d1749), ran unit & functional tests and and confirmed changes on an existing datadir ✌️ benthecarman: tACK `581b343` willcl-ark: tACK for 581b343d5bf517510ab0236583ca96628751177d, this time rebased onto master at 862fde88be706adb20a211178253636442c3ae00. shesek: tACK `581b343`. This provides what I needed, thanks! n-thumann: tACK 581b343 on master at a0a422c, ran unit & functional tests and and confirmed changes on an existing datadir ✌️ Tree-SHA512: 08dd3ac8fefae401bd8253ff3ac027603c528eeccba53cedcb127771316173a7052fce44af8fa33ac98ebc4cf2a2b11cdefd949995d55e9b9a5942b876d00dc5
2020-09-04Merge #19854: Avoid locking CTxMemPool::cs recursively in simple casesWladimir J. van der Laan
020f0519ec66d9626255b938e1c6c3f7f9aa4017 refactor: CTxMemPool::IsUnbroadcastTx() requires CTxMemPool::cs lock (Hennadii Stepanov) 7c4bd0387a01a0c3e2938d530dba3c882e4d8f2b refactor: CTxMemPool::GetTotalTxSize() requires CTxMemPool::cs lock (Hennadii Stepanov) fa5fcb032b6ed04c49ee465235288b8059fa805e refactor: CTxMemPool::ClearPrioritisation() requires CTxMemPool::cs lock (Hennadii Stepanov) 7140b31b90cbd84d75eedb3e395d0d55f83b5b95 refactor: CTxMemPool::ApplyDelta() requires CTxMemPool::cs lock (Hennadii Stepanov) 66e47e5e506043fbb9b4e487b44bf992985709c9 refactor: CTxMemPool::UpdateChild() requires CTxMemPool::cs lock (Hennadii Stepanov) 939807768acd508932f2efabee660d56324a73df refactor: CTxMemPool::UpdateParent() requires CTxMemPool::cs lock (Hennadii Stepanov) Pull request description: This is another step to transit `CTxMemPool::cs` from `RecursiveMutex` to `Mutex`. Split out from #19306. Only trivial thread safety annotations and lock assertions added. No new locks. No behavior change. Refactoring `const uint256` to `const uint256&` was [requested](https://github.com/bitcoin/bitcoin/pull/19647#discussion_r468471022) by **promag**. Please note that now, since #19668 has been merged, it is safe to apply `AssertLockHeld()` macros as they do not swallow compile time Thread Safety Analysis warnings. ACKs for top commit: promag: Core review ACK 020f0519ec66d9626255b938e1c6c3f7f9aa4017. jnewbery: Code review ACK 020f0519ec66d9626255b938e1c6c3f7f9aa4017 vasild: ACK 020f0519e Tree-SHA512: a31e389142d5a19b25fef0aaf1072a337278564528b5cc9209df88ae548a31440e1b8dd9bae0169fd7aa59ea06e22fe5e0413955386512b83ef1f3e7d941e890
2020-09-04refactor: Avoid duplicate map lookup in ScriptToAsmStrJoão Barbosa
2020-09-03wallet: Remove path checking code from bitcoin-wallet toolRussell Yanofsky
This commit does not change behavior except for error messages which now include more complete information.
2020-09-03Merge #19754: wallet, gui: Reload previously loaded wallets on startupJonas Schnelli
f1ee37319a7a211e5fb325406d62db5b61dbd30e wallet: Reload previously loaded wallets on GUI startup (Andrew Chow) Pull request description: Enable the GUI to also use the load_on_startup feature. Wallets loaded in the GUI always have load_on_startup=true. When they are unloaded from the GUI, load_on_startup=false. To facilitate this change, UpdateWalletSetting is moved into the wallet module and called from within LoadWallet, RemoveWallet, and Createwallet. This change does not actually touch the GUI code but rather the wallet functions that are shared between the GUI and RPC. ACKs for top commit: jonasschnelli: Tested ACK f1ee37319a7a211e5fb325406d62db5b61dbd30e - works as expected. Wallets loaded via bitcoin-cli (in `-server` mode) or through the RPC console won't be loaded on startup but wallets loaded via the GUI menu will. kristapsk: ACK f1ee37319a7a211e5fb325406d62db5b61dbd30e, I have tested the code. Tree-SHA512: f5b44aa763cf761d919015c5fbc0600b72434aa71e3b57007fd7530a29c3da1a9a0c98c4f22cb6cdffba61150a31170056a7d4737625e7b76f6958f3d584da8c
2020-09-03wallet: Remove path checking code from createwallet RPCRussell Yanofsky
This commit does not change behavior except for error messages which now include more complete information.
2020-09-03wallet: Remove path checking code from loadwallet RPCRussell Yanofsky
This commit does not change behavior except for error messages which now include more complete information.
2020-09-03refactor: Pass wallet database into CWallet::CreateRussell Yanofsky
No changes in behavior
2020-09-03wallet: Remove Verify and IsLoaded methodsRussell Yanofsky
Checks are now consolidated in MakeBerkeleyDatabase function instead of happening in higher level code. This commit does not change behavior except for error messages which now include more complete information.
2020-09-03refactor: Use DatabaseStatus and DatabaseOptions typesRussell Yanofsky
No changes in behavior. Just replaces arguments and return types
2020-09-03wallet: Add MakeDatabase functionRussell Yanofsky
New function is not currently called but will be called in upcoming commits. It moves database path checking, and existence checking, and already-loaded checking, and verification into a single function so this logic does not need to be repeated all over higher level wallet code, and so higher level code does not need to change when SQLite support is added in https://github.com/bitcoin/bitcoin/pull/19077. This also lets higher level wallet code make fewer assumptions about the contents of wallet directories. This commit just adds the new function and does not change behavior in any way.
2020-09-03Remove WalletLocation classRussell Yanofsky
This removes a source of complexity and indirection that makes it harder to understand path checking code. Path checks will be simplified in upcoming commits. There is no change in behavior in this commit other than a slightly more descriptive error message in `loadwallet` if the default "" wallet can't be found. (The error message is improved more in upcoming commit "wallet: Remove path checking code from loadwallet RPC".)
2020-09-03Merge #19818: p2p: change `CInv::type` from `int` to `uint32_t`, fix UBSan ↵Wladimir J. van der Laan
warning 7984c39be11ca04460883365e1ae2a496aaa6c0e test framework: serialize/deserialize inv type as unsigned int (Jon Atack) 407175e0c2bc797599ebd9c0a1f2ec89ad7af136 p2p: change CInv::type from int to uint32_t (Jon Atack) Pull request description: Fixes UBSan implicit-integer-sign-change issue per https://github.com/bitcoin/bitcoin/pull/19610#issuecomment-680686460. Credit to Crypt-iQ for finding and reporting the issue and to vasild for the original review suggestion in https://github.com/bitcoin/bitcoin/pull/19590#pullrequestreview-455788826. Closes #19678. ACKs for top commit: laanwj: ACK 7984c39be11ca04460883365e1ae2a496aaa6c0e MarcoFalke: ACK 7984c39be11ca04460883365e1ae2a496aaa6c0e 🌻 vasild: ACK 7984c39be Tree-SHA512: 59f3a75f40ce066ca6f0bb1927197254238302b4073af1574bdbfe6ed580876437be804be4e47d51467d604f0d9e3a5875159f7f2edbb2351fdb2bb9465100b5