d1b622b tests: Add check for test suite name uniqueness in lint-tests.sh (practicalswift)
dc8067b tests: Add note about uniqueness requirement for test suite names (practicalswift)
3ebfb2d tests: Avoid test suite name collision in wallet crypto_tests (MarcoFalke)
Pull request description:
* Add documentation: Add note about test suite name uniqueness requirement in developer notes
* Add regression test: Update `lint-tests.sh` to make it check also for test suite name uniqueness
Context: #12894 (`tests: Avoid test suite name collision in wallet crypto_tests`)
Tree-SHA512: 3c8502db069ef3d753f534976a86a997b12bac539e808a7285193bf81c9dd8c1b06821c3dd1bdf870ab87722b02c8aa9574c62ace70c2a1b8091785cb8c9aace
f63bc5e wallet: Initialize m_last_block_processed to nullptr. Initialize fields where defined. (practicalswift)
Pull request description:
Initialize `m_last_block_processed` to `nullptr`.
`m_last_block_processed` was introduced in 5ee3172636.
Tree-SHA512: 6e4a807e5b02115cbd80460761056f2eb22043203212d88dd0cd44c28dc0abce30ab29b078ca2c612232e76af4886f4fdbf2b0ff75e2df19b4d1a801b236cc13
db983beba6 tests: Add lint-tests.sh which checks the test suite naming convention (practicalswift)
5fd864fe8a tests: Rename test suits not following the test suite naming convention (practicalswift)
7b4a296a71 tests: Add note about test suite naming convention (practicalswift)
Pull request description:
Changes:
* Add note about test suite naming convention
* Fix exceptions
* Add regression test
Rationale:
* Consistent naming of test suites makes programmatic test running of specific tests/subsets of tests easier
* Explicit is better than implicit
Before this commit:
```
$ contrib/devtools/lint-tests.sh
The test suite in file src/test/foo_tests.cpp should be named
"foo_tests". Please make sure the following test suites follow
that convention:
src/test/blockchain_tests.cpp:BOOST_FIXTURE_TEST_SUITE(blockchain_difficulty_tests, BasicTestingSetup)
src/test/prevector_tests.cpp:BOOST_FIXTURE_TEST_SUITE(PrevectorTests, TestingSetup)
src/wallet/test/coinselector_tests.cpp:BOOST_FIXTURE_TEST_SUITE(coin_selection_tests, WalletTestingSetup)
src/wallet/test/crypto_tests.cpp:BOOST_FIXTURE_TEST_SUITE(wallet_crypto, BasicTestingSetup)
$
```
After this commit:
```
$ contrib/devtools/lint-tests.sh
$
```
Tree-SHA512: 7258ab9a6b9b8fc1939efadc619e2f2f02cfce8034c7f2e5dc5ecc769aa12e17f6fb8e363817feaf15c026c5b958b2574525b8d2d3f6be69658679bf8ceea9e9
4d9b4256d8 Fix typos (Dimitris Apostolou)
Pull request description:
Unfortunately I messed up my repo while trying to squash #12593 so I created a PR with just the correct fixes.
Tree-SHA512: 295d77b51bd2a9381f1802c263de7ffb2edd670d9647391e32f9a414705b3c8b483bb0e469a9b85ab6a70919ea13397fa8dfda2aea7a398b64b187f178fe6a06
Signed-off-by: pasta <pasta@dashboost.org>
e5468a19d1 Remove unreachable help conditions (lutangar)
Pull request description:
These conditions on `request.fHelp`, which appears in the body of the following functions are never reached:
* `walletpassphrase`
* `walletpassphrasechange`
* `encryptwallet`
```
...
if (request.fHelp || request.params.size() != 0) {
throw std::runtime_error("");
}
...
if (request.fHelp)
return true;
...
```
The first condition would throw if `request.fHelp` evaluates to `true`.
Tree-SHA512: 1aa41ed233c6bebae27151ab5cc67144d2a408335a3acef3c103e144d6343685f360b1146e14bc8dc1d53d00fcfc6ff1ab6a0eeb0805191172a23b306ab50b79
42343c748 Split up and sanitize CAccountingEntry serialization (Pieter Wuille)
029ecac1b Split up and sanitize CWalletTx serialization (Pieter Wuille)
Pull request description:
This is a small subset of changes taken from #10785, fixing a few of the craziest constness violations in the serialization code.
`CWalletTx` currently serializes some of its fields by embedding them in a key-value `mapValue`, which is modified (and then fixed up) even from the `Serialize` method (for which `mapValue` is const). `CAccountingEntry` goes even further in that it stores such a map by appending it into `strComment` after a null char, which is again later fixed up again.
Fix this by splitting the serialization and deserialization code, and making the serialization act on a copy of `mapValue` / `strComment`.
Tree-SHA512: 487e04996dea6aba5b9b8bdaf2c4e680808f111a15afc557b8d078e14b01e4f40f8ef27588869be62f9a87052117c17e0a0c26c59150f83472a9076936af035e
2736c9e05 Avoid unintentional unsigned integer wraparounds in tests (practicalswift)
Pull request description:
Avoid unintentional unsigned integer wraparounds in tests.
This is a subset of #11535 as suggested by @MarcoFalke :-)
Tree-SHA512: 4f4ee8a08870101a3f7451aefa77ae06aaf44e3c3b2f7555faa2b8a8503f97f34e34dffcf65154278f15767dc9823955f52d1aa7b39930b390e57cdf2b65e0f3
992f56876 depends: Only use D_DARWIN_C_SOURCE when building miniupnpc on darwin (fanquake)
Pull request description:
Only use D_DARWIN_C_SOURCE when building on darwin, so we don't inadvertently introduce issues elsewhere.
cc @theuni
Tree-SHA512: e49a8456ba2b9925c06e62c73e139152b6d63cc5a4cee66944e41c863ca9103e98ac81a5718eceb3d0885a677fc53ece34062b02c304a05c3280e094965e856a
* Trivial Dashification
* Tweak getnetworkinfo and dumpwallet help text
We don't have RBF and Segwit
* CopyrightHolders should also check for missing "Dash Core" copyright
f40df29d96 Fix Windows build errors introduced in #10498 (practicalswift)
Pull request description:
Fix Windows build errors introduced in #10498Fixes#12386
cc @ken2812221, @Sjors, @MarcoFalke and @floreslorca
Tree-SHA512: a807521fbd4015971e646fa4bab5495a3aaa97337e7b7d80b9161f33778e84ad6725cf4fbd5a35b50bf3a2bd97747cd7a630b51493ff516c2e1ad74acce148be
e710387ca9 test: Fix bip68 sequence test to reflect updated rpc error message (Ben Woosley)
Pull request description:
The message changed in #12356, but this test is in the extended test suite, so it didn't fail on CI.
Tree-SHA512: ce800e2636ab7bbba7876aa533e1684e75c37a7d2a119f9c4602fd89ac9215e2e28710a1f27feb642b6737ed858da049ebc52fdd476ff4637e3ac3bb1d8399ce
fc1bfcf Update mac_alias to 2.0.7 (Douglas Roark)
deee216 Delete mac_alias patch (Douglas Roark)
Pull request description:
The patch Bitcoin Core has been maintaining for mac_alias was pulled by the mac_alias maintainer in commit 4f31cb084c1c6a8626128b0b00842020b6db9037. Delete the patch and remove the patch from the depends system.
Note that this PR won't be complete until a new version of mac_alias containing the patch has been released, and the depends system is updated to reflect the new version.
Tree-SHA512: e13f1b45c0a56e95645b1aff77036c8a24c29c3f18ea0d386fba8d6d0f5fd07c434afc09dcd644d46ca096d6a7a0d5097f1eca3be5b5a5475eb3d54407044fd9
55f89da1a Don't test against the mempool min fee information in mempool_limit.py (Ben Woosley)
Pull request description:
Because the right-hand side of this comparison can be influenced
externally, e.g. via the -maxmempool argument, the existing mempool state,
host memory usage, etc.
Called out by @MarcoFalke here: https://github.com/bitcoin/bitcoin/pull/12356#discussion_r170094948
Tree-SHA512: 1644cb8046a6953fb93423a5e51af4f5c7d00a35f10389fddd6a823dae6f31ab367b53af70b3b69161adb9c48f57cf4772db7f4610fd7aadd9c0e9b3da17e9f8
bb00c95 Consistently use FormatStateMessage in RPC error output (Ben Woosley)
8b8a1c4 Add test for 'mempool min fee not met' rpc error (Ben Woosley)
c04e0f6 Fix 'mempool min fee not met' debug output (Ben Woosley)
Pull request description:
Output the value that is tested, rather than the unmodified fee value.
Prompted by looking into: #11955
Tree-SHA512: fc0bad47d4af375d208f657a6ccbad6ef7f4e2989ae2ce1171226c22fa92847494a2c55cca687bd5a1548663ed3313569bcc31c00d53c0c193a1b865dd8a7657
6ef86c9 Do not un-mark fInMempool on wallet txn if ATMP fails. (Matt Corallo)
Pull request description:
Irrespective of the failure reason, un-marking fInMempool
out-of-order is incorrect - it should be unmarked when
TransactionRemovedFromMempool fires.
Clean up of #11839, which I think was the wrong fix.
Tree-SHA512: 580731297eeac4c4c99ec695e15b09febf62249237bc367fcd1830fc811d3166f9336e7aba7f2f6f8601960984ae22cebed781200db0f04e7cd2008db1a83f64
2f960b5 [wallet] Indent only change of CWallet::AvailableCoins (João Barbosa)
1beea7a [wallet] Make CWallet::ListCoins atomic (João Barbosa)
Pull request description:
Fix a potencial race in `CWallet::ListCoins`.
Replaces `cs_main` and `cs_wallet` locks by assertions in `CWallet::AvailableCoins`.
Tree-SHA512: 09109f44a08b4b53f7605d950ab506d3f748490ab9aed474aa200e93f7b0b9f96f9bf60abe1c5f658240fd13d9e3267c0dd43fd3c1695d82384198ce1da8109f
db1cbcc [RPC] Remove deprecated addmultisigaddress return format (John Newbery)
cb28a0b [RPC] Remove deprecated createmultisig object (John Newbery)
ed45c82 [tests] Remove test for deprecated createmultsig option (John Newbery)
d066a1c [rpc] Remove deprecated getmininginfo RPC option (John Newbery)
c6f09c2 [rpc] remove deprecated estimatefee RPC (John Newbery)
a8e437a [tests] Remove estimatefee from rpc_deprecated.py test (John Newbery)
a5623b1 [tests] Remove tests for deprecated estimatefee RPC (John Newbery)
d119f2e [tests] Fix style warnings in feature_fee_estimation.py (John Newbery)
Pull request description:
There were some RPC/RPC options deprecated in v0.16. Those can now be removed from master since v0.16 has been branched.
- `estimatefee` RPC has been removed. The `feature_fee_estimation.py` test has been updated to remove the RPC, but doesn't yet have good coverage of the replacement RPC `estimatesmartfee`. Improving the test coverage should be done in a new PR. (#11031)
- the `errors` field returned by `getmininginfo` has been deprecated and replaced by a `warning` field. (#10858)
- providing addresses as inputs to `createmultisig` has been deprecated. Users should use `addmultisigaddress` instead (#11415)
- The return format from `addmultisigaddress` has changed (#11415)
`getwitnessaddress` was also deprecated in v0.16 and can be removed, but many tests are using that RPC, so it's a larger job to remove. It should be removed in a separate PR (possibly after #11739 and #11398 have been merged and the segwit test code tidied up)
Tree-SHA512: 8ffaa5f6094131339b9e9e468e8b141de4b144697d2271efa2992b80b12eb97849ade3da8df5c1c9400ed4c04e6a029926550a3e5846d2029b644f9e84ac7124
90ba2df11 Fix missing cs_main lock for GuessVerificationProgress() (Jonas Schnelli)
Pull request description:
`GuessVerificationProgress()` needs `cs_main` due to accessing the `pindex->nChainTx`.
This adds a `AssertLockHeld` in `GuessVerificationProgress()` and adds the missing locks in...
* `LoadChainTip()`
* `ScanForWalletTransactions()` (got missed in #11281)
* GUI, `ClientModel::getVerificationProgress()` <--- **this may have GUI performance impacts**, but could be relaxed later with a cache or something more efficient.
Tree-SHA512: 13302946571422375f32af8e396b9d2c1180f2693ea363aeba9e98c8266ddec64fe7862bfdcbb5a93a4b12165a61eec1e51e4e7d7a8515fa50879095dc163412
9ad6746ccd Use static_cast instead of C-style casts for non-fundamental types (practicalswift)
Pull request description:
A C-style cast is equivalent to try casting in the following order:
1. `const_cast(...)`
2. `static_cast(...)`
3. `const_cast(static_cast(...))`
4. `reinterpret_cast(...)`
5. `const_cast(reinterpret_cast(...))`
By using `static_cast<T>(...)` explicitly we avoid the possibility of an unintentional and dangerous `reinterpret_cast`. Furthermore `static_cast<T>(...)` allows for easier grepping of casts.
For a more thorough discussion, see ["ES.49: If you must use a cast, use a named cast"](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es49-if-you-must-use-a-cast-use-a-named-cast) in the C++ Core Guidelines (Stroustrup & Sutter).
Tree-SHA512: bd6349b7ea157da93a47b8cf238932af5dff84731374ccfd69b9f732fabdad1f9b1cdfca67497040f14eaa85346391404f4c0495e22c467f26ca883cd2de4d3c
7444149 Document method for reviewers to verify chainTxData (John Newbery)
Pull request description:
This commit adds the final block hash of the window to getchaintxstats
and documents how reviewers can verify changes to chainTxData.
Tree-SHA512: d16abb5f47d058e52660f4d495f1e453205b1b83716d7c810ff62a70338db721386c1808ec1fc8468f514e4d80cc58e3c96eeb3184cbbcb1d07830fa5e53f342
7f968ae107 doc: Explain how to update chainTxData in release process (Wladimir J. van der Laan)
Pull request description:
Adds a short explanation how to update chainTxData to the release process. Mention where to get the data, and link to an example.
Tree-SHA512: 66b0eb12a25afb7b1da0788c8f9f9701d566cb7ce55899402440a57bef0e177b55e926442d3c8aa482299abe7c48ab94735d501c37f0a11fefb86e2fc2e33d9b
5bdbbdc Refactor HaveKeys to early return on false result (João Barbosa)
Pull request description:
This consists in a trivial change where the return type of `HaveKeys()` is now `bool` meaning that it returns whether all keys are in the keystore, and early returns when one isn't.
Tree-SHA512: 03e35ea8486404b84884b49f6905c9f4fc161a3eeef080b06482d77985d5242a2bdd57a34b8d16abe19ee8c6cfa3e6fbcb935c73197d53f4cd468a2c7c0b889b
bdb3231 Implements a virtual destructor on the BaseRequestHandler class. (251)
Pull request description:
Granted that there is no undefined behavior in the current implementation, this PR implements a virtual destructor on the BaseRequestHandler class to protect against undefined behavior in the event that an object of a potential future derived BaseRequestHandler class with a destructor is destroyed through a pointer to this base class.
This PR also fixes "_warning: delete called on 'BaseRequestHandler' that is abstract but has non-virtual destructor [-Wdelete-non-virtual-dtor]_" warnings in environments where the project is built with the `-Wsystem-headers` flag; or environments where the `-Wdelete-non-virtual-dtor` diagnostics flag fires from system headers.
Tree-SHA512: 3c3b0797a8dbce8d8c5b244709e8bca41c4e28d5ba554a974bf7fc9128413e1098c457a00e51b21154ce6c11ce5da3071626e71d593b2550d0020bc589406eed
c409b1adac [rpc] Reduce scope of cs_main and cs_wallet locks in listtransactions (João Barbosa)
Pull request description:
Trivial change, no behaviour change.
Benchmark done as follow:
- run with `-regtest`
- wallet with 5000 transactions
- measured the time spent with the lock and the total time
- times are an average of 100 `listtransactions --count=...` calls
| `--count` | lock (ms) | total (ms) | saving |
|--:|--:|--:|--:|
| 10 | 0.2230 | 0.2510 | 11% |
| 100 | 2.5150 | 2.8690 | 12% |
| 1000 | 20.0320 | 23.3490 | 14% |
| 10000 | 105.2070 | 125.5310 | 16% |
Tree-SHA512: ebedfeeb4c8ad75c89128e53cae976a82967dbb5ffd129da0f7204ccf9c3c15070b3d509f3767bebd745512e410200cc546147c836e82409f95fc9b8d14fc3ed
b7b36decaf878a8c1dcfdb4a27196c730043474b fix uninitialized read when stringifying an addrLocal (Kaz Wesley)
8ebbef016928811756e46b9086067d1c826797a8 add test demonstrating addrLocal UB (Kaz Wesley)
Pull request description:
Reachable from either place where SetIP is used when all of:
- our best-guess addrLocal for a peer is IPv4
- the peer tells us it's reaching us at an IPv6 address
- NET logging is enabled
In that case, SetIP turns an IPv4 address into an IPv6 address without
setting the scopeId, which is subsequently read in GetSockAddr during
CNetAddr::ToStringIP and passed to getnameinfo. Fix by ensuring every
constructor initializes the scopeId field with something.
Tree-SHA512: 8f0159750995e08b985335ccf60a273ebd09003990bcf2c3838b550ed8dc2659552ac7611650e6dd8e29d786fe52ed57674f5880f2e18dc594a7a863134739e3
* Make internalId private to make sure it's set/accessed properly
* evo: Make internalId only accessible by the constructor
* Tweak constructors
Co-authored-by: xdustinface <xdustinfacex@gmail.com>
* Drop dead code in DoInvalidateBlock
* Let ActivateBestChain skip SyncWithValidationInterfaceQueue when called from IS or CL threads
* Use CL's own scheduler instead of a global one
* Revert "Let ActivateBestChain skip SyncWithValidationInterfaceQueue when called from IS or CL threads"
This reverts commit 1c9f6da50a.
b08af10fb299dc3fdcd1f022619fb112c72e5d8e disallow oversized CBlockHeaderAndShortTxIDs (Kaz Wesley)
6bed4b374daf26233e96fa7863d4324a5bfa99c2 fix a deserialization overflow edge case (Kaz Wesley)
051faf7e9d4e32142f95f7adb31d2f53f656cb66 add a test demonstrating an overflow in a deserialization edge case (Kaz Wesley)
Pull request description:
A specially-constructed BlockTransactionsRequest can cause `offset` to wrap in deserialization. In the current code, there is not any way this could be dangerous; but disallowing it reduces the potential for future surprises.
Tree-SHA512: 1aaf7636e0801a905ed8807d0d1762132ac8b4421a600c35fb6d5e5033c6bfb587d8668cd9f48c7a08a2ae793a677b7649661e3ae248ab4f8499ab7b6ede483c
27c44ef9c61f64d941ab82ec232a68141a2fde90 rpcbind: Warn about exposing RPC to untrusted networks (Luke Dashjr)
d6a1287481428d982dc03be3a6d9aeef8398f468 CNetAddr: Add IsBindAny method to check for INADDR_ANY (Luke Dashjr)
3615003952ffbc814bdb53d9d0e45790f152bd2f net: Always default rpcbind to localhost, never "all interfaces" (Luke Dashjr)
Pull request description:
A disturbingly large number of listening nodes appear to be also exposing their RPC server to the public internet. To attempt to mitigate this:
* Only ever bind localhost by default, even if `rpcallowip` is specified. (A warning is given if `rpcallowip` is specified without `rpcbind`, since it doesn't really make sense to do.)
* Warn about exposing the RPC server to untrusted networks if the user explicitly binds to any INADDR_ANY address.
* Include a warning about untrusted networks in the `--help` documentation for `rpcbind`.
Tree-SHA512: 755bbca3db416a31393672eccf6675a5ee4d1eb1812cba73ebb4ff8c6b855ecc5df4c692566e9aa7b0f7d4dce6fedb9c0e9f3c265b9663aca36c4a6ba5efdbd4
83d53058ae240a5c396ac927972ad96d2a11e903 Switch nPrevNodeCount to vNodesSize. (Patrick Strateman)
Pull request description:
These both have the same value, but the variable naming is confusing.
Tree-SHA512: 4f645e89efdc69884ff4c8bbcf42e2b35d2733687c0fc6ab3f0797e0141fe23ef9cde8bb6ba422f47a88f554e55a099b1f0b3f47cb9fde12db3d46b9a0041bb0
66b3fc5437 Skip stale tip checking if outbound connections are off or if reindexing. (Gregory Maxwell)
Pull request description:
I got tired of the pointless stale tip notices in reindex and on nodes with connections disabled.
Tree-SHA512: eb07d9c5c787ae6dea02cdd1d67a48a36a30adc5ccc74d6f1c0c7364d404dc8848b35d2b8daf5283f7c8f36f1a3c463aacb190d70a22d1fe796a301bb1f03228
fa74d3d720 qa: Remove unused deserialization code in msg_version (MarcoFalke)
fa5099ceb7 p2p: Remove dead code for nVersion=10300 (MarcoFalke)
Pull request description:
This code is undocumented and confusing as well as dead, since peers with a version that old are disconnected immediately.
Tree-SHA512: 58c131a2730b630ffdc191cd65fe736ed1bd57e184902e2af1b1399443c4654617e68774432016df023434055e85d2e8cd32fb03b40c508c3bb8db6d19427434
e254ff5d53b79bee29203b965fca572f218bff54 Introduce a maximum size for locators. (Gregory Maxwell)
Pull request description:
The largest sensible size for a locator is log in the number of blocks.
But, as noted by Coinr8d on BCT a maximum size message could encode a
hundred thousand locators. If height were used to limit the messages
that could open new attacks where peers on long low diff forks would
get disconnected and end up stuck.
Ideally, nodes first first learn to limit the size of locators they
send before limiting what would be processed, but common implementations
back off with an exponent of 2 and have an implicit limit of 2^32
blocks, so they already cannot produce locators over some size.
Locators are cheap to process so allowing a few more is harmless,
so this sets the maximum to 64-- which is enough for blockchains
with 2^64 blocks before the get overhead starts increasing.
Tree-SHA512: da28df9c46c988980da861046c62e6e7f93d0eaab3083d32e408d1062f45c00316d5e1754127e808c1feb424fa8e00e5a91aea2cc3b80326b71c148696f7cdb3
822a2a33a74c3f997e7982d629c8f6158b80c093 Modified in_addr6 cast in CConman class to work with msvc. (Aaron Clauson)
Pull request description:
Fix to allow net.cpp to compile with MSVC. Without this fix the `(in6_addr)IN6ADDR_ANY_INIT` implicit cast generates a compilation error.
Tree-SHA512: f21c5002401dc93564dcf8d49fbafe7c03ad4182df1616d2ee201e2e172f1d696ca7982fb5b42a3b7d6878c8649823044a858401b4172239fb4b0cc2a38db282
