Commit Graph

27434 Commits

Author SHA1 Message Date
Andrew Chow
e2f429e6bb wallet: Replace nFeeRateNeeded with effective_fee
Make sure that all fee calculations use the same feerate.
coin_selection_params.effective_fee is the variable we use for all fee
calculations, so get rid of remaining nFeeRateNeeded usages and just
directly set coin_selection_params.effective_fee.

Does not change behavior.
2021-03-16 12:32:56 -04:00
Andrew Chow
1a6a0b0dfb wallet: Use existing feerate instead of getting a new one
During each loop of CreateTransaction, instead of constantly getting a
new feerate, use the feerate that we have already fetched for all
fee calculations. Thix fixes a race condition where the feerate required
changes during each iteration of the loop.

This commit changes behavior as the "Fee estimation failed" error will
now take priority over "Signing transaction failed".
2021-03-16 12:30:01 -04:00
MarcoFalke
ea5a50f92a
Merge #21042: doc, test: Improve setup_clean_chain documentation
590bda79e8 scripted-diff: Remove setup_clean_chain if default is not changed (Fabian Jahr)
98892f39e3 doc: Improve setup_clean_chain documentation (Fabian Jahr)

Pull request description:

  The first commit improves documentation on setup_clean_chain which is misunderstood quite frequently. Most importantly it fixes the TestShell docs which are simply incorrect.

  The second commit removes the instances of `setup_clean_clain` in functional tests where it is not changing the default.

  This used to be part of #19168 which also sought to rename`setup_clean_chain`.

ACKs for top commit:
  jonatack:
    ACK 590bda79e8

Tree-SHA512: a7881186e65d31160b8f84107fb185973b37c6e50f190a85c6e2906a13a7472bb4efa9440bd37fe0a9ac5cd2d1e8559870a7e4380632d9a249eca8980b945f3e
2021-02-04 10:30:06 +01:00
MarcoFalke
f1239b70d1
Merge #21025: validation: Guard chainman chainstates with cs_main
20677ffa22 validation: Guard all chainstates with cs_main (Carl Dong)

Pull request description:

  ```
  This avoids a potential race-condition where a thread is reading the
  ChainstateManager::m_active_chainstate pointer while another one is
  writing to it. There is no portable guarantee that reading/writing the
  pointer is thread-safe.

  This is also done in way that mimics ::ChainstateActive(), so the
  transition from that function to this method is easy.

  More discussion:
  1. https://github.com/bitcoin/bitcoin/pull/20749#discussion_r559544027
  2. https://github.com/bitcoin/bitcoin/pull/19806#discussion_r561023961
  3. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768946522
  4. https://github.com/bitcoin/bitcoin/pull/19806#issuecomment-768955695
  ```

  Basically this PR removes the loaded-but-unfired footgun, which:
  - Is multiplied (but still unshot) in the chainman deglobalization PRs (#20158)
  - Is shot in the test framework in the au.activate PR (#19806)

ACKs for top commit:
  jnewbery:
    code review ACK 20677ffa22. I've verified by eye that neither of these members are accessed without cs_main.
  ryanofsky:
    Code review ACK 20677ffa22. It is safer to have these new `GUARDED_BY` annotations and locks than not to have them, but in the longer run I think every `LOCK(cs_main)` added here and added earlier in f92dc6557a from #20749 should be removed and replaced with `EXCLUSIVE_LOCKS_REQUIRED(cs_main)` on the accessor methods instead. `cs_main` is a high level lock that should be explicitly acquired at a high level to prevent the chain state from changing. It shouldn't be acquired recursively in low-level methods just to read pointer values atomically.

Tree-SHA512: 68a3a46d79a407b774eab77e1d682a97e95f1672db0a5fcb877572e188bec09f3a7b47c5d0cc1f2769ea276896dcbe97cb35c861acf7d8e3e513e955dc773f89
2021-02-04 10:22:44 +01:00
MarcoFalke
4e946ebcf1
Merge #20715: util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet
fa61b9d1a6 util: Add ArgsManager::GetCommand() and use it in bitcoin-wallet (MarcoFalke)
7777105a24 refactor: Move all command dependend checks to ExecuteWalletToolFunc (MarcoFalke)
fa06bce4ac test: Add tests (MarcoFalke)
fac05ccdad wallet: [refactor] Pass ArgsManager to WalletAppInit (MarcoFalke)

Pull request description:

  This not only moves the parsing responsibility out from the wallet tool, but it also makes it easier to implement bitcoin-util #19937

  Fixes: #20902

ACKs for top commit:
  ajtowns:
    ACK fa61b9d1a6
  fjahr:
    Code review ACK fa61b9d1a6

Tree-SHA512: 79622b806e8bf9dcd0dc24a8a6687345710df57720992e83a41cd8d6762a6dc112044ebc58fcf6e8fbf45de29a79b04873c5b8c2494a1eaaf902a2884703e47b
2021-02-04 09:12:05 +01:00
MarcoFalke
5a429d3d0f
Merge #21049: Add release notes for listdescriptors RPC
51f3752fbe Add release notes for listdescriptors RPC (Ivan Metlushko)

Pull request description:

  Original PR is #20226

ACKs for top commit:
  jonatack:
    ACK 51f3752
  jonasschnelli:
    ACK 51f3752fbe

Tree-SHA512: e8091d01b99a3effcd6c1738e7363a44858ba9bcf6bd99bf60f2025a25db83fc8d61354ab2002365b56071b9f3693c7d534153a259b5ebc91cbcf8d13f6555f1
2021-02-04 08:51:37 +01:00
fanquake
5f18080c29
Merge #21065: build: make macOS HOST in download-osx generic
f22a3ec140 build: make macOS HOST in download-osx generic (fanquake)

Pull request description:

  This was missed in #20419, and the update before that, so just make this non-versioned so that we don't have to worry about it. This is fine, because it's just for downloading sources.

ACKs for top commit:
  RandyMcMillan:
    ACK f22a3ec140
  dongcarl:
    utACK f22a3ec140

Tree-SHA512: 3a6993a69594a793a5185e4ba48858443a1002a37b96ff881d39ca7719c79432b35d709bd9a9379f8046bdbeb716c5e1598f273a7e7e3f3bf528b6a807abe5ec
2021-02-04 10:11:06 +08:00
fanquake
ea96e17e1f
Merge #21060: doc: More precise -debug and -debugexclude doc
572fd0f738 doc: More precise -debug and -debugexclude doc (wodry)

Pull request description:

  I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.

  This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.

ACKs for top commit:
  laanwj:
    ACK 572fd0f738
  MarcoFalke:
    ACK 572fd0f738
  theStack:
    ACK 572fd0f738

Tree-SHA512: 8d93db37602fd5ff4247e7c11478e55b99c0e3d47eaa2bb901937805d8f2a466b3a198b713b760981c5411576b74c52e2909c46c6d3f2e0e04215fd521b65cf7
2021-02-03 10:02:26 +08:00
fanquake
f22a3ec140
build: make macOS HOST in download-osx generic
This was missed in #20419, and the update before that, so just
make this un-versioned so that we don't have to worry about it.
This is fine, because it's just for downloading sources.
2021-02-02 20:28:17 +08:00
MarcoFalke
384e090f93
Merge #19509: Per-Peer Message Capture
bff7c66e67 Add documentation to contrib folder (Troy Giorshev)
381f77be85 Add Message Capture Test (Troy Giorshev)
e4f378a505 Add capture parser (Troy Giorshev)
4d1a582549 Call CaptureMessage at appropriate locations (Troy Giorshev)
f2a77ff97b Add CaptureMessage (Troy Giorshev)
dbf779d5de Clean PushMessage and ProcessMessages (Troy Giorshev)

Pull request description:

  This PR introduces per-peer message capture into Bitcoin Core.  📓

  ## Purpose

  The purpose and scope of this feature is intentionally limited.  It answers a question anyone new to Bitcoin's P2P protocol has had: "Can I see what messages my node is sending and receiving?".

  ## Functionality

  When a new debug-only command line argument `capturemessages` is set, any message that the node receives or sends is captured.  The capture occurs in the MessageHandler thread.  When receiving a message, it is captured as soon as the MessageHandler thread takes the message off of the vProcessMsg queue.  When sending, the message is captured just before the message is pushed onto the vSendMsg queue.

  The message capture is as minimal as possible to reduce the performance impact on the node.  Messages are captured to a new `message_capture` folder in the datadir.  Each node has their own subfolder named with their IP address and port.  Inside, received and sent messages are captured into two binary files, msgs_recv.dat and msgs_sent.dat, like so:

  ```
  message_capture/203.0.113.7:56072/msgs_recv.dat
  message_capture/203.0.113.7:56072/msgs_sent.dat
  ```

  Because the messages are raw binary dumps, included in this PR is a Python parsing tool to convert the binary files into human-readable JSON.  This script has been placed on its own and out of the way in the new `contrib/message-capture` folder.  Its usage is simple and easily discovered by the autogenerated `-h` option.

  ## Future Maintenance

  I sympathize greatly with anyone who says "the best code is no code".

  The future maintenance of this feature will be minimal.  The logic to deserialize the payload of the p2p messages exists in our testing framework.  As long as our testing framework works, so will this tool.

  Additionally, I hope that the simplicity of this tool will mean that it gets used frequently, so that problems will be discovered and solved when they are small.

  ## FAQ

  "Why not just use Wireshark"

  Yes, Wireshark has the ability to filter and decode Bitcoin messages.  However, the purpose of the message capture added in this PR is to assist with debugging, primarily for new developers looking to improve their knowledge of the Bitcoin Protocol.  This drives the design in a different direction than Wireshark, in two different ways.  First, this tool must be convenient and simple to use.  Using an external tool, like Wireshark, requires setup and interpretation of the results.  To a new user who doesn't necessarily know what to expect, this is unnecessary difficulty.  This tool, on the other hand, "just works".  Turn on the command line flag, run your node, run the script, read the JSON.  Second, because this tool is being used for debugging, we want it to be as close to the true behavior of the node as possible.  A lot can happen in the SocketHandler thread that would be missed by Wireshark.

  Additionally, if we are to use Wireshark, we are at the mercy of whoever it maintaining the protocol in Wireshark, both as to it being accurate and recent.  As can be seen by the **many** previous attempts to include Bitcoin in Wireshark (google "bitcoin dissector") this is easier said than done.

  Lastly, I truly believe that this tool will be used significantly more by being included in the codebase.  It's just that much more discoverable.

ACKs for top commit:
  MarcoFalke:
    re-ACK bff7c66e67 only some minor changes: 👚
  jnewbery:
    utACK bff7c66e67
  theStack:
    re-ACK bff7c66e67

Tree-SHA512: e59e3160422269221f70f98720b47842775781c247c064071d546c24fa7a35a0e5534e8baa4b4591a750d7eb16de6b4ecf54cbee6d193b261f4f104e28c15f47
2021-02-02 13:11:28 +01:00
Wladimir J. van der Laan
1e69800d5e
Merge #21059: Drop boost/preprocessor dependencies
e99db77a6e Drop boost/preprocessor dependencies (Hennadii Stepanov)
12f5028d49 refactor: Move STRINGIZE macro to macros.h (Hennadii Stepanov)

Pull request description:

  Use own macros instead of boost's ones.

ACKs for top commit:
  laanwj:
    Code review ACK e99db77a6e
  practicalswift:
    cr ACK e99db77a6e

Tree-SHA512: 7ec15c2780a661e293c990f64c41b5b451d894cc191aa7872fbcaf96da91915a351209b1f1003ab12a7a16cb464e50ac58a028db02beeedfa5f6931752c2d9e2
2021-02-02 12:44:27 +01:00
Ivan Metlushko
51f3752fbe Add release notes for listdescriptors RPC
Original PR is #20226
2021-02-02 08:21:46 +01:00
Carl Dong
20677ffa22 validation: Guard all chainstates with cs_main
Since these chainstates are:

1. Also vulnerable to the race condition described in the previous
   commit
2. Documented as having similar semantics as m_active_chainstate

we should also protect them with ::cs_main.
2021-02-01 22:09:03 -05:00
fanquake
f72d80b07a
Merge #21051: Fix -Wmismatched-tags warnings
b6aadcd5b4 build: Add -Werror=mismatched-tags (Hennadii Stepanov)
1485124291 Fix -Wmismatched-tags warnings (Hennadii Stepanov)

Pull request description:

  Warnings were introduced in #20749:
  ```
  ./validation.h:43:1: warning: class 'CCheckpointData' was previously declared as a struct; this is valid, but may result in linker errors under the Microsoft C++ ABI [-Wmismatched-tags]
  class CCheckpointData;
  ^
  ./chainparams.h:24:8: note: previous use is here
  struct CCheckpointData {
         ^
  ./validation.h:43:1: note: did you mean struct here?
  class CCheckpointData;
  ^~~~~
  struct
  1 warning generated.
  ```

  This change fixes AppVeyor build: https://ci.appveyor.com/project/DrahtBot/bitcoin/builds/37547435

ACKs for top commit:
  glozow:
    utACK b6aadcd5b4 🚗
  practicalswift:
    cr ACK b6aadcd5b4: patch looks correct

Tree-SHA512: 3ac887ebdbf9a1ae33c1fd5381b3b8d83388ad557ddeb55013acd42bb9752a5bd009e3a0eed52644a023a7a0dda1c159277981af82f58fb0abfe60b84e01bf29
2021-02-02 09:20:19 +08:00
Fabian Jahr
590bda79e8
scripted-diff: Remove setup_clean_chain if default is not changed
-BEGIN VERIFY SCRIPT-
git grep -l "self.setup_clean_chain = False" test/functional/*.py | xargs sed -i "/self.setup_clean_chain = False/d";
-END VERIFY SCRIPT-
2021-02-01 23:13:38 +01:00
Fabian Jahr
98892f39e3
doc: Improve setup_clean_chain documentation 2021-02-01 23:13:35 +01:00
Hennadii Stepanov
b6aadcd5b4
build: Add -Werror=mismatched-tags 2021-02-01 23:03:10 +02:00
wodry
572fd0f738
doc: More precise -debug and -debugexclude doc
I wondered how one could enable debug logging with `-debug=<category>` for multiple categories. Found out solution is to specify that option multiple times for each wanted category.

This PR documents this behavior and uses the same wording for the same behavior of `-debugexclude=<category>` to make that also clear and stringent.
2021-02-01 21:33:31 +01:00
Hennadii Stepanov
e99db77a6e
Drop boost/preprocessor dependencies 2021-02-01 22:30:06 +02:00
Hennadii Stepanov
12f5028d49
refactor: Move STRINGIZE macro to macros.h
This is a move-only change.
2021-02-01 22:30:05 +02:00
Wladimir J. van der Laan
2c0fc856a6
Merge #20464: refactor: Treat CDataStream bytes as uint8_t
fa29272459 Remove redundant MakeUCharSpan wrappers (MarcoFalke)
faf4aa2f47 Remove CDataStream::Init in favor of C++11 member initialization (MarcoFalke)
fada14b948 Treat CDataStream bytes as uint8_t (MarcoFalke)
fa8bdb048e refactor: Drop CDataStream constructors in favor of one taking a Span of bytes (MarcoFalke)
faa96f841f Remove unused CDataStream methods (MarcoFalke)

Pull request description:

  Using `uint8_t` for raw bytes has a style benefit:
  * The signedness is clear from reading the code, as it does not depend on the architecture

  Other clean-ups in this pull include:
  * Remove unused methods
  * Constructor is simplified with `Span`
  * Remove `Init()` member in favor of C++11 member initialization

ACKs for top commit:
  laanwj:
    code review ACK fa29272459
  theStack:
    ACK fa29272459 🍾

Tree-SHA512: 931ee28bd99843d7e894b48e90e1187ffb0278677c267044b3c0c255069d9bbd9298ab2e539b1002a30b543d240450eaec718ef4ee95a7fd4be0a295e926343f
2021-02-01 15:17:28 +01:00
Wladimir J. van der Laan
56fcf93349
Merge #21026: doc: Document use of make-tag script to make tags
cc30dfbd4b doc: Document use of make-tag script to make tags (Wladimir J. van der Laan)

Pull request description:

  To make release tags the `make-tag.py` script from the maintainer tools should be used. This ensures that all the various occurrences of the version in different files match the tagged version before proceeding. And move it into a separate section with the other per-release actions.

  Also replace other "ping wumpus" references.

ACKs for top commit:
  jonatack:
    ACK cc30dfbd4b

Tree-SHA512: c09748a0bea85573b3f04fdb86430a53b683ff4d956edc1f49d471e2e526715cbde7cf6ad83a43a1a3fca4ff3c5af011e3d1b8cb61f5d8e065cfa71ba0138c88
2021-02-01 14:27:09 +01:00
Hennadii Stepanov
1485124291
Fix -Wmismatched-tags warnings 2021-02-01 14:37:14 +02:00
Wladimir J. van der Laan
d0d256536c
Merge #21016: refactor: remove boost::thread_group usage
dc8be12510 refactor: remove boost::thread_group usage (fanquake)

Pull request description:

  Post #18710, there isn't much left using `boost::thread_group`, so should just be able to replace it with the standard library. This also removes the last use of `boost::thread_interrupted`.

  After this change, last piece of Boost Thread we'd be using is `boost::shared_mutex`. See the commentary [here](https://github.com/bitcoin/bitcoin/issues/16684#issuecomment-726214696) as to why it may be non-trivial to swap that for `std::shared_mutex` in the near future.

  Closes #17307

ACKs for top commit:
  laanwj:
    Code review re-ACK dc8be12510
  MarcoFalke:
    review ACK dc8be12510 🔁
  jonatack:
    Non-expert code review ACK dc8be12510, also checked range-diff since last review and that local debug build is clean with gcc 10.2.1-6 on Debian

Tree-SHA512: 5510e2d760cce824234207dc86b1551ca8f21cbf3a2ce753c0254a0d03ffd83c94e449aec202fb7bd76e6fc64df783a6b70a736b0add9ece3734bb9c8ce8fc2f
2021-02-01 13:27:28 +01:00
Wladimir J. van der Laan
44f4bcd302
Merge #20749: [Bundle 1/n] Prune g_chainman usage related to ::LookupBlockIndex
67c9a83df1 style-only: Remove redundant sentence in ActivateBestChain comment (Carl Dong)
b8e95658d5 style-only: Make TestBlockValidity signature readable (Carl Dong)
0cdad75390 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock (Carl Dong)
ea4fed9021 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders (Carl Dong)
e0dc305727 validation: Move LoadExternalBlockFile to CChainState (Carl Dong)
5f8cd7b3a5 validation: Remove global ::ActivateBestChain (Carl Dong)
2a696472a1 validation: Pass in chainstate to ::NotifyHeaderTip (Carl Dong)
9c300cc8b3 validation: Pass in chainstate to TestBlockValidity (Carl Dong)
0e17c833cd validation: Make CChainState.m_blockman public (Carl Dong)
d363d06bf7 validation: Pass in blockman to ContextualCheckBlockHeader (Carl Dong)
f11d11600d validation: Move GetLastCheckpoint to BlockManager (Carl Dong)
e4b95eefbc validation: Move GetSpendHeight to BlockManager (Carl Dong)
b026e318c3 validation: Move FindForkInGlobalIndex to BlockManager (Carl Dong)
3664a150ac validation: Remove global LookupBlockIndex (Carl Dong)
eae54e6e60 scripted-diff: Use BlockManager::LookupBlockIndex (Carl Dong)
15d20f40e1 validation: Move LookupBlockIndex to BlockManager (Carl Dong)
f92dc6557a validation: Guard the active_chainstate with cs_main (Carl Dong)

Pull request description:

  Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)

  Note to reviewers:
  1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
  3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
  1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
  2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
  3. Remove `old_function`

ACKs for top commit:
  jnewbery:
    utACK 67c9a83df1
  laanwj:
    re-ACK 67c9a83df1
  ryanofsky:
    Code review ACK 67c9a83df1. Changes since last review:

Tree-SHA512: 8744aba2dd57a40cd2fedca809b0fe24d771bc60da1bffde89601999384aa0df428057a86644a3f72fbeedbc8b04db6c4fd264ea0db2e73c279e5acc6d056cbf
2021-02-01 13:09:46 +01:00
MarcoFalke
636e754a81
Merge #20941: rpc: document RPC_TRANSACTION_ALREADY_IN_CHAIN exception
74d23bf7fb rpc: document RPC_TRANSACTION_ALREADY_IN_CHAIN exception (Jarol Rodriguez)

Pull request description:

  It is not documented in the `RPCHelpMan` of `sendrawtransaction` that if you attempt to send a transaction which already exists in a block, an `RPC_TRANSACTION_ALREADY_IN_CHAIN` exception will be raised. It is best to make developers aware of this so that it can be properly caught and avoid any headaches.

  Closes #5638

ACKs for top commit:
  jonatack:
    ACK 74d23bf7fb

Tree-SHA512: d1d5fc242574377c8a76b4ef7b12239996424d8bee186533b5a8fe337bbeb3186e51dbdd28c5eafb982601e44e17b68a7f52db5dd7bc647429f6f95e2de289f6
2021-02-01 10:59:50 +01:00
MarcoFalke
87394b6741
Merge #20868: validation: remove redundant check on pindex
c943282b5e validation: remove redundant check on pindex (jarolrod)

Pull request description:

  This removes a redundant check on `pindex` being a `nullptr`. By the time we get to this step `pindex` is always a `nullptr` as the branch where it has been set would have already returned.

  Closes #19223

ACKs for top commit:
  Zero-1729:
    re-ACK c943282
  ajtowns:
    ACK c943282b5e - code review only
  MarcoFalke:
    review ACK c943282b5e 📨
  theStack:
    re-ACK c943282b5e

Tree-SHA512: d2dc58206be61d2897b0703ee93af67abed492a80e84ea03dcfbf7116833173da3bdafa18ff80422d5296729d5254d57cc1db03fdaf817c8f57c62c3abef673c
2021-02-01 10:56:23 +01:00
Samuel Dobson
7dc4807691
Merge #20040: wallet: Refactor OutputGroups to handle fees and spending eligibility on grouping
5d4597666d Rewrite OutputGroups to be clearer and to use scriptPubKeys (Andrew Chow)
f6b3052739 Explicitly filter out partial groups when we don't want them (Andrew Chow)
416d74fb16 Move OutputGroup positive only filtering into Insert (Andrew Chow)
d895e98b59 Move EligibleForSpending into GroupOutputs (Andrew Chow)
99b399aba5 Move fee setting of OutputGroup to Insert (Andrew Chow)
6148a8acda Move GroupOutputs into SelectCoinsMinConf (Andrew Chow)
2acad03657 Remove OutputGroup non-default constructors (Andrew Chow)

Pull request description:

  Even after #17458, we still deal with setting fees of an `OutputGroup` and filtering the `OutputGroup` outside of the struct. We currently make all of the `OutputGroup`s in `SelectCoins` and then copy and modify them within each `SelectCoinsMinConf` scenario. This PR changes this to constructing the `OutputGroup`s within the `SelectCoinsMinConf` so that the scenario can be taken into account during the group construction. Furthermore, setting of fees and filtering for effective value is moved into `OutputGroup::Insert` itself so that we don't add undesirable outputs to an `OutputGroup` rather than deleting them afterwards.

  To facilitate fee calculation and effective value filtering during `OutputGroup::Insert`, `OutputGroup` now takes the feerates in its constructor and computes the fees and effective value for each output during `Insert`.

  While removing `OutputGroup`s in accordance with the `CoinEligibilityFilter` still requires creating the `OutputGroup`s first, we can do that within the function that makes them - `GroupOutput`s.

ACKs for top commit:
  Xekyo:
    Code review ACK: 5d4597666d
  fjahr:
    Code review ACK 5d4597666d
  meshcollider:
    Light utACK 5d4597666d

Tree-SHA512: 35965b6d49a87f4ebb366ec4f00aafaaf78e9282481ae2c9682b515a3a9f2cbcd3cd6e202fee29489d48fe7f3a7cede4270796f5e72bbaff76da647138fb3059
2021-02-01 22:43:17 +13:00
MarcoFalke
4c55f92c76
Merge #20954: test: Declare nodes type in test_framework.py.
5353b0c64d Change type definitions for "chain" and "setup_clean_chain" from type comments to Python 3.6+ types. Additionally, set type for "nodes". (Kiminuo)

Pull request description:

  ### Motivation

  When I wanted to understand better https://github.com/bitcoin/bitcoin/pull/19145/files#diff-4bebbd3b112dc222ea7e75ef051838ceffcee63b9e9234a98a4cc7251d34451b test, I noticed that navigation in PyCharm/VS Code did not work for `nodes` variable. I think this is frustrating, especially for newcomers.

  ### Summary

  * This PR modifies Python 3.5 [type comments](https://mypy.readthedocs.io/en/stable/cheat_sheet_py3.html#variables) to Python 3.6+ types and adds a proper type for `nodes` [instance attribute](https://mypy.readthedocs.io/en/stable/class_basics.html#instance-and-class-attributes).
  * This PR does not change behavior.
  * This PR is intentionally very small, if the concept is accepted, a follow-up PRs can be more ambitious.

  ### End result

  1. Open `test/functional/feature_abortnode.py`
  2. Move your caret to: `self.nodes[0].generate[caret here](3)`
  3. Use "Go to definition" [F12] should work now.

  I have tested this on PyCharm (Windows, Ubuntu) and VS Code (Windows, Ubuntu).

  Note: Some `TestNode` methods (e.g. `self.nodes[0].getblock(...)` ) use `__call__` mechanism and navigation does not work for them even with this PR.

ACKs for top commit:
  laanwj:
    ACK 5353b0c64d
  theStack:
    ACK 5353b0c64d

Tree-SHA512: 821773f052ab9b2889dc357d38c59407a4af09e3b86d7134fcca7d78e5edf3a5ede9bfb37595ea97caf9ebfcbda372bcf73763b7f89b0677670f21b3e396a12b
2021-01-31 09:28:21 +01:00
MarcoFalke
1f514332ed
Merge #21037: fuzz: Avoid designated initialization (C++20) in fuzz tests
dee2d6fbf9 fuzz: Avoid designated initialization (C++20) in fuzz tests (practicalswift)

Pull request description:

  Avoid designated initialization (C++20) in fuzz tests.

  Context: https://github.com/bitcoin/bitcoin/pull/20197#discussion_r565270556, https://github.com/bitcoin/bitcoin/pull/20936#discussion_r566708730

ACKs for top commit:
  MarcoFalke:
    cr ACK dee2d6fbf9
  dhruv:
    code review ACK dee2d6fbf9
  ajtowns:
    utACK dee2d6fbf9

Tree-SHA512: 5940fab6e97a2b11dd3b1475d2cffa2840dc2e6ec34bd9f9df90f948709cab98fd1c513d5dd104816d33a525a6e9710b8715b02db941e35d84f92bc211f56d1d
2021-01-31 08:43:54 +01:00
practicalswift
dee2d6fbf9 fuzz: Avoid designated initialization (C++20) in fuzz tests 2021-01-30 08:22:32 +00:00
Wladimir J. van der Laan
16b784d953
Merge #20689: contrib: replace binary verification script verify.sh with python rewrite
c86b9a65eb contrib: remove verify.sh (Sebastian Falbesoner)
c84838e7af contrib: binary verification script verify.sh rewritten in python (Sebastian Falbesoner)

Pull request description:

  The rationale for the PR is the same as for #18132:
  > Most of our test scripts are written in python. We don't have enough reviewers for bash scripts and they tend to be clumsy anyway. Especially when it comes to argument parsing.

  Note that there are still a lot of things that could be improved in this replacement (e.g. using regexps for version string parsing, adding type annotations, dividing up into more functions, getting a pylint score closer to 10, etc.), but I found the original shell script quite hard to read, so it's possibly still a good first step for an improvement.
  ~Not sure though if it's worth the reviewers time, and if it's even continued to be used long-term (maybe there are plans to merge it with `get_previous_releases.py`, which partly does the same?), so chasing for Concept ACKs right now.~

ACKs for top commit:
  laanwj:
    Tested and code review ACK c86b9a65eb

Tree-SHA512: f7949eead4ef7e5913fe273923ae5c5299408db485146cf996cdf6f8ad8c0ee4f4b30bb6b08a5964000d97b2ae2e7a1bdc88d11c613c16d2d135d80b444e3b16
2021-01-29 20:08:22 +01:00
Wladimir J. van der Laan
cc30dfbd4b doc: Document use of make-tag script to make tags
To make release tags the `make-tag.py` script from the maintainer tools
should be used. This ensures that all the various occurences of the
version in different files match the tagged version before proceeding.

Also replace other "ping wumpus" references.
2021-01-29 08:46:11 +01:00
fanquake
dc8be12510
refactor: remove boost::thread_group usage 2021-01-29 15:39:44 +08:00
MarcoFalke
c8b83510f4
Merge #20724: Cleanup of -debug=net log messages
48c8a9b964 net_processing: log txrelay flag from version message (Anthony Towns)
98fab37ca0 net: use peer=N instead of from=N in debug log (Anthony Towns)
12302105bb net_processing: additional debug logging for ignored messages (Anthony Towns)
f7edea3b7c net: make debug logging conditional on -debug=net (Anthony Towns)
a410ae8cb0 net, net_processing: log disconnect reasons with -debug=net (Anthony Towns)

Pull request description:

  A few changes to -debug=net logging:

   * always log when disconnecting a peer
   * only log various connection errors when -debug=net is enabled, since errors from random untrusted peers is completely expected
   * log when ignoring a message due to violating protocol (primarily to make it easier to debug other implementations)
   * use "peer=123" rather than "from 123" to make grepping logs a bit easier
   * log the value of the bip-37 `fRelay` field in version messages both when sending and receiving a version message

ACKs for top commit:
  jnewbery:
    ACK 48c8a9b964
  MarcoFalke:
    re-ACK 48c8a9b964 only change is rebase 🚓
  practicalswift:
    re-ACK 48c8a9b964

Tree-SHA512: 6ac530d883dffc4fd7fe20b1dc5ebb5394374c9b499aa7a253eb4a3a660d8901edd72e5ad21ce4a2bf71df25e8f142087755f9756f3497f564ef453a7e9246c1
2021-01-29 07:44:22 +01:00
MarcoFalke
40dd757bf6
Merge #21012: ci: Fuzz with integer sanitizer
faff3991a9 ci: Fuzz with integer sanitizer (MarcoFalke)

Pull request description:

  Otherwise the suppressions file will go out of sync

ACKs for top commit:
  practicalswift:
    cr ACK faff3991a9: patch looks correct

Tree-SHA512: 349216d071a2c5ccf24565fe0c52d7a570ec148d515d085616a284f1ab9992ce10ff82eb17962dddbcda765bbd3a9b15e8b25f34bdbed99fc36922d4161d307c
2021-01-29 07:43:17 +01:00
MarcoFalke
bc5f26d4ee
Merge bitcoin-core/gui#139: doc: Improve gui/src/qt README.md
5d1f260713 Improve gui/src/qt README.md (Jarol Rodriguez)

Pull request description:

  **Master/Before:** [Render of Master](https://github.com/bitcoin-core/gui/blob/master/src/qt/README.md)

  **PR/After:** [Render of PR](5d1f260713/src/qt/README.md)

  **Changes:**
  The README.md found in `gui/src/qt` seems to not have gotten any love in a while. This PR fixes some grammatical errors, makes it easier to follow, and modernizes the logic of using Qt Creator.
  1. Makes several sections more informative
  2. Directories under `Files and Directories` now end with a forward slash denoting that they are a directory
  3. Modernize the Qt Creator Logic for the current setup flow
  4. Add UNIX Qt Creator Setup Instructions (Ubuntu & Debian)

ACKs for top commit:
  RandyMcMillan:
    ACK 5d1f260 👍🏼
  jonatack:
    ACK 5d1f260713

Tree-SHA512: bd5cc24b95460f34a1efa489a6acc10d7632c84eabdcdc5729ef077d8303cf037ed664aae033af8883252433ea0999d8ec7d92e8b03d03a873d32b041a94f813
2021-01-29 07:40:44 +01:00
fanquake
e213382920
Merge #21029: bitcoin-cli: Correct docs (no "generatenewaddress" exists)
71430aec43 bitcoin-cli: Correct docs (no "generatenewaddress" exists) (Luke Dashjr)

Pull request description:

ACKs for top commit:
  jonatack:
    ACK 71430aec43

Tree-SHA512: 45ee7a51080598141fac4446563bdf99a59bfaa0ee00d9e092511087a968e8535d9208683ed589be1c975531be6d0319d33720f1f0dc1c3635c7d5ea6d726a41
2021-01-29 11:36:57 +08:00
Luke Dashjr
71430aec43 bitcoin-cli: Correct docs (no "generatenewaddress" exists) 2021-01-29 00:38:57 +00:00
Wladimir J. van der Laan
6a726cb534
Merge #20963: gitian-linux: Build binaries for 64-bit POWER (continued)
543bf745d3 gitian-linux: Extend noexec-stack workaround to powerpc (Wladimir J. van der Laan)
00f67c8aa1 gitian-linux: Build binaries for 64-bit POWER (Luke Dashjr)
63fc2b1782 gitian: Properly quote arguments in wrappers (Luke Dashjr)
798bc0b29a Support glibc-back-compat on 64-bit POWER (Luke Dashjr)

Pull request description:

  Rebase of #14066 by luke-jr.

  Let's try to get PowerPC support in in the beginning of the 22.0 cycle so that it gets some testing, and is not a last-minute decision this time, like for last … 2 or 3 major versions.

  The symbol/security tooling-related changes have been dropped since they were part of #20434.

Top commit has no ACKs.

Tree-SHA512: df0f8cd320c90f359f8b512c5cb8b59bb277516b57a05482cc8923c656106513b7428e315aaa8ab53e0bd6f80556b07d3639c47f6d9913bcfbfe388b39ef47c4
2021-01-28 22:43:10 +01:00
Carl Dong
67c9a83df1 style-only: Remove redundant sentence in ActivateBestChain comment 2021-01-28 14:15:26 -05:00
Carl Dong
b8e95658d5 style-only: Make TestBlockValidity signature readable 2021-01-28 14:15:26 -05:00
Carl Dong
0cdad75390 validation: Use accessible chainstate in ChainstateManager::ProcessNewBlock
[META] This commit should be followed up by removing the comments and
       assertions meant only to show that the change is correct.
2021-01-28 14:15:26 -05:00
Carl Dong
ea4fed9021 validation: Use existing chainstate in ChainstateManager::ProcessNewBlockHeaders
[META] This commit should be followed up by removing the comments and
       assertions meant only to show that the change is correct.
2021-01-28 14:15:26 -05:00
Carl Dong
e0dc305727 validation: Move LoadExternalBlockFile to CChainState
[META] This commit should be followed up by removing the comments and
       assertions meant only to show that the change is correct.

LoadExternalBlockFile mainly acts on CChainState.
2021-01-28 14:15:26 -05:00
Carl Dong
5f8cd7b3a5 validation: Remove global ::ActivateBestChain
Instead use CChainState::ActivateBestChain, which is what the global one
calls anyway.
2021-01-28 14:15:26 -05:00
Carl Dong
2a696472a1 validation: Pass in chainstate to ::NotifyHeaderTip
[META] This commit should be followed up by removing the comments and
       assertions meant only to show that the change is correct.
2021-01-28 14:15:26 -05:00
Carl Dong
9c300cc8b3 validation: Pass in chainstate to TestBlockValidity
[META] This commit should be followed up by removing the comments and
       assertions meant only to show that the change is correct.
2021-01-28 14:15:26 -05:00
Carl Dong
0e17c833cd validation: Make CChainState.m_blockman public 2021-01-28 14:15:26 -05:00
Carl Dong
d363d06bf7 validation: Pass in blockman to ContextualCheckBlockHeader
[META] This commit should be followed up by removing the comments and
       assertions meant only to show that the change is correct.
2021-01-28 14:15:26 -05:00