fa815255c70d32809aac640db4a8762c7d71e8db test: Add missing sync_all to wallet_balance test (MarcoFalke)
Pull request description:
A `syncwithvalidationinterfacequeue` should be sufficient.
Fixes #16020
ACKs for top commit:
promag:
ACK fa81525. This can be tested by adding sleep in `CWallet::BlockConnected` just before `LOCK(cs_wallet)` - master will always fail while this PR will succeed.
Tree-SHA512: 07e067c698627f90f0b9848f921b7067adc70c27105db3258e056384197e50dbee055c87839d238cc11bde11179d3f5879b39e1c8e15465f8f07558c694b677d
89339d14607434b33cfa343dc75877b62b1dfe0e tests: Add test for loadblock option (Fabian Jahr)
Pull request description:
Fixes#17019
Was initially part of #17044 but as the test got larger it made sense to split it into its own commit as suggested in #17019 .
This is testing the `-loadblock` option by using the scripts in `contrib/linearize` to generate a `bootstrap.dat` file and starting a disconnected node with it. So it is also testing the linearize scripts which were untested before and needed to be made available for the CI environment, hence they are added to `DIST_CONTRIB` in `Makefile.am`.
ACKs for top commit:
laanwj:
ACK 89339d14607434b33cfa343dc75877b62b1dfe0e
Tree-SHA512: aede0cd6e8b21194973f3633bc07fa2672d66a6f85dfe6a57cee2bb269a65d19ea49d5f9ed7914a173b3847c76e70257aa865f44bde170c1999d9655b4862d1c
0616138a0797cf68ad869906c36cf0767e20b313 tests: Remove no longer needed UBSan suppressions (issues fixed). Add documentation. (practicalswift)
Pull request description:
Remove no longer needed UBSan suppressions (issues fixed). Add documentation.
This PR is the CI-only subset of #17208 (which touches code).
From a fuzzing perspective it would be really nice to be able to run UBSan with as few suppressions as possible :)
Top commit has no ACKs.
Tree-SHA512: a926ab3e80e12a805af110fbff470cdc61ef4db536919a5b8896ea8b70f761114a52d9b1c0f48b11c1d48338351bf2e003e01ce60c613612f26ba298dcc29cd9
fa677d1801fb9153a95a1fc9855fd5f21fc440c0 ci: Remove redundant check for TRAVIS_OS_NAME (MarcoFalke)
fadccb263baf6b8694f750623add42f966e423a3 doc: Document that GNU tools are required for linters (MarcoFalke)
4444704ca9f66cdc24ab2d444941354db1dfed06 ci: Cleanup macOS runs (MarcoFalke)
Pull request description:
* Remove a commented out cleanup task in `before_cache`
* Remove the linter run on macOS, and document that GNU tools are required to run the linters
ACKs for top commit:
Sjors:
Code review ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0
laanwj:
ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0
ryanofsky:
Code review ACK fa677d1801fb9153a95a1fc9855fd5f21fc440c0 for new third commit replacing TRAVIS_OS_NAME check with NO_DEPENDS setting
Tree-SHA512: 9122a63bbe7887d9e379123152ea4ba44324cb18033b9e6b45bfdb1af665c10ea598564b9fcd57330d208a08e4696e41b4d6175f05f0843a3a76530da114f8c6
9576614d2d91ca946d164dfffca441f8bcbd6e2c doc: Describe log files + consistent paths in test READMEs (Martin Erlandsson)
Pull request description:
picks up #15830
I saw this was almost ready to merge but the test logging part was not 100% correct. I reworked that part, the rest is the same.
ACKs for top commit:
GChuf:
ACK 9576614d2d91ca946d164dfffca441f8bcbd6e2c
Tree-SHA512: 3de7f1b0a1b0419df6e7b55964d00e715b6cb7874b1849ad6f120597610d7df4182c4b61b9c9691ce04f4e392ed3caead4c623374be2066ac31319e702d45d09
32d665c2657793c8b2cc7248d26d80a940acfe20 test: fix "tx-size-small" errors after default address change (Sebastian Falbesoner)
Pull request description:
Addresses #17043, affects RBF and BIP68 functional tests.
The "tx-size-small" policy rule rejects transactions with a non-witness size of
smaller than 82 bytes (see `src/validation.cpp:MemPoolAccept::PreChecks(...)`),
which corresponds to a transaction with 1 segwit input and 1 P2WPKH output.
Through the default address change, the created test transactions have segwit
inputs now and sending to short scriptPubKeys might violate this rule. By
bumping the dummy scriptPubKey size to 22 bytes (= the size of a P2WPKH
scriptPubKey), on all occurences the problem is solved.
The dummy scriptPubKey has the format:
```21 <21-byte-long string of 'a' or 1s>```
ACKs for top commit:
instagibbs:
reACK 32d665c265 just s/Bytes/bytes/
MarcoFalke:
ACK 32d665c2657793c8b2cc7248d26d80a940acfe20
Tree-SHA512: 80e0386ff3c3f462901ba5c1e5ef2cbf095d9c0a40c8c3cfeacd4a3ab676afe744aa95b9eed77b4b3eec88bed930b33aa718117ed0977f6374e858a2f3bd5c57
d20d756752 rpc: faster getblockstats using BlockUndo data (Felix Weis)
Pull request description:
Using undo data for a block (rev?????.dat) we can retrieve value information about prevouts and calculate the final transaction fee (rate). This approach is about 80x faster, drops the requirement for `-txindex`, and works for all non-pruned blocks.
```
# 2018-11-25T16:36:19Z Bitcoin Core version v0.17.99.0-edc715240-dirty (release build)
seq 550100 550200 0.00s user 0.00s system 62% cpu 0.004 total
xargs -n1 src/bitcoin-cli getblockstats 0.21s user 0.19s system 17% cpu 2.302 total
# 2018-11-25T16:39:17Z Bitcoin Core version v0.17.0 (release build)
seq 550100 550200 0.00s user 0.00s system 87% cpu 0.002 total
xargs -n1 src/bitcoin-cli getblockstats 0.24s user 0.22s system 0% cpu 3:19.42 total
```
ACKs for commit d20d75:
MarcoFalke:
re-utACK d20d7567528e216badb8475df298bb3cec008985
Tree-SHA512: 5babc3eb8d2fee2cb23dc12f522656b80737a540cbf2b13390a8f388304c46c064cca76f896b46a6e2abae8cc582d28e1ab20dd4bb17ad6142f20630c2d30c54
3fd7e76f6d [tests] Move deterministic address import to setup_nodes (John Newbery)
Pull request description:
This requires a small changes to a few tests, but means that
deterministic addresses will always be imported (unless setup_nodes
behaviour is explicitly overridden).
Tidies up the way we import deterministic addresses, requested in review comment here: https://github.com/bitcoin/bitcoin/pull/14468#discussion_r225594586.
Tree-SHA512: 2b32edf500e286c463398487ab1153116a1dc90f64a53614716373311abdc83d8a251fdd8f42d1146b56e308664deaf62952113f66e98bc37f23968096d1a961
07cae5287c [wallet] remove unused GetScriptForMining (Sjors Provoost)
8bb3e4c487 [rpc] remove deprecated generate method (Sjors Provoost)
Pull request description:
As announced in v0.18, the wallet generate rpc method is deprecated and will be fully removed in v0.19.
Clients should transition to using the node rpc method `generatetoaddress`.
Tree-SHA512: 9e5e913b59f3e18440b2b7b356124c7b87ad19f81a1ab6ada06a6c396b84e734895465f569296f1ba8c12abf74863bab5fd77765c9e806c239713aa83a59485f
8f7b93047581c67f2133cdb8c7845471de66c30f Drop the leading 0 from the version number (Andrew Chow)
Pull request description:
Removes the leading 0 from the version number. The minor version, which we had been using as the major version, is now the major version. The revision, which we had been using as the minor version, is now the minor version. The revision number is dropped. The build number is promoted to being part of the version number. This also avoids issues where it was accidentally not included in the version number.
The CLIENT_VERSION remains the same format as previous as previously, as the Major version was 0 so it never actually got included in it.
The user agent string formatter is updated to follow this new versioning.
***
Honestly I'm just tired of all of the people asking for "1.0" that maybe this'll shut them up. Skip the whole 1.0 thing and go straight to version 22.0!
Also, this means that the terminology we commonly use lines up with how the variables are named. So major versions are actually bumping the major version number, etc.
ACKs for top commit:
jnewbery:
Code review ACK 8f7b930475
MarcoFalke:
review ACK 8f7b93047581c67f2133cdb8c7845471de66c30f 🎻
Tree-SHA512: b5c3fae14d4c0a9c0ab3b1db7c949ecc0ac3537646306b13d98dd0efc17c489cdd16d43f0a24aaa28e9c4a92ea360500e05480a335b03f9fb308010cdd93a436
cf9ed307e6 qa: blocktools enforce named args for amount (MarcoFalke)
Pull request description:
Since #13669 changed some signatures, I think it might be worthwhile to enforce named args for primitive types such as amounts.
Tree-SHA512: 2733e7b6a20590b54bd54e81a09e3f5e2fadf4390bed594916b70729bcf485b048266012c1203369e0968032a2c6a2719107ac17ee925d8939af3df916eab1a6
Issues with current implementation: params list is not mentioning `baseBlockHashes`, `baseBlockHashesNb` looks excessive, no default values, handling of baseBlockHash-es is off by 1 (`3 + i` should be `4 + i`).
before:
```
> help quorum rotationinfo
quorum rotationinfo "blockRequestHash" baseBlockHashesNb extraShare
Get quorum rotation information
Arguments:
1. blockRequestHash (string, required) The blockHash of the request.
2. baseBlockHashesNb (numeric, required) Number of baseBlockHashes
3. extraShare (boolean, required) Extra share
```
after:
```
> help quorum rotationinfo
quorum rotationinfo "blockRequestHash" ( extraShare "baseBlockHash..." )
Get quorum rotation information
Arguments:
1. blockRequestHash (string, required) The blockHash of the request.
2. extraShare (boolean, optional, default=false) Extra share
3. baseBlockHash... (string, optional, default=) baseBlockHashes
```
* tests: move `move_to_next_cycle` to `DashTestFramework`
* tests: set correct defaults for `mine_cycle_quorum`
* tests: use correct quorum type in `create_islock`
* tests: fix `rpc_verifyislock.py`
* tests: fix `feature_llmq_is_cl_conflicts.py`
* tests: isolate zmq subscribers in `interface_zmq_dash.py`
this lets us call `test_*_publishers()` in any order and any number of times
* tests: check zmq for both deterministic and non-deterministic islocks
27fcb40fc0c9ba5608a982be7372924b1edefb17 doc: replace outdated OpenSSL comment in test README (fanquake)
Pull request description:
The OpenSSL dependency was removed in #15826.
ACKs for top commit:
laanwj:
ACK 27fcb40fc0c9ba5608a982be7372924b1edefb17
theStack:
ACK 27fcb40fc0
Tree-SHA512: eb7a3b18fefa91e6f27c50fa065d6cc330f7b633ae8ee51145cdeec4df51dea5155f0d1fa91e75f1202adef04e063f3eda12773cd00a093f29f5a5e83c4fda73
* tests: fix `wait_for_*` methods
should not override `all_ok` once it's `False`
* tests: force non-mns to switch to "fully synced" mnsync status in `setup_network`
like we do this for the controller node and all mns already
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
* Added GET_SNAPSHOT_INFO message handling
* Quorum members by rotation
* Quorum utils functions
* Handle GET_QUORUM_ROTATION_INFO with baseBlockHash from client
* Storing QuorumSnaphots in evoDB when requesting them
* Added DIP Enforcement param
* quorumIndex cache
* Quorum Rotation deployment control
* Usage of Bitsets for storing CQuorumSnapshots
* Correct handling of early quorum quarters
* More asserts
* Corrections
* Handling of quorumIndex
* Refactoring of truncate mechanism
* Various fixes
* Interface correction
* Added template type for indexed cache
* Added quorumIndex into commitmenHash
* Various changes
* Needs to update maqQuorumsCache along with indexedQuorumsCache
* Added CFinalCommitment version 2
* Renamed variables
* Fixes
* Refactoring & correct caching of quorumMembers by rotation
* Added assertions
* Refactoring
* Interface change
* Handling of previous DKG session failure
* Applied refactoring
* Build quarter members improvments
* Merge Quorum Rotation and Decreased fee into one deployment (DIP24)
* Added new LLMQ Type
* Added functional tests + refactoring
* Refactoring
* Spreaded Quorum creation and Quorum Index adaptation
* quorumIndex adaptations
* Added quorumIndex in CFinalCommitment
* Latest work
* Final refactoring
* Batch of refactoring
* Fixes for tests
* Fix for CFinalCommitment
* Fix for Quorums
* Fix
* Small changes
* Thread sync fic
* Safety changes
* Reuse mns when needed
* Refactoring
* More refactoring
* Fixes for rotationinfo handling
* Fix for rotation of members
* Correct order of MNs lists in Quorum Snapshots
* Adding extra logs
* Sync rotation quorums + qrinfo changes
* Fix + extra logs
* Removed redundant field
* Fix for null final commitment + refactoring
* Added timers in tests
* Fix for qrinfo message: quorumdiff and merkleRootQuorums
* Small changes for rotation test
* Remove reading from scanQuorumCache
* Added quorum list output
* Crash fix
* Experimental commit
* apply changes to specialtxman.cpp from specialtx.cpp
* all the changes
* substancially speed up feature_llmq_rotation.py
* reenable asserts, add check for reorgs
* Refactoring
* Added extra logs
* format
* trivial
* drop extra boost includes
* drop ContainsMN
* fix ScanQuorums
* check quorum hash and index in CFinalCommitment::Verify
* fix/tweak tests
* IsQuorumRotationEnabled should be aware of the context
* Calculating members based on earlier block.
* Fix for Quorum Members Cache
* Removed duplicate size of baseBlockHashes
* Adaptations of qrinfo to -8 mn lists
* Introduction of llmqTypeDIP24InstantSend
* Adaptation for llmqTypeDIP24InstantSend
* Adaptations for IS
* bump protocol version
* Added feature_llmq_is_migration test
* Various cleanups
* use unordered_lru_cache for quorumSnapshotCache
* trivial refactor ComputeQuorumMembersByQuarterRotation
* Reduced CFinalCommitment::quorumIndex from 32 to 16 bits
* Keep verified LLMQ relay connections
* Experimental Relay connection fix
* Fix for EnsureQuorumConnections rotation
* Using only valid Mns for checking
* Override of nPowTargetSpacing (devnet only)
* Show penalty score in masternode rpc
* fixups
* Rotation refactoring
* Update src/chainparams.cpp
* Replaced LogPrintf with LogPrint
* IS locking fix once DIP24 activation
* Various cleanup
* Updated MIN_MASTERNODE_PROTO_VERSION
* Introduce LLMQ_TEST_INSTANTSEND reg-test only quorum and actually test switching to dip0024 quorums
* Renamed field lastQuorumHashPerIndex
* Renamed to DIP0024
* chore: update nStartTime and nTimeout for mainnet / testnet for DEPLOYMENT_DIP0024
Co-authored-by: Kittywhiskers Van Gogh <63189531+kittywhiskers@users.noreply.github.com>
Co-authored-by: pasta <pasta@dashboost.org>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
460fa8e0d9c1d08b5b5bca3fa02ba2763337aaf1 test: remove `import socket` in test_ipv6_local (brunoerg)
Pull request description:
Since this module (`socket`) is imported at the top of file, there is no need to import it again within the function.
ACKs for top commit:
MarcoFalke:
cr ACK 460fa8e0d9c1d08b5b5bca3fa02ba2763337aaf1
Tree-SHA512: 031c17a776dedaa21b3ec6458ca822304e76a5a3f4494406e6b7b04f08cc2abefcfe742c462b60c9b3e2fee3cd110a69ed5ad413357886dc7b823abc916ea40e
d8dfc403f74858b79c87f650b5d609aa60a4dc3b script: redirecting stderr to stdout before pipelining into grep (anouar kappitou)
30df5c3dd4d39d9027b0341d01d3233400104893 script: preventing non-compatible sed binary. (anouar kappitou)
Pull request description:
This Pull request improve scripted diff by checking for non-compatible sed binary. Fixes#19815
ACKs for top commit:
shaavan:
reACK d8dfc403f74858b79c87f650b5d609aa60a4dc3b
theStack:
Tested ACK d8dfc403f74858b79c87f650b5d609aa60a4dc3b
Tree-SHA512: 4813abb02195e04b8953662e51cba4599789e6185a6d384f56255e2d37c2b16be46c7c8861e44ba1a13d42573a58eb708901472925cb308dba2f602020e9f0f6
140a49ce5e547a1b520a7cd063af8308184e7cbf test: check that pruneblockchain RPC fails for future block or timestamp (Sebastian Falbesoner)
Pull request description:
This PR adds missing test coverage for the `pruneblockchain` RPC for the case that a future block or timestamp is passed:
8c0bd871fc/src/rpc/blockchain.cpp (L1101)8c0bd871fc/src/rpc/blockchain.cpp (L1111)
Note that the test method `manual_test` gets called twice, once each with `use_timestamp` set to True/False, respectively. Depending on that, the helper function `height` either converts the passed block height to the timestamp of that block, or just returns it without modification.
The other tests for failures in this RPC are also changed to be more detailled ("Cannot prune blocks because node is not in prune mode", "Negative block height"), as I don't think there is any value in just checking a sub-string. If there is ever an error with the same sub-string is introduced, it's not clear which error is exactly checked with the test, so it makes sense to be as specific as possible.
ACKs for top commit:
brunoerg:
tACK 140a49ce5e547a1b520a7cd063af8308184e7cbf
Tree-SHA512: bee3cee9f35c2a63a1839d7ec1f83e354d9d3c0c2ca32d300dca2de8b755d555f769ba2b80ac37d31df6ee7e2b8eaefb8134c4727a7144e47c0f5e34f2cc5822
4105a543817c47bda21739d5ad3269677f84e861 lint: remove boost::bind linter (fanquake)
Pull request description:
I don't think we need to maintain a linter for reintroducing boost::bind at this point.
ACKs for top commit:
hebasto:
ACK 4105a543817c47bda21739d5ad3269677f84e861, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 86bda91ee7ed11f0aa7ac95e9e7b62dbba626dcea75444d2851a3d40e794ab16bef09a1f0c956a716d43602b23c1cf67e1ff3a51184ea1ee7d686fbb76316cb0
fa6c62f34b50818102ad58f2ffd152189f54d973 test: Replace log with assert_equal in wallet_abandonconflict (MarcoFalke)
Pull request description:
This will make the test fail as soon as the bug is fixed, forcing it to
be updated. Otherwise, the bug might be fixed (or made worse)
accidentally, leaving the test in an inconsistent state.
ACKs for top commit:
theStack:
Concept and code-review ACK fa6c62f34b50818102ad58f2ffd152189f54d973
brunoerg:
tACK fa6c62f34b50818102ad58f2ffd152189f54d973
Tree-SHA512: 416f72380164bf3f93332a5cfa81a8e61d8ada3714ef6815889ed3cf2d16c09411760dee4acf19629227e565b765b3dea491a0b23efd38eb370254cfadf7c441
bda620aecd690004c52e550ad7de187ce0eb655d test: check abandoned tx in listsinceblock (brunoerg)
Pull request description:
This PR tests if the abandoned transaction is correct in listsinceblock return (wallet_abandonconflict.py).
ACKs for top commit:
jonatack:
ACK bda620aecd690004c52e550ad7de187ce0eb655d
theStack:
ACK bda620aecd690004c52e550ad7de187ce0eb655d
stratospher:
Tested ACK bda620a. This PR verifies whether the transaction txAB1 has been abandoned in listsinceblock and is a nice addition to the test!
Tree-SHA512: e4dce344cf621de7a8b5bd8660d252419772a293080fc881f6f448b6df85c6b1c8f0df619e855a40b6393f53c836f0d7fadbd3916c20cccd3a95830b8b502991
502f50da12694dd0193744427f0f89238e612f54 Test transactions conflicted by double spend in listtransactions (Jon Atack)
Pull request description:
Test the properties of transactions conflicted by a double spend as returned by RPC listtransactions in the abandoned, confirmations, trusted and walletconflicts fields. These fields are also returned by RPCs listsinceblock and gettransactions.
ACKs for top commit:
brunoerg:
tACK 502f50da12694dd0193744427f0f89238e612f54
rajarshimaitra:
Concept + tACK 502f50da12
Tree-SHA512: 28968f4a5f1960ea45b2e6f5b20fe25c1b51f66944062dcddea52ea970ad21c74d583793d091b84e8a5e506d6aecc1f0435c5b918213975b22c38e02bba19aa1
54b5eb2b149a1f2a4a1dbdb9e0648c5a390d8e22 tests: Add std::locale::global to list of locale dependent functions in lint-locale-dependence.sh (practicalswift)
Pull request description:
Add `std::locale::global` to list of locale dependent functions in `lint-locale-dependence.sh`.
We currently flag `setlocale(...)` as locale dependent, but prior to this commit we didn't flag
`std::locale::global(...)` as such.
In addition to setting the global C++ locale `std::locale::global(...)` also does the equivalent of `std::setlocale(LC_ALL, ...);`.
Thus the functionality of `std::locale::global(...)` is a superset of `setlocale(...)` :)
ACKs for top commit:
MarcoFalke:
ACK 54b5eb2b149a1f2a4a1dbdb9e0648c5a390d8e22, fine with me
Tree-SHA512: bcf2f1c765add6ed09c3debca968b75eeea81602503f109c0f76ec98635911d453f4834a39e741703c3d470f123178e8952191a9b1a3429394b99c07765dcf1f
e61de6306fd89fe9aae90253062e7b1b20343f8a Change ismine to take a CWallet instead of CKeyStore (Andrew Chow)
7c611e20007bf5face34d33dffa26c8db67e29ec Move ismine to wallet module (Andrew Chow)
Pull request description:
`IsMine` isn't used outside of the wallet except for the tests. It also doesn't make sense to be outside of the wallet. This PR moves `IsMine` into the wallet module and for it to take a `CWallet` instead of `CKeyStore`. The test that used `IsMine` is also moved to the wallet tests.
This is first [prerequisites](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Class-Structure-Changes#ismine) for the wallet structure changes.
ACKs for commit e61de6:
MarcoFalke:
re-ACK e61de6306f (only change is rebase with git auto-merge)
meshcollider:
Very light code review ACK e61de6306f
Tree-SHA512: 1cb4ad12652aef7922ab7460c6d413e8b9d1855dca78c0a286ae49d5c0765bc7996c55f262c742001d434eb9bd4215dc2cc7aae1b371ee1a82d46b32c17e6341
Co-authored-by: MeshCollider <dobsonsa68@gmail.com>
5db506ba5943868cc2c845f717508739b7f05714 tests: Add option --valgrind to run nodes under valgrind in the functional tests (practicalswift)
Pull request description:
What is better than fixing bugs? Fixing entire bug classes of course! :)
Add option `--valgrind` to run the functional tests under Valgrind.
Regular functional testing under Valgrind would have caught many of the uninitialized reads we've seen historically.
Let's kill this bug class once and for all: let's never use an uninitialized value ever again. Or at least not one that would be triggered by running the functional tests! :)
My hope is that this addition will make it super-easy to run the functional tests under Valgrind and thus increase the probability of people making use of it :)
Hopefully `test/functional/test_runner.py --valgrind` will become a natural part of the pre-release QA process.
**Usage:**
```
$ test/functional/test_runner.py --help
…
--valgrind run nodes under the valgrind memory error detector:
expect at least a ~10x slowdown, valgrind 3.14 or
later required
```
**Live demo:**
First, let's re-introduce a memory bug by reverting the recent P2P uninitialized read bug fix from PR #17624 ("net: Fix an uninitialized read in ProcessMessage(…, "tx", …) when receiving a transaction we already have").
```
$ git diff
diff --git a/src/consensus/validation.h b/src/consensus/validation.h
index 3401eb64c..940adea33 100644
--- a/src/consensus/validation.h
+++ b/src/consensus/validation.h
@@ -114,7 +114,7 @@ inline ValidationState::~ValidationState() {};
class TxValidationState : public ValidationState {
private:
- TxValidationResult m_result = TxValidationResult::TX_RESULT_UNSET;
+ TxValidationResult m_result;
public:
bool Invalid(TxValidationResult result,
const std::string &reject_reason="",
```
Second, let's test as normal without Valgrind:
```
$ test/functional/p2p_segwit.py -l INFO
2019-11-28T09:30:42.810000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test__fc8q3qo
…
2019-11-28T09:31:57.187000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
…
2019-11-28T09:32:08.265000Z TestFramework (INFO): Tests successful
```
Third, let's test with `--valgrind` and see if the test fail (as we expect) when the unitialized value is used:
```
$ test/functional/p2p_segwit.py -l INFO --valgrind
2019-11-28T09:32:33.018000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_gtjecx2l
…
2019-11-28T09:40:36.702000Z TestFramework (INFO): Subtest: test_non_standard_witness_blinding (Segwit active = True)
2019-11-28T09:40:37.813000Z TestFramework (ERROR): Assertion failed
ConnectionRefusedError: [Errno 111] Connection refused
```
ACKs for top commit:
MarcoFalke:
ACK 5db506ba5943868cc2c845f717508739b7f05714
jonatack:
ACK 5db506ba5943868cc2c845f717508739b7f05714
Tree-SHA512: 2eaecacf4da166febad88b2a8ee6d7ac2bcd38d4c1892ca39516b6343e8f8c8814edf5eaf14c90f11a069a0389d24f0713076112ac284de987e72fc5f6cc3795
fa37e0a68bea65979f9f8f2e5258fe608acf2bdf test: Show debug log on unit test failure (MarcoFalke)
Pull request description:
Often, it is hard to debug unit test failures without the debug log. Especially when the failure happens remotely (e.g. on a ci system).
Fix that by printing the log on failure.
ACKs for top commit:
jamesob:
ACK fa37e0a68bea65979f9f8f2e5258fe608acf2bdf ([`jamesob/ackr/16975.1.MarcoFalke.test_show_debug_log_on_u`](https://github.com/jamesob/bitcoin/tree/ackr/16975.1.MarcoFalke.test_show_debug_log_on_u))
Tree-SHA512: 2ca4150c4ae3d4ad47e03b5e5e70da2baffec928ddef1fdf53a3ebc061f14aee249205387cb1b12ef6d4eb55711ef0080c0b41d9d18000b5da124ca80299793b
9743432034586385cfef87df4b377c255ed0cba8 Fix bug where duplicate PSBT keys are accepted (John L. Jegutanis)
Pull request description:
As per the BIP 174 spec a PSBT key cannot be duplicated,
however the current code accepts key duplication.
The PSBT key/value entries can be duplicated when the value
is `empty()` or `IsNull()` for `CScript` or `CTxOut` respectively
and if those key/value entries are serialized before the non-empty ones.
For example, the following PSBT, included in the test vectors,
contains a duplicate field:
```
// magic
70736274ff
// global tx
//// key
0100
//// value
2a02000000000140420f000000000017a9146e91b72d5593e7d4391e2ff44e91e985c31641f08700000000
//// separator
00
// no inputs
// outputs
//// key PSBT_OUT_WITNESSSCRIPT
0101
//// value (empty script)
00
//// key PSBT_OUT_WITNESSSCRIPT (same as the above)
0101
//// value (an OP_RETURN script)
016a
//// separator
00
```
ACKs for top commit:
achow101:
ACK 9743432034586385cfef87df4b377c255ed0cba8
instagibbs:
code review ACK 9743432034
Tree-SHA512: 34f4b34c8e6561c6a6ab745cdd319f6687eac6f7cecc735c94035eeca8c5157e17a27f2ae853dbaa6634fcd5a8f4e1c6cc13d1ebd7e563459665d72bb147cc1e
fac86ac7b3ceac2f884412c7a9f4bd5bab5e3916 scripted-diff: Add missed copyright headers (Hennadii Stepanov)
6fde9d5e47fc9a1042b3fb68031eab5bf55e508d script: Update EXLUDE list in copyright_header.py (Hennadii Stepanov)
1998152f15fd2b0e83f5068c375a34feaf73db8c script: Add empty line after C++ copyright (Hennadii Stepanov)
071f2fc204f542c5a287ca8835115a2ee0bf2f50 script: Add ability to insert copyright to *.sh (Hennadii Stepanov)
Pull request description:
This PR improves `contrib/devtools/copyright_header.py` script and adds copyright headers to the files in `src` and `test` directories with two exceptions:
- [`src/reverse_iterator.h`](https://github.com/bitcoin/bitcoin/blob/master/src/reverse_iterator.h) (added to exceptions)
- [`src/test/fuzz/FuzzedDataProvider.h`](https://github.com/bitcoin/bitcoin/blob/master/src/test/fuzz/FuzzedDataProvider.h) (added to exceptions)
On master 5622d8f3156a293e61d0964c33d4b21d8c9fd5e0:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
25 with zero copyrights
```
With this PR:
```
$ ./contrib/devtools/copyright_header.py report . | grep zero
2 with zero copyrights
```
~I am uncertain about our copyright policy with `build_msvc` and `contrib` directories content, so they are out of scope of this PR.~
ACKs for top commit:
MarcoFalke:
ACK fac86ac7b3ceac2f884412c7a9f4bd5bab5e3916
Tree-SHA512: d7832c4a7a1a3b7806119775b40ec35d7982f49ff0e6199b8cee4c0e0a36e68d51728b6ee9924b1c161df4bc6105bd93391b79d42914357fa522f499cb113fa8
8a6810d0d2759c69f63b53c48aa79e0cfdd88ffb Add a 'logpath' field to getrpcinfo (darosior)
Pull request description:
as discussed in #15438
ACKs for top commit:
laanwj:
Tested ACK 8a6810d0d2759c69f63b53c48aa79e0cfdd88ffb
Tree-SHA512: 752c7d90f670677c8144efb338c5c97c2264f85f1e65e031fd5a44f04230b6eafbabd0f634db263eb42c25642ecc1c4b1b602d4735e3fab07ec00b566134ddab
* Merge #16291: gui: Stop translating PACKAGE_NAME
fa64b947bb3075ff8737656706b941af691908ab util: No translation of `Bitcoin Core` in the copyright (MarcoFalke)
fab85208f678ba1be53bdb73a73ce3c5c937d448 qt: Run «make translate» in ./src/ (MarcoFalke)
fabe87d2c923ab3a70b8cde2666a4d1cda8b22fb scripted-diff: Avoid passing PACKAGE_NAME for translation (MarcoFalke)
fa5e9f157e568b7fbbea1482b393181f0733f2ba build: Stop translating PACKAGE_NAME (MarcoFalke)
Pull request description:
Generally the package name is not translated, but the package description is.
E.g. `GIMP` or `Firefox` are always called that way regardless of the system language. However, "`Firefox` webbrowser" or "`GIMP` image manipulation program" are translated.
ACKs for top commit:
hebasto:
ACK fa64b947bb3075ff8737656706b941af691908ab, I have not tested the code, but I have reviewed it and it looks OK, I agree it can be merged.
Tree-SHA512: 626f811531182d0ba0ef1044930d32726773349bcb49b10261288a86ee6b80a183db30a87d817d5b0d501fad058ac22d6272311716b4f5a154f17c6f391a5a1a
* more
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
b8909b074603a05a1a255461ca78d8a013dd3044 Run functional tests with all possible flags (Samuel Dobson)
Pull request description:
Functional tests which use flags like `--descriptors` or `--legacy-wallet` won't run if only the base script is given to `test_runner.py` because it doesn't match any script in the list exactly. It would be easier if it would just run both options.
For example, instead of:
```
test_runner.py 'wallet_basic.py --legacy-wallet' 'wallet_basic.py --descriptors'
```
We can now just run:
```
test_runner.py wallet_basic
```
Also useful for `--usecli`, the IPv4/IPv6/nonloopback `rpc_bind.py` variations, etc.
ACKs for top commit:
laanwj:
Code review ACK b8909b074603a05a1a255461ca78d8a013dd3044
MarcoFalke:
review ACK b8909b074603a05a1a255461ca78d8a013dd3044
Tree-SHA512: d367037cb170e705551726d47fe4569ebc3ceadece280dd3edbb3ecb41e19f3263d6d272b407316ed6011164e850df4321fb340b1b183b34497c9f7cc439f4d8
c427a5800bb53208d30eeb03a73ab8be879e5f45 doc: Set PYTHONUTF8=1 for functional tests on Windows (Hennadii Stepanov)
Pull request description:
The `PYTHONUTF8` environment variable is defined in [PEP 540](https://www.python.org/dev/peps/pep-0540/), and it is actually used in our CI: 5e3380b9f5/.cirrus.yml (L89)
This PR documents such usage to avoid users' [errors](https://github.com/bitcoin/bitcoin/pull/22922#issuecomment-915511037).
ACKs for top commit:
MarcoFalke:
cr ACK c427a5800bb53208d30eeb03a73ab8be879e5f45
Tree-SHA512: 441b8cecfe47d548cfe403b0e1cd0aef25c1a70ff556434ead1f1e26372919931ac6f208a4ed6fd8dcca46e8709245e4fb06f95259a43c8e1221473ce1ee497b
* Compressed headers implementation.
First header is always compressed in a headers2 msg
Version is uncompressed if it’s not matched within the last 7 unique versions to be sent in the current msg
Service flag to signal that the peer supports compressed headers
If compressed headers services is active, the peer will receive headers compressed
If both sendheaders and sendheaders2 are sent, the peer will respond with compressed headers
Functional tests as for uncompressed headers
Updates regarding the existing functional tests to use the compressed headers if the NODE_HEADERS_COMPRESSED service flag is active
* style: add missing comma
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Co-authored-by: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com>
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
fab17e8272f5f70213f186809479ee7a75898b1d test: Add basic test for BIP34 (MarcoFalke)
Pull request description:
BIP34 was disabled for testing, which explains why it had no test.
Fix that by enabling it and adding a test.
Tree-SHA512: 9cb5702d474117ce6420226eb93ee09d6fb5fc856fabc8b67abe56a088cd727674e0e5462000e1afa83b911374036f90abdbdde56a8c236a75572ed47e10a00f
fa38d3df69851212fea7544badadc1c3e5369bf5 [rpc] Correct reconsiderblock help text, add test (MarcoFalke)
Pull request description:
Rework documentation and test to match the implementation
Tree-SHA512: d0adef6b054a341bcc1cb87783a4e4cf9be124ba6812e1ac88246a5e01b2861a8071b12dba880b2b428c37da3fa860bfec3fe3e5fbb7c28696872113faa84a9f
a6b5ec18f rpc: creates possibility to preserve labels on importprivkey (marcoagner)
Pull request description:
Closes#13087.
As discussed in the issue, this is a feature request instead of a bug report since the behaviour was as intended (i.e. label with default: `''`). With this, the old behaviour is kept while the possibility to achieve the preservation of labels, as expected in the open issue, is added.
Tree-SHA512: b33be50e1e7f62f7ddfae953177ba0926e2d848961f9fac7501c2b513322c0cb95787745d07d137488267bad1104ecfdbe800c6747f94162eb07c976835c1386
893aa207e84b74e7623243967d29f03570fdfd6f tests: Add fuzzing harness for CheckBlock(...) and other CBlock related functions (practicalswift)
ec8dcb0199c6d6ae47a13abbd158f59532554adb tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)
Pull request description:
Add fuzzing harness for `CheckBlock(...)` and other `CBlock` related functions.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz --with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/block
…
# And to to quickly verify that the relevant code regions are triggered, that the
# fuzzing throughput seems reasonable, etc.
$ contrib/devtools/test_fuzzing_harnesses.sh '^block$'
```
`test_fuzzing_harnesses.sh` can be found in PR #17000.
Top commit has no ACKs.
Tree-SHA512: 275abd46d8ac970b28d8176f59124988b1e07c070173e001acd55995b830333417f301c309199fc589da08a6ac4c03aa74650d5e1638f6e3023dfbd3c9f6921d
597d10ceb9fd2a118c7e551cd6263379691d9295 tests: Add fuzzing harness for various functions consuming only integrals (practicalswift)
575383b3e1361e60ba88738a34d92b1662f915a7 tests: Add corpora suppression (FUZZERS_MISSING_CORPORA) for fuzzers missing in https://github.com/bitcoin-core/qa-assets/tree/master/fuzz_seed_corpus (practicalswift)
Pull request description:
Add fuzzing harness for various functions consuming only integrals.
**Testing this PR**
Run:
```
$ CC=clang CXX=clang++ ./configure --enable-fuzz \
--with-sanitizers=address,fuzzer,undefined
$ make
$ src/test/fuzz/integer
```
Top commit has no ACKs.
Tree-SHA512: f0ccbd63671636f8e661385b682e16ad287fef8f92e7f91327ee2093afc36fcd424e1646fe90279388e28a760bcc795766eb80cf6375e0f873efff37fc7e2393
dddd09eb33d14fabda0aa40fa008b23b2bd6e589 test: Wait until mempool is loaded in wallet_abandonconflict (MarcoFalke)
Pull request description:
This might or might not fix intermittent issues such as https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/28724018#L4091
I believe the mempool was not loaded fully after the restart, in which case it was not dumped either on the next restart. Thus, the previous mempool was attempted to be loaded a second time, which succeeded and contained the txs.
ACKs for top commit:
laanwj:
ACK dddd09eb33d14fabda0aa40fa008b23b2bd6e589
Tree-SHA512: ab7061f946b5e5388f825dddceadb125f5197b24af3a7fcf1e700235d106a323419a56bfb4d84a2e27442e0de63e540c623b704343d83a98deaab3c02fcbdcbe
f4b00b70e Import public keys in order (Andrew Chow)
9e1551b9c Test pubkey import to keypool (Andrew Chow)
513719c5f Add option to importmulti add an imported pubkey to the keypool (Andrew Chow)
9b81fd19a Fetch keys from keypool when private keys are disabled (Andrew Chow)
99cccb900 Add a method to add a pubkey to the keypool (Andrew Chow)
Pull request description:
If the wallet has private keys disabled, allow importing public keys into the keypool. A `keypool` option has been added to `importmulti` in order to signal that the keys should be added to the keypool.
Tree-SHA512: e88ea7bf726c13031aa739389a0c2662e6b22a4f9a4dc45b042418c692a950d98f170e0db80eb59e9c9063cda8765eaa85b2927d1790b9625744f7a87bad5fc8
fa178a6385 [rpc] mining: Omit uninitialized currentblockweight, currentblocktx (MarcoFalke)
Pull request description:
Previously we'd report "0", which could be mistaken for a valid number. E.g. the number of transactions is 0 or the block weight is 0, whatever that means.
Tree-SHA512: ee94ab203a329e272211b726f4c23edec4b09c650ec363b77fd59ad9264165d73064f78ebb9e11b5c2c543b73c157752410a307655560531c7d5444d203aa0ea
e3e1a5631e [test] functional: set cwd of nodes to tmpdir (Sjors Provoost)
Pull request description:
Any process launched by bitcoind will have `self.datadir` as its `cwd`.
Tree-SHA512: 0b311643bb96c7dc2f693774620173243b3add40bf373284695af2f0071823b23485289fd2ffe152b7f63e0bfe989b16720077cfc2ce33905f9b8e7f2630f3c0
cb3511b9d Add release notes for importing key origin info change (Andrew Chow)
4c75a69f3 Test importing descriptors with key origin information (Andrew Chow)
02d6586d7 Import KeyOriginData when importing descriptors (Andrew Chow)
3d235dff5 Implement a function to add KeyOriginInfo to a wallet (Andrew Chow)
eab63bc26 Store key origin info in key metadata (Andrew Chow)
345bff601 Remove hdmasterkeyid (Andrew Chow)
bac8c676a Add a method to CWallet to write just CKeyMetadata (Andrew Chow)
e7652d3f6 Add WriteHDKeypath function and move *HDKeypath to util/bip32.{h,cpp} (Andrew Chow)
c45415f73 Refactor keymetadata writing to a separate method (Andrew Chow)
Pull request description:
This PR allows for key origin data as defined by the descriptors document to be imported to the wallet when importing a descriptor using `importmulti`. This allows the `walletprocesspsbt` to include the BIP 32 derivation paths for keys that it is watching that are from a different HD wallet.
In order to make this easier to use, a new field `hdmasterkeyfingerprint` has been added to `getaddressinfo`. Additionally I have removed `hdmasterkeyid` as was planned. I think that this API change is fine since it was going to be removed in 0.18 anyways. `CKeyMetadata` has also been extended to store key origin info to facilitate this.
Tree-SHA512: 9c7794f3c793da57e23c5abbdc3d58779ee9dea3d53168bb86c0643a4ad5a11a446264961e2f772f35eea645048cb60954ed58050002caee4e43cd9f51215097
* feat: introduce devnetVersion as a method for breaking changes to devnets
include devnetVersion in expected version for devnet connections
* feat: always use DGW on devnets
* fix p2p_connect_to_devnet.py
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
bb41e632ca wallet_balance.py: Prevent edge cases (Steven Roose)
Pull request description:
I ran into this edge case when running the test on Elements. I had a 0-value output as change.
ACKs for commit bb41e6:
Tree-SHA512: ef4c25289cafcdb4437f11ed537664dff5afedcefab75a46f985d3be70551de2d3bc8e9cfcb22c0f3d7d2eb95ff40df78b8d01dbacbf90c36bca00426937b0a2
fa79a783d6 test: Add reorg test to wallet_balance (MarcoFalke)
fad03cd046 test: Check that wallet txs not in the mempool are untrusted (MarcoFalke)
fa195315e6 test: Add getunconfirmedbalance test with conflicts (MarcoFalke)
fa464e8211 test: Add wallet_balance test for watchonly (MarcoFalke)
Pull request description:
Second commit can be reviewed with `--ignore-all-space`
ACKs for commit fa79a7:
jnewbery:
utACK fa79a783d63060dc6a8521c1de58b158979a59e9
Tree-SHA512: ec4919a3c93b6dcb35d58e7c65bdffe7f4c8cb87b9287f3679631c1823ef5bd72789f233def94e60c1ab332711601751645566f5997ce250af55b328ed60e917
0d101a340c44841cbbc5982d55354b1787bc39e2 test: Add test for maxtxfee option (MarcoFalke)
177550101b600ccb32886695326eb72cd9752c8b wallet: Remove unreachable code in CreateTransaction (MarcoFalke)
5c1b9714cb0a13be28324f91f4ec9ca66a1de8c7 wallet: Fix -maxtxfee check by moving it to CWallet::CreateTransaction (João Barbosa)
Pull request description:
Follow up to #16257, this PR makes `bumpfee` aware of `-maxtxfee`.
It also prevents dangling locked unspents when calling `fundrawtransaction` - because the previous check was after `LockCoin`.
ACKs for top commit:
MarcoFalke:
re-ACK 0d101a340c44841cbbc5982d55354b1787bc39e2, only change is small test fixup
Tree-SHA512: 3464b24ae7cd4e72ed41438c6661828ba1304af020f05da62720b23668ae734e16cf47c6d97e150cc84ef631ee099b16fc786c858f3d089905845437338fd512
fa89badf887dcc01e5bdece248b5e7d234fee227 test: Require standard txs in regtest (MarcoFalke)
fa9b4191609c3ef75e69d391eb91e4d5c1e0bcf5 test: Add test that mainnet requires standard txs (MarcoFalke)
fa613ca0a8f99c4771859de9e571878530d3ecb5 chainparams: Remove unused fMineBlocksOnDemand (MarcoFalke)
Pull request description:
I don't see a reason why regtest should allow non-standard txs, as it makes testing mainnet behaviour such as #15846 unnecessarily hard and unintuitive.
Of course, testnet policy remains unchanged to allow propagation of non-standard txs.
ACKs for top commit:
ajtowns:
ACK fa89badf887dcc01e5bdece248b5e7d234fee227
Tree-SHA512: c4c675affb054868850bd2683aa07f4c741a448cbacb2ea8334191e105f426b0790fe6a468be61e9c5880d24154f7bf1c7075051697172dce92180c1bc3a1c90
3f592b8 [QA] add wallet-rbf test (Jonas Schnelli)
8222e05 Disable wallet fallbackfee by default on mainnet (Jonas Schnelli)
Pull request description:
Removes the default fallback fee on mainnet (but keeps it on testnet/regtest).
Transactions using the fallbackfee in case the fallback fee has not been set are getting rejected.
Tree-SHA512: e54d2594b7f954e640cc513a18b0bfbe189f15e15bdeed4fe02b7677f939bca1731fef781b073127ffd4ce08a595fb118259b8826cdaa077ff7d5ae9495810db
faf8318c55a6001270a6fc8ed2298767099bafba test: Split fundrawtx test into subtests (MarcoFalke)
fa6fba3bc8013d7f813edd71f152d86eab907e4d test: Make local symbols in run_test members (MarcoFalke)
Pull request description:
This prevents scope-leak of symbols that are supposed to be local to one test case.
Top commit has no ACKs.
Tree-SHA512: 9b2a4ca2cdd631ef915d2f7e6cd62375df9a0919448350aa6e5ae4aa8a8fe3ba53870f7a9a25a57736894b4e3a45e861018253ed2d57d9a64c2bb65fa270fad8
* refactor: break circular dependencies(-13, +2)
introduces specialtxman, which handles validation of special transactions, specialtx is now simply the primitive underlying type. This breaks a lot of the circular depends
Also removes an unneeded `#include <masternode/payments.h>` in net_processing.cpp, which resolves a circular dependency. (we know it's okay to remove b/c masternode/payments.h isn't included in any header files, and removing it doesn't break compilation)
* format: make clang-format happy
* remove unrelated change
* remove some unneeded includes to `evo/deterministicmns.h`, explicitly include some previously implicitly included includes.
Resolves two circular dependencies
* refactor: remove circular depend, unused include
c4b0c08f7c91bcef48dd023982ff132795575247 Update tx-size-small comment with relevant CVE disclosure (Gregory Sanders)
Pull request description:
Code first introduced under https://github.com/bitcoin/bitcoin/pull/11423 with essentially no description and no discussion.
ACKs for top commit:
MarcoFalke:
ACK c4b0c08f7c91bcef48dd023982ff132795575247
fanquake:
ACK c4b0c08f7c91bcef48dd023982ff132795575247
Tree-SHA512: 95d5c92998b8b1e944c477dbaee265b62612b6e815099ab31d9ff580b4dff777abaf7f326a284644709f918aa1510412d62310689b1250ef6e64de7b19ca9f71
59e387705c7e55ec40400301346354fa2d0c613f test: add invalid tx templates for use in functional tests (James O'Beirne)
Pull request description:
This change adds a list of `CTransaction`-generating templates which each correspond to a specific type of invalid transaction. We then use this list to test for a wider variety of invalid tx types in `p2p_invalid_tx.py` and `feature_block.py`.
Consolidating all invalid tx types will allow us to more easily cover all tx reject cases from a variety of tests without repeating ourselves. Validation logic doesn't differ much between mempool and block acceptance, but there *is* a difference and we should be sure we're testing both comprehensively.
Right now, I've only added templates covering the tx reject types listed below but if this approach seems worthwhile I will expand the list to be fully comprehensive.
```
bad-txns-in-belowout
bad-txns-inputs-duplicate
bad-txns-too-many-sigops
bad-txns-vin-empty
bad-txns-vout-empty
bad-txns-vout-negative
```
Tree-SHA512: 05407f4a953fbd7c44c08bb49bb989cefd39a2b05ea00f5b3c92197a3f05e1b302f789e33832445734220e1c333d133aba385740b77b84139b170c583471ce20
2222c96deec0f636dee6e49efb745f29b06a40a5 test: Add notes on how to generate data/wallets/high_minversion (MarcoFalke)
Pull request description:
I forgot to do this in #16796
ACKs for top commit:
ryanofsky:
ACK 2222c96deec0f636dee6e49efb745f29b06a40a5
Tree-SHA512: 5f24ffa641b97eac4febad42ade7228b14fa72335c918a10880c5dec86a3ecc3075a31526f275188e07fea95b8e2c6320c64f716099f604b00e13d5366fcee37
Make fMixing atomic as it has concurrent access
Add tsan suppression for zmq namespace
Suppress deadlock false positive in ConnectTip
Switch ubsan target to linux32
Add new test job for linux64_cxx17 target without any sanitizers
Increase rpc time out for block reward reallocation test
Fix heap use after free in CConnman::GetExtraOutboundCount()
Different builds for linux32 and linux64 tsan and ubsan
Increase timeout for llmq_signing functional test
67f4e9c522 Include core_io.h from core_read.cpp (practicalswift)
eca9767673 Make reasoning about dependencies easier by not including unused dependencies (practicalswift)
Pull request description:
Make reasoning about dependencies easier by not including unused dependencies.
Please note that the removed headers are _not_ "transitively included" by other still included headers. Thus the removals are real.
As an added bonus this change means less work for the preprocessor/compiler. At least 51 393 lines of code no longer needs to be processed:
```
$ git diff -u HEAD~1 | grep -E '^\-#include ' | cut -f2 -d"<" | cut -f1 -d">" | \
sed 's%^%src/%g' | xargs cat | wc -l
51393
```
Note that 51 393 is the lower bound: the real number is likely much higher when taking into account transitively included headers :-)
ACKs for commit 67f4e9:
Tree-SHA512: 0c8868aac59813f099ce53d5307eed7962dd6f2ff3546768ef9e5c4508b87f8210f1a22c7e826c3c06bebbf28bdbfcf1628ed354c2d0fdb9a31a42cefb8fdf13
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
fab0c820fa4c0c3227eec85c64310a3bf938a149 rpc: Clarify that block count means height excl genesis (MarcoFalke)
Pull request description:
There is a common misconception that the block count returned by the blockchain rpcs includes the genesis block. See for example the discussion in https://github.com/bitcoin/bitcoin/pull/16292#issuecomment-506303256.
However, it really returns the height, which is `0` for the genesis block.
So clarify that and also remove the misleading "longest blockchain" comment.
Finally, fix the wallet test that incorrectly used this rpc.
ACKs for top commit:
instagibbs:
utACK fab0c820fa
promag:
ACK fab0c82, sorry for the misconception.
Tree-SHA512: 0d087cbb628d3866352bca6420402f392e6a997e579941701a408a7fca355d84645045661f39b022e4479cc07f85a6cddaa9095b6fd9911b245692482420a5e4
fa4c8679ed94f215ce895938f7c3c169a2ce101e rpc: Avoid creating non-standard raw transactions (MarcoFalke)
Pull request description:
Multiple OP_RETURN outputs in a transaction are not standard and unlikely to be relayed, so avoid creating them.
Apart from that, the logic was broken in that it duplicated the same hex-data for each data output: Closes#14868.
Tree-SHA512: b08d08062b5622e8a7b497e490ccaf53b06e844c863fda3bf3f932a98684a809e8341aeb98232059a795afb32d8770a6c5591a66f8e6ee372b672af245607887
* optimize: somehow optimize circular-dependencies.py
Signed-off-by: pasta <pasta@dashboost.org>
* optimize: use parallel if available to lint in parallel
Signed-off-by: pasta <pasta@dashboost.org>
* suggestions
* more suggestions
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
* src/evo/evodb.cpp:57:29: warning: Assert statement calls a function which may have desired side effects: 'IsClean'. [assertWithSideEffect]
* src/llmq/quorums.cpp:635:37: note: Null pointer dereference
src/llmq/quorums.cpp:635:37: warning: Either the condition 'pFrom==nullptr' is redundant or there is possible null pointer dereference: pFrom. [nullPointerRedundantCheck]
src/llmq/quorums.cpp:636:81: note: Assuming that condition 'pFrom==nullptr' is not redundant
* fix a bunch of cppcheck warnings
* cppcheck: run on many more files. Enable all checks except a few ignored ones.
ignored
```
"Consider using std::transform algorithm instead of a raw loop."
"Consider using std::accumulate algorithm instead of a raw loop."
```
* ci: build specific version of cppcheck instead of install from apt
* ci: use cppcheck 2.4, remove commented out line, fix symlink
cppcheck 2.6 is latest, however causes issues
```
src/spork.cpp:135:51: warning: Analysis failed. If the code is valid then please report this failure. [cppcheckError]
```
cppcheck 2.5 appears to get into an infinite loop
* no need to check presence before insertion
* use if-init, remove redundant check
* remove redundant check
* don't remove cmake? fix macOs depends build?
* cppcheck: one per line, alphabetize
* remove duplicate cmake install
