From 86eff460781a249ccebb1e4656c3c0a00fde77c9 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 4 Apr 2018 17:26:48 -0400 Subject: [PATCH 1/9] Merge #12702: [wallet] [rpc] [doc] importprivkey: hint about importmulti 4e05687153 [wallet] [rpc] [doc] importprivkey: hint about importmulti (Karl-Johan Alm) Pull request description: From #12701, a hint about `importmulti` inside the help for `importprivkey` seems useful. Tree-SHA512: 09ddfd384062b4365f678167076cb9f5af1eb8f083714a20c2a9bb14fef1c886d1666196272bf09862537166d15ae89c3330cdc6836eee76cb54d137e53301df --- src/wallet/rpcdump.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 77686b89dc..68cee2e6fd 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -81,6 +81,7 @@ UniValue importprivkey(const JSONRPCRequest& request) throw std::runtime_error( "importprivkey \"privkey\" ( \"label\" ) ( rescan )\n" "\nAdds a private key (as returned by dumpprivkey) to your wallet. Requires a new wallet backup.\n" + "Hint: use importmulti to import more than one private key.\n" "\nArguments:\n" "1. \"privkey\" (string, required) The private key (see dumpprivkey)\n" "2. \"label\" (string, optional, default=\"\") An optional label\n" From 5d007d8b3fc458fd8804c6dec0bec7ef662244a6 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Sat, 7 Apr 2018 16:42:22 +0200 Subject: [PATCH 2/9] Merge #12896: docs: Fix conflicting statements about initialization in developer notes b119e78 docs: Fix conflicting statements about initialization in developer notes (practicalswift) Pull request description: Fix conflicting statements about initialization in developer notes. Context: https://github.com/bitcoin/bitcoin/pull/12785#issuecomment-378941151 Tree-SHA512: 601b18cbeb963f99a4180e652d6c1b78210df89743fd3565c0bce95fd2dcc9784b6af212795a43d3a40a5858b1a03e0d2c7982295c92d6ea710db0e6ee69f0b4 --- doc/developer-notes.md | 27 ++++++++++++--------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index f1e52a0779..d3f2aac8c1 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -375,12 +375,21 @@ C++ data structures - Vector bounds checking is only enabled in debug mode. Do not rely on it -- Make sure that constructors initialize all fields. If this is skipped for a - good reason (i.e., optimization on the critical path), add an explicit - comment about this +- Initialize all non-static class members where they are defined. + If this is skipped for a good reason (i.e., optimization on the critical + path), add an explicit comment about this - *Rationale*: Ensure determinism by avoiding accidental use of uninitialized values. Also, static analyzers balk about this. + Initializing the members in the declaration makes it easy to + spot uninitialized ones. + +```cpp +class A +{ + uint32_t m_count{0}; +} +``` - By default, declare single-argument constructors `explicit`. @@ -399,18 +408,6 @@ C++ data structures - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those that are not language lawyers -- Initialize all non-static class members where they are defined - - - *Rationale*: Initializing the members in the declaration makes it easy to spot uninitialized ones, - and avoids accidentally reading uninitialized memory - -```cpp -class A -{ - uint32_t m_count{0}; -} -``` - Strings and formatting ------------------------ From ea5c11abf2cae702b5d6961773fe1f3e798e726b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 9 Apr 2018 08:04:08 -0400 Subject: [PATCH 3/9] Merge #12007: [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit c198dc00e1 [Doc] Clarify the meaning of fee delta not being a fee rate in prioritisetransaction RPC (Jan Čapek) Pull request description: Hi, I have faced some confusion among our developers considering this being a fee rate. Would you consider including this tiny doc update? Best regards, Jan Capek Tree-SHA512: cd0560540418e53c5c19ceab2d5aca229f4ef6b788b9543695742522e1c63a7f2cce2574b47fead098a106da2f77e297f0c728474565f6259b50d62369bbe7da --- src/rpc/mining.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b5a26218c3..fae1da438c 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -245,6 +245,7 @@ UniValue prioritisetransaction(const JSONRPCRequest& request) "\nArguments:\n" "1. \"txid\" (string, required) The transaction id.\n" "2. fee_delta (numeric, required) The fee value (in duffs) to add (or subtract, if negative).\n" + " Note, that this value is not a fee rate. It is a value to modify absolute fee of the TX.\n" " The fee is not actually paid, only the algorithm for selecting transactions into a block\n" " considers the transaction as it would have paid a higher (or lower) fee.\n" "\nResult:\n" From ff2901e6b34dcf4ebdc4394220141804c63147d3 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 9 Apr 2018 19:09:54 -0400 Subject: [PATCH 4/9] Merge #12927: Docs: fixed link, replaced QT with Qt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7039319db5 Docs: fixed link, replaced QT with Qt (Darko Janković) Pull request description: Tree-SHA512: 6c071189b4c030d03d3d09535333d2ed7115fba07ee2561591124c2063041966cc8012e4d8416c3dda155f2df5e15b8f772712cac35b4d266b50c48f4d74b6e4 --- src/qt/README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/qt/README.md b/src/qt/README.md index 888f8f7c85..c5ec232201 100644 --- a/src/qt/README.md +++ b/src/qt/README.md @@ -1,6 +1,6 @@ -This directory contains the DashQT graphical user interface (GUI). It uses the cross platform framework [QT](https://www1.qt.io/developers/). +This directory contains the DashQT graphical user interface (GUI). It uses the cross-platform framework [Qt](https://www1.qt.io/developers/). -The current precise version for QT 5 is specified in [qt.mk](/depends/packages/qt.mk). QT 4 is not supported. +The current precise version for Qt 5 is specified in [qt.mk](/depends/packages/qt.mk). Qt 4 is not supported. ## Compile and run @@ -36,7 +36,7 @@ Represents the main window of the Dash UI. ### \*model.(h/cpp) -The model. When it has a corresponding controller, it generally inherits from [QAbstractTableModel](http://doc.qt.io/qt-5/qabstracttablemodel.html). Models that are used by controllers as helpers inherit from other QT classes like [QValidator](http://doc.qt.io/qt-5/qvalidator.html). +The model. When it has a corresponding controller, it generally inherits from [QAbstractTableModel](http://doc.qt.io/qt-5/qabstracttablemodel.html). Models that are used by controllers as helpers inherit from other Qt classes like [QValidator](http://doc.qt.io/qt-5/qvalidator.html). ClientModel is used by the main application `bitcoingui` and several models like `peertablemodel`. @@ -69,7 +69,7 @@ Represents the view to a single wallet. ## Contribute -See [CONTRIBUTING.md](/CONTRIBUTING.md) for general guidelines. Specifically for QT: +See [CONTRIBUTING.md](/CONTRIBUTING.md) for general guidelines. Specifically for Qt: * don't change `local/dash_en.ts`; this happens [automatically](/doc/translation_process.md#writing-code-with-translations) @@ -83,7 +83,7 @@ Uncheck everything except Qt Creator during the installation process. Instructions for OSX: -1. Make sure you installed everything through Homebrew mentioned in the [OSX build instructions](/docs/build-osx.md) +1. Make sure you installed everything through Homebrew mentioned in the [OSX build instructions](/doc/build-osx.md) 2. Use `./configure` with the `--enable-debug` flag 3. In Qt Creator do "New Project" -> Import Project -> Import Existing Project 4. Enter "dash-qt" as project name, enter src/qt as location From 7b091fb99fe67a5b6c49042b9588bca9dfc87f58 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 11 Apr 2018 10:45:45 -0400 Subject: [PATCH 5/9] Merge #12933: doc: Refine header include policy fad0fc3c9a Refine travis check for duplicate includes (MarcoFalke) Pull request description: Since there is no harm in having "duplicate" includes and it makes it obvious what are the dependencies of each file, without having to do static analysis or jumping between files, I'd suggest to revert the travis check for duplicate includes. Generally, I think that enforcing minor style preferences should not be done via travis. The cost of maintaining and the burden on other developers is too high. C.f discussion in https://github.com/bitcoin/bitcoin/pull/10973#discussion_r180142594 Tree-SHA512: 97ab0e769d457ccfb873fff6c99613f8b944cd7ef95bfdccb0e1bbe8f5df1f16548c658fa03af42516f806546e75646d338a061e7b057619490235d311ca21f1 --- contrib/devtools/lint-includes.sh | 11 ----------- doc/developer-notes.md | 3 +-- 2 files changed, 1 insertion(+), 13 deletions(-) diff --git a/contrib/devtools/lint-includes.sh b/contrib/devtools/lint-includes.sh index 12c9985d95..4ed8299021 100755 --- a/contrib/devtools/lint-includes.sh +++ b/contrib/devtools/lint-includes.sh @@ -24,17 +24,6 @@ for HEADER_FILE in $(filter_suffix h); do echo EXIT_CODE=1 fi - CPP_FILE=${HEADER_FILE/%\.h/.cpp} - if [[ ! -e $CPP_FILE ]]; then - continue - fi - DUPLICATE_INCLUDES_IN_HEADER_AND_CPP_FILES=$(grep -hE "^#include " <(sort -u < "${HEADER_FILE}") <(sort -u < "${CPP_FILE}") | grep -E "^#include " | sort | uniq -d) - if [[ ${DUPLICATE_INCLUDES_IN_HEADER_AND_CPP_FILES} != "" ]]; then - echo "Include(s) from ${HEADER_FILE} duplicated in ${CPP_FILE}:" - echo "${DUPLICATE_INCLUDES_IN_HEADER_AND_CPP_FILES}" - echo - EXIT_CODE=1 - fi done for CPP_FILE in $(filter_suffix cpp); do diff --git a/doc/developer-notes.md b/doc/developer-notes.md index d3f2aac8c1..47e0532342 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -526,8 +526,7 @@ Source code organization avoided when using only lowercase characters in source code filenames. - Every `.cpp` and `.h` file should `#include` every header file it directly uses classes, functions or other - definitions from, even if those headers are already included indirectly through other headers. One exception - is that a `.cpp` file does not need to re-include the includes already included in its corresponding `.h` file. + definitions from, even if those headers are already included indirectly through other headers. - *Rationale*: Excluding headers because they are already indirectly included results in compilation failures when those indirect dependencies change. Furthermore, it obscures what the real code From fbc1bfd1d5c119dbb5492a29fca662e331c0bf84 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 16 Apr 2018 08:37:44 +0200 Subject: [PATCH 6/9] Merge #12951: [doc] Fix comment in FindForkInGlobalIndex 0ef7b40 [doc] Fix comment in FindForkInGlobalIndex (James O'Beirne) Pull request description: The comment erroneously implies that we're searching `chainActive` for the first block common to `locator`, but we're using the parameter `chain`. Tree-SHA512: 42ba0fb378597820bdf1eaff1e3e284097baa312e7dd8448421c8c71aa91c353ea6c840860afcb7725f392431f3134d4feb271b96ab7058a62f84f48e468e714 --- src/validation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 91531b4bef..73d39f8193 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -283,7 +283,8 @@ namespace { CBlockIndex* FindForkInGlobalIndex(const CChain& chain, const CBlockLocator& locator) { - // Find the first block the caller has in the main chain + // Find the latest block common to locator and chain - we expect that + // locator.vHave is sorted descending by height. for (const uint256& hash : locator.vHave) { BlockMap::iterator mi = mapBlockIndex.find(hash); if (mi != mapBlockIndex.end()) From a9822d4a34d7dfbe743cab27dfeee714e9d2e034 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Mon, 23 Apr 2018 17:08:34 +0200 Subject: [PATCH 7/9] Merge #13012: [doc] Add comments for chainparams.h, validation.cpp 18326ae [doc] Add comments for chainparams.h, validation.cpp (James O'Beirne) Pull request description: Added a few comments during a leisurely read through some of the validation code. If this kind of thing seems useful, I can add similar documentation for most of the `CChainState` interface. Tree-SHA512: a4d9db60383a8ff02e74ac326ed88902eec1ee441e8cd4e1845bcf257072673c15974225288cebf0a633e76a3410f99e2206616b4694725a2a5b0d19c78327d6 --- src/chainparams.h | 6 ++++++ src/validation.cpp | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/chainparams.h b/src/chainparams.h index 3a216cb0fa..f3a19f050b 100644 --- a/src/chainparams.h +++ b/src/chainparams.h @@ -25,6 +25,12 @@ struct CCheckpointData { MapCheckpoints mapCheckpoints; }; +/** + * Holds various statistics on transactions within a chain. Used to estimate + * verification progress during chain sync. + * + * See also: CChainParams::TxData, GuessVerificationProgress. + */ struct ChainTxData { int64_t nTime; int64_t nTxCount; diff --git a/src/validation.cpp b/src/validation.cpp index 73d39f8193..227ca4f229 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -166,6 +166,10 @@ public: bool ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock); + /** + * If a block header hasn't already been seen, call CheckBlockHeader on it, ensure + * that it doesn't descend from an invalid block, and then add it to mapBlockIndex. + */ bool AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main); bool AcceptBlock(const std::shared_ptr& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main); @@ -198,6 +202,11 @@ private: CBlockIndex* AddToBlockIndex(const CBlockHeader& block, enum BlockStatus nStatus = BLOCK_VALID_TREE) EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Create a new block index entry for a given block hash */ CBlockIndex* InsertBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + /** + * Make various assertions about the state of the block index. + * + * By default this only executes fully when using the Regtest chain; see: fCheckBlockIndex. + */ void CheckBlockIndex(const Consensus::Params& consensusParams); void InvalidBlockFound(CBlockIndex *pindex, const CValidationState &state); @@ -2937,6 +2946,10 @@ static void NotifyHeaderTip() LOCKS_EXCLUDED(cs_main) { * Make the best chain active, in multiple steps. The result is either failure * or an activated best chain. pblock is either nullptr or a pointer to a block * that is already loaded (to avoid loading it again from disk). + * + * ActivateBestChain is split into steps (see ActivateBestChainStep) so that + * we avoid holding cs_main for an extended period of time; the length of this + * call may be quite long during reindexing or a substantial reorg. */ bool CChainState::ActivateBestChain(CValidationState &state, const CChainParams& chainparams, std::shared_ptr pblock) { // Note that while we're often called here from ProcessNewBlock, this is @@ -3607,6 +3620,9 @@ bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& if (!ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime())) return error("%s: Consensus::ContextualCheckBlockHeader: %s, %s", __func__, hash.ToString(), FormatStateMessage(state)); + // If the previous block index isn't valid, determine if it descends from any block which + // has been found invalid (g_failed_blocks), then mark pindexPrev and any blocks + // between them as failed. if (!pindexPrev->IsValid(BLOCK_VALID_SCRIPTS)) { for (const CBlockIndex* failedit : g_failed_blocks) { if (pindexPrev->GetAncestor(failedit->nHeight) == failedit) { From 16f1845ed3ef4627adbff0147802339b46445bda Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 2 May 2018 03:55:36 +0200 Subject: [PATCH 8/9] Merge #13141: [doc] qt: fixes broken link on readme 12ad33a [doc] qt: fixes broken link on readme (marcoagner) Pull request description: I was reading qt files and just fixed a trivial mistake on its readme file. "#use-qt-Creator-as IDE" should be "#using-qt-creator-as-ide" Tree-SHA512: b2610821370dd7cac86725750ce7fd7a9e414c175e525f2e53e87302363c2a4a86206e0f9ce1d02657c751c395a33f0505953a94fd8124ea1943e39bb0711777 --- src/qt/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/README.md b/src/qt/README.md index c5ec232201..cba965b7a7 100644 --- a/src/qt/README.md +++ b/src/qt/README.md @@ -16,7 +16,7 @@ To run: ### forms -Contains [Designer UI](http://doc.qt.io/qt-5.9/designer-using-a-ui-file.html) files. They are created with [Qt Creator](#use-qt-Creator-as IDE), but can be edited using any text editor. +Contains [Designer UI](http://doc.qt.io/qt-5.9/designer-using-a-ui-file.html) files. They are created with [Qt Creator](#using-qt-creator-as-ide), but can be edited using any text editor. ### locale From 6cd3050c7f22d2dc469fcd45fb094641e1610f17 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 29 May 2018 16:03:17 +0200 Subject: [PATCH 9/9] Merge #13340: doc: remove leftover check-doc documentation 93843f68918f234929cfddc62b507041ce06805e doc: remove leftover check-doc documentation (fanquake) Pull request description: Remove leftover check-doc.py documentation. Mentioned [here](https://github.com/bitcoin/bitcoin/pull/13281#issuecomment-392010168), it's now [here](https://github.com/bitcoin/bitcoin/tree/master/test/lint#check-docpy). Tree-SHA512: 95a0ac221ffae109c1d4baf18a9220cf993fc07c005920a0bd09abdf52e8fb298e3b5df31fa18887719c5080d8531d18b84b7bd9c7c664ee2501ccd9e0975eb6 --- contrib/devtools/README.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/contrib/devtools/README.md b/contrib/devtools/README.md index 5a53c16815..343e3c8d72 100644 --- a/contrib/devtools/README.md +++ b/contrib/devtools/README.md @@ -2,12 +2,6 @@ Contents ======== This directory contains tools for developers working on this repository. -check-doc.py -============ - -Check if all command line args are documented. The return value indicates the -number of undocumented args. - clang-format-diff.py ===================