aboutsummaryrefslogtreecommitdiff
AgeCommit message (Collapse)Author
2020-10-02Explicitly filter out partial groups when we don't want themAndrew Chow
Instead of hacking OutputGroup::m_ancestors to discourage the inclusion of partial groups via the eligibility filter, add a parameter to the eligibility filter that indicates whether we want to include the group. Then for those partial groups, don't return them in GroupOutputs if we indicate they aren't desired.
2020-10-02Move OutputGroup positive only filtering into InsertAndrew Chow
2020-09-29Move EligibleForSpending into GroupOutputsAndrew Chow
Instead of filtering after the OutputGroups have been made, do it as they are being made.
2020-09-29Move fee setting of OutputGroup to InsertAndrew Chow
OutputGroup will handle the fee and effective value computations inside of Insert. It now needs to take the effective feerate and long term feerates as arguments to its constructor.
2020-09-29Move GroupOutputs into SelectCoinsMinConfAndrew Chow
2020-09-29Remove OutputGroup non-default constructorsAndrew Chow
2020-09-29Merge #20015: doc: Added default signet config for linearize scriptWladimir J. van der Laan
8a4dcda414fcf8281d8b3b79039b735c0a7d0d4c doc: Added default signet config for linearize script (gr0kchain) Pull request description: Updated the example-linearize.cfg file to include support for the signet chain network config which is used by the ./linearize-data.py Problem: Without the signet magic, genesis hash and path config, the linearize-data.py script cannot generate a bootstrap.dat file. Example: ``` ./linearize-data.py ./linearize.cfg Read 4776 hashes Genesis block not found in hashlist ``` Solution: Added netmagic, genesis and input example parameters to file. Netmagic in terms of signet is derived from the signet-challenge and not static as with other networks. The provided signet magic is based on the default public signet. Resolution ``` ./linearize-hashes.py ./linearize.cfg > ./hashlist.txt ./linearize-data.py ./linearize.cfg $ ./linearize-data.py ./linearize.cfg Read 4776 hashes Input file /Users/gr0kchain/.bitcoin/signet/blocks/blk00000.dat Output file /Users/gr0kchain/Downloads/bootstrap.dat 1000 blocks scanned, 1000 blocks written (of 4776, 20.9% complete) 2000 blocks scanned, 2000 blocks written (of 4776, 41.9% complete) 3011 blocks scanned, 3000 blocks written (of 4776, 62.8% complete) 4010 blocks scanned, 4000 blocks written (of 4776, 83.8% complete) Done (4776 blocks written) ``` ACKs for top commit: laanwj: Tested ACK 8a4dcda414fcf8281d8b3b79039b735c0a7d0d4c Tree-SHA512: ad4d330358cf67e7424fb1d97ca828c28ca2758102e45747f4059c11a8acce801162da024a20cfb892f997fd4c3f4d5af988d1ca67c74efa78bf7d4ed27dd421
2020-09-29Merge #19984: log: Remove static log message "Initializing chainstate ↵Wladimir J. van der Laan
Chainstate [ibd] @ height -1 (null)" f22d6a11423a4462196de24cd68e7f45513cc001 log: Remove static log message "Initializing chainstate Chainstate [ibd] @ height -1 (null)" (practicalswift) Pull request description: Remove static log message `Initializing chainstate Chainstate [ibd] @ height -1 (null)`. AFAICT `chainstate->ToString()` will always equal `"Chainstate [ibd] @ height -1 (null)"` here which makes the log message neither relevant nor interesting :) ACKs for top commit: laanwj: ACK f22d6a11423a4462196de24cd68e7f45513cc001 promag: ACK f22d6a11423a4462196de24cd68e7f45513cc001, just get rid of it. hebasto: ACK f22d6a11423a4462196de24cd68e7f45513cc001, I agree that the removed log message in its current state is cryptic and useless. Tree-SHA512: 1a65c0d14c9a433afcdaadef9bfcdd5d63276d5d2caee1bf3c48ac477e54fa28138f64020e6e26ca5e67872954a1e7d93fa24a12accc7c7211bc6e7a6039051d
2020-09-29Merge #20003: net: Exit with error message if -proxy is specified without ↵Wladimir J. van der Laan
arguments (instead of continuing without proxy server) 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7 net: Print error message if -proxy is specified without arguments (instead of continuing without proxy server) (practicalswift) Pull request description: Exit with error message if `-proxy` is specified without arguments (instead of continuing without proxy server). Continuing without a proxy server when the end-user has specified `-proxy` may result in accidental loss of privacy. (The end-user might think he/she is using a proxy when he/she is not.) Before this patch: ``` $ src/bitcoind -proxy … 2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -listen=0 2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -upnp=0 2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -discover=0 2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0 … 2020-09-23T00:24:33Z init message: Starting network threads... ``` `bitcoind` is now running *without* a proxy server (`GetProxy(…, …) == false`, `HaveNameProxy() == false`, etc.). Note that the "-proxy set" log messages above which the end-user might interpret as "good, my traffic is now routed via the proxy". After this patch: ``` $ src/bitcoind -proxy Error: No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>. $ echo $? 1 ``` ACKs for top commit: laanwj: re-ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7 kristapsk: ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7, I have tested the code. hebasto: re-ACK 9b4fa0af40cd88ed25dd77962235fbf268bdcaa7 Tree-SHA512: 4ba7a011991699a54b5bb87ec68367c681231bf5dcd36f8c89ff9ddc2e8d29df453817b7e362597e652ad6b341a22b7274be0fd78d435e5f0fd8058e5221c4ce
2020-09-29Merge #19898: log: print unexpected version warning in validation log categoryfanquake
62dba9628d2532dca0c44934600f5aac3b21e275 log: print unexpected version warning in validation log category (nthumann) Pull request description: Fixes #19603: As suggested by practicalswift, instead of always printing `<n> of the last 100 blocks have unexpected version` as a warning appended to UpdateTip, it is now printed in the validation log category and therefore only visible with `-debug=validation` enabled. Before: `2020-09-06T15:56:00Z UpdateTip: new best=00000000000000000001b2872e107a98b57913120e5c6c87ce2715a34c40adf8 height=646969 version=0x20400000 log2_work=92.261571 tx=565651941 date='2020-09-06T10:35:36Z' progress=0.999888 cache=32.2MiB(237417txo) warning='72 of last 100 blocks have unexpected version'` After: `2020-09-06T16:31:26Z UpdateTip: new best=0000000000000000000b3bd786dc42745dd7be4a8c695500a04518cb9e2f4dc1 height=646971 version=0x20000000 log2_work=92.261607 tx=565655901 date='2020-09-06T10:57:19Z' progress=0.999883 cache=3.8MiB(27550txo)` `2020-09-06T16:31:26Z 71 of last 100 blocks have unexpected version` Ran unit & functional tests, confirmed that the warning is now only printed when validation category is enabled. ACKs for top commit: theStack: ACK 62dba9628d2532dca0c44934600f5aac3b21e275 MarcoFalke: re-ACK 62dba96 practicalswift: ACK 62dba9628d2532dca0c44934600f5aac3b21e275 -- only change since last ACK is `s/nUpgraded/num_unexpected_version/` hebasto: re-ACK 62dba9628d2532dca0c44934600f5aac3b21e275, https://github.com/bitcoin/bitcoin/pull/19898#pullrequestreview-483158708 is resolved now. Tree-SHA512: 2100ca7d6d3fd67c92e81d75162d2506d6f1ecf1761d5180d76663fac06771b35e5c4235ebe1a00731b5f7db82db3cd19328627929c8f22912df592686ba51d3
2020-09-29Merge #19630: Cleanup fee estimation codefanquake
a3abeec33a6ae903e514c7a7b6f587b7c17288a0 policy/fees: remove a floating-point division by zero (Antoine Poinsot) c36869bbf6a38626833b4aea53be024c48ede475 policy/fees: unify some duplicated for loops (Antoine Poinsot) 569d92a4d2924a1f6d50775980b591552f6372e7 policy/fees: small readability improvements (Antoine Poinsot) 5b8cb35621891b681f9b49a9de5f6d8da4ccdecc policy/fee: remove requireGreater parameter in EstimateMedianVal() (Antoine Poinsot) dba8196b447b6a85be66890db70928100e867d8b policy/fees: correct decay explanation comments (Antoine Poinsot) Pull request description: This (*does not* change behaviour and) cleans up a bit of unused code in `CBlockPolicyEstimator` and friends, and slightly improves readability of the rest (comment correction etc.). The last commit is a small reformatting one which I could not resist but am happy to remove at will. ACKs for top commit: jnewbery: utACK a3abeec33a6ae903e514c7a7b6f587b7c17288a0 MarcoFalke: ACK a3abeec33a6ae903e514c7a7b6f587b7c17288a0 💹 ariard: Code Review ACK a3abeec. Tree-SHA512: b7620bcd23a2ffa8f7ed859467868fc0f6488279e3ee634f6d408872cb866ad086a037e8ace76599a05b7e9c07768adf5016b0ae782d153196b9c030db4c34a5
2020-09-29Merge #19107: p2p: Move all header verification into the network layer, ↵fanquake
extend logging deb52711a17236d0fca302701b5af585341ab42a Remove header checks out of net_processing (Troy Giorshev) 52d4ae46ab822d0f54e246a6f2364415cda149bd Give V1TransportDeserializer CChainParams& member (Troy Giorshev) 5bceef6b12fa16d20287693be377dace3dfec3e5 Change CMessageHeader Constructor (Troy Giorshev) 1ca20c1af8f08f07c407c3183c37b467ddf0f413 Add doxygen comment for ReceiveMsgBytes (Troy Giorshev) 890b1d7c2b8312d41d048d2db124586c5dbc8a49 Move checksum check from net_processing to net (Troy Giorshev) 2716647ebf60cea05fc9edce6a18dcce4e7727ad Give V1TransportDeserializer an m_node_id member (Troy Giorshev) Pull request description: Inspired by #15206 and #15197, this PR moves all message header verification from the message processing layer and into the network/transport layer. In the previous PRs there is a change in behavior, where we would disconnect from peers upon a single failed checksum check. In various discussions there was concern over whether this was the right choice, and some expressed a desire to see how this would look if it was made to be a pure refactor. For more context, see https://bitcoincore.reviews/15206.html#l-81. This PR improves the separation between the p2p layers, helping improvements like [BIP324](https://github.com/bitcoin/bitcoin/pull/18242) and #18989. ACKs for top commit: ryanofsky: Code review ACK deb52711a17236d0fca302701b5af585341ab42a just rebase due to conflict on adjacent line jnewbery: Code review ACK deb52711a17236d0fca302701b5af585341ab42a. Tree-SHA512: 1a3b7ae883b020cfee1bef968813e04df651ffdad9dd961a826bd80654f2c98676ce7f4721038a1b78d8790e4cebe8060419e3d8affc97ce2b9b4e4b72e6fa9f
2020-09-29Merge #19969: Send RPC bug fix and touch-upsfanquake
f7b331ea85d45c7337e527b6e77a45da7a689b7d rpc: add brackets to ConstructTransaction (Sjors Provoost) d813d26f06248aaa7be3c698c87939cc777fafd0 [rpc] send: various touch-ups (Sjors Provoost) 0fc1c685e1ca68ca8ed2b35f623bbe6a9fc36d66 [rpc] send: fix parsing replaceable option (Sjors Provoost) efc9b85e6f4aa431d308089874a18f0bbdcdd0fd Mark send RPC experimental (Sjors Provoost) Pull request description: Followup based on #16378 nits. It also fixes an argument parsing error (uncaught because the test wasn't sufficiently thorough). I marked the RPC as experimental so we can tweak it a bit over the next release cycle. ACKs for top commit: meshcollider: utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d fjahr: utACK f7b331ea85d45c7337e527b6e77a45da7a689b7d kallewoof: ACK f7b331ea85d45c7337e527b6e77a45da7a689b7d Tree-SHA512: 82dd8ac76a6558872db3f5249d4d6440469400aaa339153bc627d1ee673a91ecfadecb486bc1939ba87ebbd80e26ff29698e93e358599f3d26fde0e526892afe
2020-09-29Merge #20028: test: Check that invalid peer traffic is accounted forfanquake
faa94cb1675d8bd511eb593176cd07aa59465225 test: Check that invalid peer traffic is accounted for (MarcoFalke) fae243f0cb92b5648d07d0a5033e2f4de862ae99 test: Remove confusing cast to same type (int to int) (MarcoFalke) Pull request description: Couldn't find a test for this and it seems something we should test, so I wrote one. ACKs for top commit: vasild: ACK faa94cb16 practicalswift: ACK faa94cb1675d8bd511eb593176cd07aa59465225: patch looks correct Tree-SHA512: efcdd35960045cdfbd14480e16e0d1d09e81eb01670ac541ac2b105e1a63818a157c95853270242234a224880873e79957832bf4231374d7fe81de30f35e3abf
2020-09-29Merge #20014: doc: Mention signet in -help outputfanquake
b3972bca9f74b3e75918676ef029f1964b0622a1 doc: Mention signet in -help output (Hennadii Stepanov) Pull request description: ``` $ src/bitcoind -help | grep -A 4 -e '-chain=' | head -8 -chain=<chain> Use the chain <chain> (default: main). Allowed values: main, test, signet, regtest -signet Use the signet chain. Equivalent to -chain=signet. Note that the network is defined by the -signetchallenge parameter ``` ``` $ src/bitcoind -help | grep -A 3 -e '-port=' -port=<port> Listen for connections on <port> (default: 8333, testnet: 18333 signet: 38333, regtest: 18444) ``` ``` $ src/bitcoind -help | grep -A 3 -e '-rpcport=' -rpcport=<port> Listen for JSON-RPC connections on <port> (default: 8332, testnet: 18332, signet: 38332, regtest: 18443) ``` ACKs for top commit: jonatack: ACK b3972bca9f74b3e75918676ef029f1964b0622a1 kallewoof: ACK b3972bca9f74b3e75918676ef029f1964b0622a1 ajtowns: ACK b3972bca9f74b3e75918676ef029f1964b0622a1 - skimmed code only, looks fine to me Tree-SHA512: 66c59cdc3c19e8f8a02d3f3f992ff1db06769df63244d4af62629e18aaf4a12b3b7e75e4a0b9f616033cdc4415da046053ba36fede8be145b2dc695b2aa69a02
2020-09-28Merge #19845: net: CNetAddr: add support to (un)serialize as ADDRv2Pieter Wuille
7be6ff61875a8d5d2335bff5d1f16ba40557adb0 net: recognize TORv3/I2P/CJDNS networks (Vasil Dimov) e0d73573a37bf4b519f6f61e5678572d48a64517 net: CNetAddr: add support to (un)serialize as ADDRv2 (Vasil Dimov) fe42411b4b07b99c591855f5f00ad45dfeec8e30 test: move HasReason so it can be reused (Vasil Dimov) d2bb681f96fb327b4c4d5b2b113692ca22fdffbf util: move HasPrefix() so it can be reused (Vasil Dimov) Pull request description: (chopped off from #19031 to ease review) Add an optional support to serialize/unserialize `CNetAddr` in ADDRv2 format (BIP155). The new serialization is engaged by ORing a flag into the stream version. So far this is only used in tests to ensure the new code works as expected. ACKs for top commit: Sjors: re-tACK 7be6ff61875a8d5d2335bff5d1f16ba40557adb0 sipa: re-utACK 7be6ff61875a8d5d2335bff5d1f16ba40557adb0 eriknylund: ACK 7be6ff61875a8d5d2335bff5d1f16ba40557adb0 I built the PR on macOS Catalina 10.15.6, ran both tests and functional tests. I've reviewed the code and think the changes look good and according to BIP155. I verified that the added Base32 encoding test looks as proposed and working. I've run a node for a week only with Onion addresses `-onlynet=onion` without issues and I can connect to other peer reviewers running TorV3 on their nodes and I can connect both of my test nodes to each other. jonatack: re-ACK 7be6ff61875a8d5d2335bff5d1f16ba40557adb0 per `git diff b9c46e0 7be6ff6`, debug build, ran/running bitcoind with this change and observed the log and `-netinfo` peer connections while connected as a tor v2 service to both tor v2 peers and also five tor v3 peers. hebasto: ACK 7be6ff61875a8d5d2335bff5d1f16ba40557adb0, tested on Linux Mint 20 (x86_64): on top of this pull and #19031 I'm able to connect to onion v3 addresses, and jonatack is able to connect to my created onion v3 address. Tree-SHA512: dc621411ac4393993aa3ccad10991717ec5f9f2643cae46a24a89802df0a33d6042994fc8ff2f0f397a3dbcd1c0e58fe4724305a2f9eb64d9342c3bdf784d9be
2020-09-28Merge #15367: feature: Added ability for users to add a startup commandMarcoFalke
090530cc24054d6b4658752bb88f75a3b73eab5d feature: Added ability for users to add a startup command (Ben Carman) Pull request description: Thoughts for adding the feature is for users to be able to add things like electrum-personal-server or lnd to run whenever Bitcoin Core is running. Open to feedback about the feature. ACKs for top commit: MarcoFalke: re-ACK 090530cc24 dongcarl: tACK 090530c Tree-SHA512: ba514d2fc8b4fb12b781c1a9c89845a25fce0b80ba7c907761cde4abb81edd03fa643682edc895986dc20b273ac3b95769508806db7fbd99ec28623f85c41e67
2020-09-28feature: Added ability for users to add a startup commandBen Carman
2020-09-28test: Check that invalid peer traffic is accounted forMarcoFalke
2020-09-28test: Remove confusing cast to same type (int to int)MarcoFalke
2020-09-28Merge #20016: uint256: 1 is a constantMarcoFalke
4cc7171c9887e88027642927c979e507d7b78dda wallet: no need for duplicate storage for ABANDON_HASH constant (Anthony Towns) 82cf4641f4a161834d07ce83c18982d9b143c040 scripted-diff: Replace UINT256_ONE() with uint256::ONE (Anthony Towns) 183f308fff4caad3e3ada654b6fdf597d262c4c1 uint256: Update constructors to c++11, make ONE static (Anthony Towns) Pull request description: `UINT256_ONE()` returns a reference to a global; mark it as const to be sure someone doesn't accidently modify it. ACKs for top commit: promag: ACK 4cc7171c9887e88027642927c979e507d7b78dda MarcoFalke: re ACK 4cc7171c98, only change is some constexpr shuffling 🛁 kallewoof: ACK 4cc7171c9887e88027642927c979e507d7b78dda Tree-SHA512: 7f399658bfd9ffa4075bc2349049476d842b9579a67518fb7151f56eab36907ef24b1474ee1e89bdc69fe181abe7295dfe19e33b3623d43cec71fc00e356e347
2020-09-28Merge #20026: test: typo in a functional test docMarcoFalke
a5a6965157f458e18f420f62df3c00620123c74d [Trivial] python help message (kanon) Pull request description: before: ` --segwit Test behaviour with SegWit txn (which should fail` after: ` --segwit Test behaviour with SegWit txn (which should fail)` ACKs for top commit: kristapsk: ACK a5a6965157f458e18f420f62df3c00620123c74d Tree-SHA512: b1176bca8ec4d722fca71fe790ca21dcf05bf7b6562f56ef2758a3e6d7a0554c16a08efc5b154446f75b9a09bd0ff1dfcf22e71010c748caeb02b1f5c883f9ca
2020-09-28wallet: no need for duplicate storage for ABANDON_HASH constantAnthony Towns
2020-09-28scripted-diff: Replace UINT256_ONE() with uint256::ONEAnthony Towns
-BEGIN VERIFY SCRIPT- sed -i '/inline.* UINT256_ONE() {/,+1d' src/uint256.h sed -i 's/UINT256_ONE()/uint256::ONE/' $(git grep -l UINT256_ONE) -END VERIFY SCRIPT-
2020-09-28uint256: Update constructors to c++11, make ONE staticAnthony Towns
Replace the memset with C++11 value/aggregate initialisation of the m_data array, which still ensures the unspecified values end up as zero-initialised. This then allows changing UINT256_ONE() from dynamically allocating an object, to a simpler referencing a static allocation.
2020-09-27[Trivial] python help messagekanon
2020-09-26Merge #20023: test: remove unused constants in functional testsMarcoFalke
92e28fa8b2590cce0e8f0adadae80e46cb63a9ef test: remove unused constants in functional tests (Sebastian Falbesoner) Pull request description: This mini-PR gets rid of constants in functional tests that are not used anymore. Found by [vulture ](https://pypi.org/project/vulture/)via the following script that has been lying around here locally for quite some time (I think it was once proposed by practicalswift, but I don't remember the concrete topic/PR): ``` #!/bin/sh for F in $(git ls-files -- "*.py"); do vulture "$F" | grep "unused variable"; done ``` ACKs for top commit: practicalswift: ACK 92e28fa8b2590cce0e8f0adadae80e46cb63a9ef: patch looks correct. Tree-SHA512: 16516abc8014207bcefdf0545dffaecff1fbba66f45b54c02371dcfd1f18194855c6b72598c11b5407009561eafe8048d47af3471f0efb1795d52477d5a0232e
2020-09-26Merge #19725: [RPC] Add connection type to getpeerinfo, improve logsMarcoFalke
a512925e19a70d7f6b80ac530a169f45ffaafa1c [doc] Release notes (Amiti Uttarwar) 50f94b34a33c954f6e207f509c93d33267a5c3e2 [rpc] Deprecate getpeerinfo addnode field (Amiti Uttarwar) df091b9b509f0b10e4315c0bfa2da0cc0c31c22f [refactor] Rename test file to allow any getpeerinfo deprecations. (Amiti Uttarwar) 395acfa83a5436790c1a722a5609ac9d48df235f [rpc] Add connection type to getpeerinfo RPC, update tests (Amiti Uttarwar) 49c10a9ca40967d28ae16dfea9cccc6f3a6624a1 [log] Add connection type to log statement (Amiti Uttarwar) Pull request description: After #19316, we can more directly expose information about the connection type on the `getpeerinfo` RPC. Doing so also makes the existing addnode field redundant, so this PR begins the process of deprecating this field. This PR also includes one commit that improves a log message, as both use a shared function to return the connection type as a string. Suggested by sdaftuar- https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468001604 & https://github.com/bitcoin/bitcoin/pull/19316#discussion_r468018093 ACKs for top commit: jnewbery: Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c. sipa: utACK a512925e19a70d7f6b80ac530a169f45ffaafa1c guggero: Tested and code review ACK a512925e. MarcoFalke: cr ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c 🌇 promag: Code review ACK a512925e19a70d7f6b80ac530a169f45ffaafa1c. Tree-SHA512: 601a7a38aee235ee59aca690784f886dc2ae4e418b2e6422c4b58cd597376c00f74910f66920b08a08a0bec28bf8022e71a1435785ff6ba8a188954261aba78e
2020-09-26Merge #20022: test: use explicit p2p objects where availableMarcoFalke
0fcaf731997c4989b869e42d8990f742637799c2 test: use explicit p2p objects where available (Oliver Gugger) Pull request description: This is a follow-up patch to #19804 as suggested by MarcoFalke (https://github.com/bitcoin/bitcoin/pull/19804#discussion_r494950062). To make the intent of the tests easier to understand, we reference the p2p connection objects by their explicit names instead of the p2ps array. ACKs for top commit: theStack: ACK 0fcaf731997c4989b869e42d8990f742637799c2 Tree-SHA512: 37db22185077beeadfa7245bb768b87d6b7a2cfb906c3c859ab92ec3d657122db7301777f0838e13dfc748f37303850144fb7553e6cb6c66903e304d6e10e659
2020-09-26test: remove unused constants in functional testsSebastian Falbesoner
2020-09-26test: use explicit p2p objects where availableOliver Gugger
To make the intent of the tests easier to understand, we reference the p2p connection objects by their explicit names instead of the p2ps array.
2020-09-25doc: Added default signet config for linearize scriptgr0kchain
2020-09-25doc: Mention signet in -help outputHennadii Stepanov
2020-09-25Merge #19804: test/refactor: reference p2p objects explicitly and remove ↵MarcoFalke
confusing Test_Node.p2p property 10d61505fe77880d6989115defa5e08417f3de2d [test] remove confusing p2p property (gzhao408) 549d30faf04612d9589c81edf9770c99e3221885 scripted-diff: replace p2p with p2ps[0] in p2p_invalid_tx (gzhao408) 7a0de46aeafb351cffa3410e1aae9809fd4698ad [doc] sample code for test framework p2p objects (gzhao408) 784f757994c1306bb6584b14c0c78617d6248432 [refactor] clarify tests by referencing p2p objects directly (gzhao408) Pull request description: The `TestNode` has a `p2p` property which is an alias for `p2ps[0]`. I think this should be removed because it can be confusing and misleading (to both the test writer and reviewer), especially if a TestNode has multiple p2ps connected (which is the case for many tests). Another example is when a test has multiple subtests that connect 1 p2p and use the `p2p` property to reference it. If the subtests don't completely clean up after themselves, the subtests may affect one another. The best way to refer to a connected p2p is use the object returned by `add_p2p_connection` like this: ```py p2p_conn = node.add_p2p_connection(P2PInterface()) ``` A good example is [p2p_invalid_locator.py](https://github.com/bitcoin/bitcoin/blob/master/test/functional/p2p_invalid_locator.py), which cleans up after itself (waits in both `wait_for_disconnect` and in `disconnect_p2ps`) but wouldn't need so much complexity if it just referenced the connections directly. If there is only one connected, it's not really that tedious to just use `node.p2ps[0]` instead of `node.p2p` (and it can always be aliased inside the test itself). ACKs for top commit: robot-dreams: utACK 10d61505fe77880d6989115defa5e08417f3de2d jnewbery: utACK 10d61505fe77880d6989115defa5e08417f3de2d guggero: Concept ACK 10d61505. Tree-SHA512: 5965548929794ec660dae03467640cb2156d7d826cefd26d3a126472cbc2494b855c1d26bbb7b412281fbdc92b9798b9765a85c27bc1a97f7798f27f64db6f13
2020-09-23Merge #19927: validation: Reduce direct g_chainman usageMarcoFalke
72a1d5c6f3834e206719ee5121df7727aed5b786 validation: Remove review-only comments + assertions (Carl Dong) 3756853b15902d63f4b5a3129e8b5d82e84e125b docs: Move FindFilesToPrune{,Manual} doxygen comment (Carl Dong) 485899a93c6f5fff62090907efb0ac938992e1fb style: Make FindFilesToPrune{,Manual} match style guide (Carl Dong) 3f5b5f3f6db0e5716911b3fba1460ce327e8a845 validation: Move FindFilesToPrune{,Manual} to BlockManager (Carl Dong) f8d4975ab3fcd3553843cf0862251289c88c106b validation: Move PruneOneBlockFile to BlockManager (Carl Dong) 74f73c783d46b012f375d819e2cd09c792820cd5 validation: Pass in chainman to UnloadBlockIndex (Carl Dong) 4668ded6d6ea4299d998abbb57543f37519812e2 validation: Move ~CMainCleanup logic to ~BlockManager (Carl Dong) Pull request description: This PR paves the way for de-globalizing `g_chainman` entirely by removing the usage of `g_chainman` in the following functions/methods: - `~CMainCleanup` - `CChainState::FlushStateToDisk` - `UnloadBlockIndex` The remaining direct uses of `g_chainman` are as follows: 1. In initialization codepaths: - `AppTests` - `AppInitMain` - `TestingSetup::TestingSetup` 2. `::ChainstateActive` 3. `LookupBlockIndex` - Note: `LookupBlockIndex` is used extensively throughout the codebase and require a much larger set of changes, therefore I've left it out of this initial PR ACKs for top commit: MarcoFalke: re-ACK 72a1d5c6f3 👚 jnewbery: utACK 72a1d5c6f3834e206719ee5121df7727aed5b786 Tree-SHA512: 944a4fa8405eecf39706ff944375d6824373aaeea849d11473f08181eff26b12f70043a8348a5b08e6e9021b243b481842fbdfbc7c3140ca795fce3688b7f5c3
2020-09-23Merge #19868: build: Fix target nameMarcoFalke
7a89f2e6c539a54bcaa24bff41aae3910244ad3d build: Fix target name (Hennadii Stepanov) Pull request description: It seems like a typo :) This PR: - fixes errors when building a package in depends for `HOST=x86_64-apple-darwin16` (fix #19799) - is a correct alternative to https://github.com/bitcoin/bitcoin/pull/19764/commits/d25e0e308f51f6b995e5d0033224c424dc8afc2c from #19764 ACKs for top commit: icota: tACK https://github.com/bitcoin/bitcoin/pull/19868/commits/7a89f2e6c539a54bcaa24bff41aae3910244ad3d dongcarl: Code Review ACK 7a89f2e6c539a54bcaa24bff41aae3910244ad3d theuni: ACK 7a89f2e6c539a54bcaa24bff41aae3910244ad3d. Tree-SHA512: a0bcbc6805d3450e201476ef1e22e0eb53903db1586c5515314c19afd337bded887e56de0fbe62feaf359b2de15dbccd49a44f1a8b566b4c64f5ae3d94a2ab6d
2020-09-23Merge #19994: Assert that RPCArg names are equal to CRPCCommand ones (net, ↵MarcoFalke
rpcwallet) fa14f57fbc3c1fa2b9eea5df687f0fb36d452bd5 Assert that RPCArg names are equal to CRPCCommand ones (net, rpcwallet) (MarcoFalke) Pull request description: This is the last part split out from #18531 to just touch some RPC methods. Description from the main pr: ### Motivation RPCArg names in the rpc help are currently only used for documentation. However, in the future they could be used to teach the server the named arguments. Named arguments are currently registered by the `CRPCCommand`s and duplicate the RPCArg names from the documentation. This redundancy is fragile, and has lead to errors in the past (despite having linters to catch those kind of errors). See section "bugs found" for a list of bugs that have been found as a result of the changes here. ### Changes The changes here add an assert in the `CRPCCommand` constructor that the RPCArg names are identical to the ones in the `CRPCCommand`. ### Future work > Here or follow up, makes sense to also assert type of returned UniValue? Sure, but let's not get ahead of ourselves. I am going to submit any further works as follow-ups, including: * Removing the CRPCCommand arguments, now that they are asserted to be equal and thus redundant * Removing all python regex linters on the args, now that RPCMan can be used to generate any output, including the cli.cpp table * Auto-formatting and sanity checking the RPCExamples with RPCMan * Checking passed-in json in self-check. Removing redundant checks * Checking returned json against documentation to avoid regressions or false documentation * Compile the RPC documentation at compile-time to ensure it doesn't change at runtime and is completely static ### Bugs found * The assert identified issue #18607 * The changes itself fixed bug #19250 ACKs for top commit: fjahr: tACK fa14f57fbc3c1fa2b9eea5df687f0fb36d452bd5 ryanofsky: Code review ACK fa14f57fbc3c1fa2b9eea5df687f0fb36d452bd5. Just straightforward replacements except code moved in `addnode`, and displatching updated in `bumpfee_helper` Tree-SHA512: e07af150f1d95a88e558256ce197a6b7dc6cd722a6d6c13c75d944c49c2e2441f8b8237e9f94b03db69fa18f9bda627b0781d5e1da70bf5415e09b38728a8cb1
2020-09-23net: Print error message if -proxy is specified without arguments (instead ↵practicalswift
of continuing without proxy server)
2020-09-23Merge #19979: Replace LockAssertion with AssertLockHeld, remove LockAssertionMarcoFalke
0bd1184adf6610c0bd14f4e9a25c0a200e65ae25 Remove unused LockAssertion struct (Hennadii Stepanov) ab2a44297fd0796bf5797ae2a477e8e56d9c3c12 Replace LockAssertion with a proper thread safety annotations (Hennadii Stepanov) 73f71e19965e07534eb47701f2b23c9ed59ef475 refactor: Use explicit function type instead of template (Hennadii Stepanov) Pull request description: This PR replaces `LockAssertion` with `AssertLockHeld`, and removes `LockAssertion`. This PR is compared with alternatives in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/AssertLockHeld-PRs ACKs for top commit: MarcoFalke: ACK 0bd1184adf6610c0bd14f4e9a25c0a200e65ae25 ajtowns: ACK 0bd1184adf6610c0bd14f4e9a25c0a200e65ae25 vasild: ACK 0bd1184ad Tree-SHA512: ef7780dd689faf0bb479fdb97c49bc652e2dd10c148234bb95502dfbb676442d8565ee37864d923ca21a25f9dc2a335bf46ee82c095e387b59a664ab05c0ae41
2020-09-23Merge #19572: ZMQ: Create "sequence" notifier, enabling client-side mempool ↵Wladimir J. van der Laan
tracking 759d94e70f6844443106404882c7b105f3a4dba7 Update zmq notification documentation and sample consumer (Gregory Sanders) 68c3c7e1bdd00bbe7d70592a8eb39520fa3f87f1 Add functional tests for zmq sequence topic and mempool sequence logic (Gregory Sanders) e76fc2b84d065c9d06010d0a10b316f1f9d36fb9 Add 'sequence' zmq publisher to track all block (dis)connects, mempool deltas (Gregory Sanders) 1b615e61bfc464f215a1b48e6e27d1e8fc16b2d1 zmq test: Actually make reorg occur (Gregory Sanders) Pull request description: This PR creates a new ZMQ notifier that gives a "total hash history" of block (dis)connection, mempool addition/substraction, all in one pipeline. It also exposes a "mempool sequence number" to both this notifier and `getrawmempool` results, which allows the consumer to use the results together without confusion about ordering of results and without excessive `getrawmempool` polling. See the functional test `interfaces_zmq.py::test_mempool_sync` which shows the proposed user flow for the client-side tracking of mempool contents and confirmations. Inspired by https://github.com/bitcoin/bitcoin/pull/19462#issuecomment-656140421 Alternative to https://github.com/bitcoin/bitcoin/pull/19462 due to noted deficiencies in current zmq notification streams. Also fixes a legacy zmq test that didn't actually trigger a reorg because of identical blocks being generated on each side of the split(oops) ACKs for top commit: laanwj: Code review ACK 759d94e70f6844443106404882c7b105f3a4dba7 Tree-SHA512: 9daf0d7d996190f3a68ff40340a687519323d7a6c51dcb26be457fbc013217ea7b62fbd0700b74b654433d2e370704feb61e5584399290692464fcfcb72ce3b7
2020-09-23Merge #19993: refactor: Signet fixupsMarcoFalke
facaf9e61f4b9ea91fab554d495ebea1043d08fb doc: Document signet BIP (MarcoFalke) faf0a26711eed9264113463e56b988cf9fe549fd doc: Update comments for new chain settings (-signet and -chain=signet) (MarcoFalke) fae0548686cb3d095086d3f0fef38dcfcd31d8ca fuzz: Remove needless guard (MarcoFalke) 77771a03df6c5d940b340d15eb88f2ac9a29c13a refactor: Remove SignetTxs::m_valid and use optional instead (MarcoFalke) fa2ad5dae17b237641b8ece0e68ffcdd79d543bf test: Run signet test even when wallet was not compiled (MarcoFalke) Pull request description: Some doc and test fixups for #18267 ACKs for top commit: ajtowns: ACK facaf9e61f4b9ea91fab554d495ebea1043d08fb -- code review only dr-orlovsky: Reviewed & ACK https://github.com/bitcoin/bitcoin/pull/19993/commits/facaf9e61f4b9ea91fab554d495ebea1043d08fb kallewoof: Code review ACK facaf9e61f4b9ea91fab554d495ebea1043d08fb Tree-SHA512: 8085027c488d84bb4bddccba78bd2d4c5af0d8e2644ee72265f1f30fa8c83f61a961d9da2c796f2940e69682291cbee7b1028b6a6ce123ad9134c0ebbf4723b0
2020-09-22Remove header checks out of net_processingTroy Giorshev
This moves header size and netmagic checking out of net_processing and into net. This check now runs in ReadHeader, so that net can exit early out of receiving bytes from the peer. IsValid is now slimmed down, so it no longer needs a MessageStartChars& parameter. Additionally this removes the rest of the m_valid_* members from CNetMessage.
2020-09-22Give V1TransportDeserializer CChainParams& memberTroy Giorshev
This adds a CChainParams& member to V1TransportDeserializer member, and use it in place of many Params() calls. In addition to reducing the number of calls to a global, this removes a parameter from GetMessage (and will later allow us to remove one from CMessageHeader::IsValid())
2020-09-22Change CMessageHeader ConstructorTroy Giorshev
This commit removes the single-parameter contructor of CMessageHeader and replaces it with a default constructor. The single parameter contructor isn't used anywhere except for tests. There is no reason to initialize a CMessageHeader with a particular messagestart. This messagestart should always be replaced when deserializing an actual message header so that we can run checks on it. The default constructor initializes it to zero, just like the command and checksum. This also removes a parameter of a V1TransportDeserializer constructor, as it was only used for this purpose.
2020-09-22Add doxygen comment for ReceiveMsgBytesTroy Giorshev
2020-09-22Move checksum check from net_processing to netTroy Giorshev
This removes the m_valid_checksum member from CNetMessage. Instead, GetMessage() returns an Optional. Additionally, GetMessage() has been given an out parameter to be used to hold error information. For now it is specifically a uint32_t used to hold the raw size of the corrupt message. The checksum check is now done in GetMessage.
2020-09-22Give V1TransportDeserializer an m_node_id memberTroy Giorshev
This is intended to only be used for logging. This will allow log messages in the following commits to keep recording the peer's ID, even when logging is moved into V1TransportDeserializer.
2020-09-22Merge #19963: Clarify blocksonly whitelistforcerelay testMarcoFalke
e15344889aac50aadee9211ac34e466867d5862b Clarify blocksonly whitelistforcerelay test (t-bast) Pull request description: As discussed in https://github.com/bitcoin/bitcoin/issues/19943, this test may be a bit misleading to newcomers. We underscore the fact that our peer needs to run a modified version of Bitcoin Core to actually relay transactions to a `blocksonly` node and benefit from the `whitelistforcerelay` parameter. ACKs for top commit: naumenkogs: ACK e15344889aac50aadee9211ac34e466867d5862b Tree-SHA512: cc3526ac26c40a2d878b0ad863008663040683fd21092fcdc93866c2b0a79db8c2d29767d1f70bf56195092fca2aa2961cdbee52b5f0b1ae757cece9cd206301
2020-09-22Merge #19959: build: patch qt libpng to fix powerpc buildMarcoFalke
f07fb5a55e2d5d87f288c988a24ae8b9e3f827a1 build: patch qt libpng to fix powerpc build (fanquake) Pull request description: This is an alternative to #19751 that fixes the build without requiring splitting out libpng. This patch can be dropped once we are building qt 5.12.0 or later. One of the [concerns posted in #19751](https://github.com/bitcoin/bitcoin/pull/19751#issuecomment-675455993) was: > the one bundled with Qt is at best a slower "bare minimum". However for our usage, I don't think the performance of libpng is a concern. If it is, I'd like to see some numbers/justification as to why we should be worried about it. This patch should be enough to unblock PowerPC binaries (combined with other changes) for 0.21.0, and for 0.22.0, when we switch to qt 5.15.x in depends, we should be able to drop it. Related upstream issue: https://bugreports.qt.io/browse/QTBUG-66388. ACKs for top commit: laanwj: ACK f07fb5a55e2d5d87f288c988a24ae8b9e3f827a1 theuni: ACK f07fb5a55e2d5d87f288c988a24ae8b9e3f827a1. hebasto: ACK f07fb5a55e2d5d87f288c988a24ae8b9e3f827a1, the patch is the same as https://codereview.qt-project.org/gitweb?p=qt/qtbase.git;a=blobdiff;f=src/3rdparty/libpng/libpng.pro;h=a2f56669b47e09629b7a88a0964091404d6a9b06;hp=577b61d833f7842f3f0d6b1c94a3b3920006695c;hb=dc2aead842f4cdf74f9259d3606c53c8bdae2c6b;hpb=ceeecbae510af6e2d1ebbf865761e4761d404033 Tree-SHA512: 865b843f5049eca80215774274fb7ae0dacccc3dd7f4a2eec93a73057115dcea85e715f919f96441424f9dd902dd97f0a238d96d4074babcee66b30577104009
2020-09-22doc: Document signet BIPMarcoFalke