From 447ff166fb3df9b7c880dfe312cd43ef772e62e3 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 18 Feb 2019 10:32:26 -0500 Subject: [PATCH 01/13] Merge #15348: doc: Add separate productivity notes document 5b76c314d6 doc: Add separate productivity notes document (Carl Dong) Pull request description: Many developers have their own tools and tricks to be more productive during their cycles, so let's document the best ones so that everyone can benefit from them. Tree-SHA512: b4989e7a815e972a9a646f448fb6c08bd896b4bce77fd7fb22a71a7602971d4cbe34f88183f503f5b851d002784d9e91b87df5348c661eeb9cefa69c52e0de2b --- doc/README.md | 1 + doc/developer-notes.md | 48 ------------ doc/productivity.md | 161 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 162 insertions(+), 48 deletions(-) create mode 100644 doc/productivity.md diff --git a/doc/README.md b/doc/README.md index ec8298b7bb..00759cf03b 100644 --- a/doc/README.md +++ b/doc/README.md @@ -47,6 +47,7 @@ Development The Dash Core repo's [root README](/README.md) contains relevant information on the development process and automated testing. - [Developer Notes](developer-notes.md) +- [Productivity Notes](productivity.md) - [Release Notes](release-notes.md) - [Release Process](release-process.md) - Source Code Documentation ***TODO*** diff --git a/doc/developer-notes.md b/doc/developer-notes.md index f1fcbd8378..c8d0c327f3 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -785,54 +785,6 @@ would be to revert the upstream fix before applying the updates to Bitcoin's copy of LevelDB. In general you should be wary of any upstream changes affecting what data is returned from LevelDB queries. -Git and GitHub tips ---------------------- - -- For resolving merge/rebase conflicts, it can be useful to enable diff3 style using - `git config merge.conflictstyle diff3`. Instead of - - <<< - yours - === - theirs - >>> - - you will see - - <<< - yours - ||| - original - === - theirs - >>> - - This may make it much clearer what caused the conflict. In this style, you can often just look - at what changed between *original* and *theirs*, and mechanically apply that to *yours* (or the other way around). - -- When reviewing patches which change indentation in C++ files, use `git diff -w` and `git show -w`. This makes - the diff algorithm ignore whitespace changes. This feature is also available on github.com, by adding `?w=1` - at the end of any URL which shows a diff. - -- When reviewing patches that change symbol names in many places, use `git diff --word-diff`. This will instead - of showing the patch as deleted/added *lines*, show deleted/added *words*. - -- When reviewing patches that move code around, try using - `git diff --patience commit~:old/file.cpp commit:new/file/name.cpp`, and ignoring everything except the - moved body of code which should show up as neither `+` or `-` lines. In case it was not a pure move, this may - even work when combined with the `-w` or `--word-diff` options described above. - -- When looking at other's pull requests, it may make sense to add the following section to your `.git/config` - file: - - [remote "upstream-pull"] - fetch = +refs/pull/*:refs/remotes/upstream-pull/* - url = git@github.com:bitcoin/bitcoin.git - - This will add an `upstream-pull` remote to your git repository, which can be fetched using `git fetch --all` - or `git fetch upstream-pull`. Afterwards, you can use `upstream-pull/NUMBER/head` in arguments to `git show`, - `git checkout` and anywhere a commit id would be acceptable to see the changes from pull request NUMBER. - Scripted diffs -------------- diff --git a/doc/productivity.md b/doc/productivity.md new file mode 100644 index 0000000000..862017290d --- /dev/null +++ b/doc/productivity.md @@ -0,0 +1,161 @@ +Productivity Notes +================== + +Table of Contents +----------------- + +* [General](#general) + * [Cache compilations with `ccache`](#cache-compilations-with-ccache) + * [Disable features with `./configure`](#disable-features-with-configure) + * [Make use of your threads with `make -j`](#make-use-of-your-threads-with-make--j) + * [Multiple working directories with `git worktrees`](#multiple-working-directories-with-git-worktrees) +* [Writing code](#writing-code) + * [Format C/C++/Protobuf diffs with `clang-format-diff.py`](#format-ccprotobuf-diffs-with-clang-format-diffpy) + * [Format Python diffs with `yapf-diff.py`](#format-python-diffs-with-yapf-diffpy) +* [Rebasing/Merging code](#rebasingmerging-code) + * [More conflict context with `merge.conflictstyle diff3`](#more-conflict-context-with-mergeconflictstyle-diff3) +* [Reviewing code](#reviewing-code) + * [Reduce mental load with `git diff` options](#reduce-mental-load-with-git-diff-options) + * [Reference PRs easily with `refspec`s](#reference-prs-easily-with-refspecs) + * [Diff the diffs with `git range-diff`](#diff-the-diffs-with-git-range-diff) + +General +------ + +### Cache compilations with `ccache` + +The easiest way to faster compile times is to cache compiles. `ccache` is a way to do so, from its description at the time of writing: + +> ccache is a compiler cache. It speeds up recompilation by caching the result of previous compilations and detecting when the same compilation is being done again. Supported languages are C, C++, Objective-C and Objective-C++. + +Install `ccache` through your distribution's package manager, and run `./configure` with your normal flags to pick it up. + +To use ccache for all your C/C++ projects, follow the symlinks method [here](https://ccache.samba.org/manual/latest.html#_run_modes) to set it up. + +### Disable features with `./configure` + +After running `./autogen.sh`, which generates the `./configure` file, use `./configure --help` to identify features that you can disable to save on compilation time. A few common flags: + +```sh +--without-miniupnpc +--disable-bench +--disable-wallet +--without-gui +``` + +### Make use of your threads with `make -j` + +If you have multiple threads on your machine, you can tell `make` to utilize all of them with: + +```sh +make -j"$(($(nproc)+1))" +``` + +### Multiple working directories with `git worktrees` + +If you work with multiple branches or multiple copies of the repository, you should try `git worktrees`. + +To create a new branch that lives under a new working directory without disrupting your current working directory (useful for creating pull requests): +```sh +git worktree add -b my-shiny-new-branch ../living-at-my-new-working-directory based-on-my-crufty-old-commit-ish +``` + +To simply check out a commit-ish under a new working directory without disrupting your current working directory (useful for reviewing pull requests): +```sh +git worktree add --checkout ../where-my-checkout-commit-ish-will-live my-checkout-commit-ish +``` + +----- + +This synergizes well with [`ccache`](#cache-compilations-with-ccache) as objects resulting from unchanged code will most likely hit the cache and won't need to be recompiled. + +You can also set up [upstream refspecs](#reference-prs-easily-with-refspecs) to refer to pull requests easier in the above `git worktree` commands. + +Writing code +------------ + +### Format C/C++/Protobuf diffs with `clang-format-diff.py` + +See [contrib/devtools/README.md](contrib/devtools/README.md#clang-format-diff.py). + +### Format Python diffs with `yapf-diff.py` + +Usage is exactly the same as [`clang-format-diff.py`](#format-ccprotobuf-diffs-with-clang-format-diffpy). You can get it [here](https://github.com/MarcoFalke/yapf-diff). + +Rebasing/Merging code +------------- + +### More conflict context with `merge.conflictstyle diff3` + +For resolving merge/rebase conflicts, it can be useful to enable diff3 style using `git config merge.conflictstyle diff3`. Instead of + +```diff +<<< +yours +=== +theirs +>>> +``` + + you will see + +```diff +<<< +yours +||| +original +=== +theirs +>>> +``` + +This may make it much clearer what caused the conflict. In this style, you can often just look at what changed between *original* and *theirs*, and mechanically apply that to *yours* (or the other way around). + +Reviewing code +-------------- + +### Reduce mental load with `git diff` options + +When reviewing patches which change indentation in C++ files, use `git diff -w` and `git show -w`. This makes the diff algorithm ignore whitespace changes. This feature is also available on github.com, by adding `?w=1` at the end of any URL which shows a diff. + +When reviewing patches that change symbol names in many places, use `git diff --word-diff`. This will instead of showing the patch as deleted/added *lines*, show deleted/added *words*. + +When reviewing patches that move code around, try using `git diff --patience commit~:old/file.cpp commit:new/file/name.cpp`, and ignoring everything except the moved body of code which should show up as neither `+` or `-` lines. In case it was not a pure move, this may even work when combined with the `-w` or `--word-diff` options described above. `--color-moved=dimmed-zebra` will also dim the coloring of moved hunks in the diff on compatible terminals. + +### Reference PRs easily with `refspec`s + +When looking at other's pull requests, it may make sense to add the following section to your `.git/config` file: + +``` +[remote "upstream-pull"] + fetch = +refs/pull/*:refs/remotes/upstream-pull/* + url = git@github.com:bitcoin/bitcoin.git +``` + +This will add an `upstream-pull` remote to your git repository, which can be fetched using `git fetch --all` or `git fetch upstream-pull`. Afterwards, you can use `upstream-pull/NUMBER/head` in arguments to `git show`, `git checkout` and anywhere a commit id would be acceptable to see the changes from pull request NUMBER. + +### Diff the diffs with `git range-diff` + +It is very common for contributors to rebase their pull requests, or make changes to commits (perhaps in response to review) that are not at the head of their branch. This poses a problem for reviewers as when the contributor force pushes, the reviewer is no longer sure that his previous reviews of commits are still valid (as the commit hashes can now be different even though the diff is semantically the same). `git range-diff` can help solve this problem by diffing the diffs. + +For example, to identify the differences between your previously reviewed diffs P1-5, and the new diffs P1-2,N3-4 as illustrated below: +``` + P1--P2--P3--P4--P5 <-- previously-reviewed-head + / +...--m <-- master + \ + P1--P2--N3--N4--N5 <-- new-head (with P3 slightly modified) +``` + +You can do: +```sh +git range-diff master previously-reviewed-head new-head +``` + +Note that `git range-diff` also work for rebases. + +----- + +`git range-diff` also accepts normal `git diff` options, see [Reduce mental load with `git diff` options](#reduce-mental-load-with-git-diff-options) for useful `git diff` options. + +You can also set up [upstream refspecs](#reference-prs-easily-with-refspecs) to refer to pull requests easier in the above `git range-diff` commands. From 9276c784974c197f591b29bf76789bd4846929cc Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 15 Feb 2019 15:57:07 +0100 Subject: [PATCH 02/13] Merge #15391: Add compile time verification of assumptions we're currently making implicitly/tacitly 7cee85807c4db679003c6659d247a2fe74c2464a Add compile time verification of assumptions we're currently making implicitly/tacitly (practicalswift) Pull request description: Add compile time verification of assumptions we're currently making implicitly/tacitly. As suggested by @sipa in https://github.com/bitcoin/bitcoin/pull/14239#issuecomment-462508012 and @MarcoFalke in https://github.com/bitcoin/bitcoin/pull/14479#issuecomment-462534878. Tree-SHA512: e68fe51164dbd3eeb76aa8a7e83dfcd3b4d5a66037c0f1822bbbd189bbe3c280e03b3b10af870880ecc09b612e62fb3d9bcd6cf1e16cb7ba818c257db0712ce4 --- src/Makefile.am | 1 + src/compat/assumptions.h | 49 ++++++++++++++++++++++++++++++++++++++++ src/util/system.h | 1 + 3 files changed, 51 insertions(+) create mode 100644 src/compat/assumptions.h diff --git a/src/Makefile.am b/src/Makefile.am index c266dc4a70..462d2e4f30 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -142,6 +142,7 @@ BITCOIN_CORE_H = \ coinjoin/coinjoin-util.h \ coins.h \ compat.h \ + compat/assumptions.h \ compat/byteswap.h \ compat/endian.h \ compat/sanity.h \ diff --git a/src/compat/assumptions.h b/src/compat/assumptions.h new file mode 100644 index 0000000000..a3d81fe127 --- /dev/null +++ b/src/compat/assumptions.h @@ -0,0 +1,49 @@ +// Copyright (c) 2009-2010 Satoshi Nakamoto +// Copyright (c) 2009-2019 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +// Compile-time verification of assumptions we make. + +#ifndef BITCOIN_COMPAT_ASSUMPTIONS_H +#define BITCOIN_COMPAT_ASSUMPTIONS_H + +#include + +// Assumption: We assume that the macro NDEBUG is not defined. +// Example(s): We use assert(...) extensively with the assumption of it never +// being a noop at runtime. +#if defined(NDEBUG) +# error "Dash Core cannot be compiled without assertions." +#endif + +// Assumption: We assume the floating-point types to fulfill the requirements of +// IEC 559 (IEEE 754) standard. +// Example(s): Floating-point division by zero in ConnectBlock, CreateTransaction +// and EstimateMedianVal. +static_assert(std::numeric_limits::is_iec559, "IEEE 754 float assumed"); +static_assert(std::numeric_limits::is_iec559, "IEEE 754 double assumed"); + +// Assumption: We assume eight bits per byte (obviously, but remember: don't +// trust -- verify!). +// Example(s): Everywhere :-) +static_assert(std::numeric_limits::digits == 8, "8-bit byte assumed"); + +// Assumption: We assume floating-point widths. +// Example(s): Type punning in serialization code (ser_{float,double}_to_uint{32,64}). +static_assert(sizeof(float) == 4, "32-bit float assumed"); +static_assert(sizeof(double) == 8, "64-bit double assumed"); + +// Assumption: We assume integer widths. +// Example(s): GetSizeOfCompactSize and WriteCompactSize in the serialization +// code. +static_assert(sizeof(short) == 2, "16-bit short assumed"); +static_assert(sizeof(int) == 4, "32-bit int assumed"); + +// Some important things we are NOT assuming (non-exhaustive list): +// * We are NOT assuming a specific value for sizeof(std::size_t). +// * We are NOT assuming a specific value for std::endian::native. +// * We are NOT assuming a specific value for std::locale("").name(). +// * We are NOT assuming a specific value for std::numeric_limits::is_signed. + +#endif // BITCOIN_COMPAT_ASSUMPTIONS_H diff --git a/src/util/system.h b/src/util/system.h index 8e4d0f60fd..327c913d7e 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -17,6 +17,7 @@ #include #include +#include #include #include #include From 589bbc9166f59e7d0f8e1ca0d12826a99cb5678a Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 14 Feb 2019 16:42:06 -0500 Subject: [PATCH 03/13] Merge #15285: build: Prefer Python 3.4 even if newer versions are present on the system 0890339fb3 build: prefer python3.4 even if newer versions are present on the system (Sjors Provoost) Pull request description: Python 3.4 is this mimimum supported version according to [doc/dependencies.md](https://github.com/bitcoin/bitcoin/blob/master/doc/dependencies.md) Systems with [PyEnv](https://github.com/pyenv/pyenv) ensure (via [.python-version](https://github.com/bitcoin/bitcoin/blob/master/.python-version)) that Python 3.4 is used for the functional tests. However `make check` calls `bitcoin-util-test.py` using the Python command found by `configure.ac`, which looks system wide. On systems with multiple versions of Python this would cause `make check` to fail, as it tries to call a version of Python that PyEnv blocks. This is solved by preferring python3.4 in `configure.ac`. I missed this in #14884, so ideally this should be tagged 0.18 Tree-SHA512: b7487081a1ee7c2cb672a2e4bc1943ec8d23825fb941e567cb00fb123e6d59b1d8b7ddbf97d48aca770b9ddb9eacbfe73d8ac8cb1e1cdc34587ee1cee9929840 --- configure.ac | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index e45a8c72db..8db754e1c9 100644 --- a/configure.ac +++ b/configure.ac @@ -95,8 +95,8 @@ AC_PATH_TOOL(RANLIB, ranlib) AC_PATH_TOOL(STRIP, strip) AC_PATH_TOOL(GCOV, gcov) AC_PATH_PROG(LCOV, lcov) -dnl Python 3.x is supported from 3.4 on (see https://github.com/bitcoin/bitcoin/issues/7893) -AC_PATH_PROGS([PYTHON], [python3.7 python3.6 python3.5 python3.4 python3 python]) +dnl Python 3.4 is specified in .python-version and should be used if available, see doc/dependencies.md +AC_PATH_PROGS([PYTHON], [python3.4 python3.5 python3.6 python3.7 python3 python]) AC_PATH_PROG(GENHTML, genhtml) AC_PATH_PROG([GIT], [git]) AC_PATH_PROG(CCACHE,ccache) From ca9247fac44307a6d3d2a54d1d9c530853c7ed84 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 12 Feb 2019 15:27:13 -0500 Subject: [PATCH 04/13] Merge #15378: tests: Added missing tests for RPC wallet errors dc3b2ccb5f tests: Added missing tests for RPC wallet errors (Ben Carman) Pull request description: Tree-SHA512: b18dcd4f7547c974c93ae67dcd92a168bdb55951b164cf174cb1e59e0daa463187068aec43108309a75d65721a5c0bcdf10a16a9869620f160121e2287559926 --- test/functional/wallet_basic.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 0b4ef954dc..b2af98ac39 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -325,12 +325,38 @@ class WalletTest(BitcoinTestFramework): tx_obj = self.nodes[0].gettransaction(txid) assert_equal(tx_obj['amount'], Decimal('-0.0001')) + # General checks for errors from incorrect inputs # This will raise an exception because the amount type is wrong assert_raises_rpc_error(-3, "Invalid amount", self.nodes[0].sendtoaddress, self.nodes[2].getnewaddress(), "1f-4") # This will raise an exception since generate does not accept a string assert_raises_rpc_error(-1, "not an integer", self.nodes[0].generate, "2") + # This will raise an exception for the invalid private key format + assert_raises_rpc_error(-5, "Invalid private key encoding", self.nodes[0].importprivkey, "invalid") + + # This will raise an exception for importing an address with the PS2H flag + temp_address = self.nodes[1].getnewaddress() + assert_raises_rpc_error(-5, "Cannot use the p2sh flag with an address - use a script instead", self.nodes[0].importaddress, temp_address, "label", False, True) + + # This will raise an exception for attempting to dump the private key of an address you do not own + assert_raises_rpc_error(-3, "Address does not refer to a key", self.nodes[0].dumpprivkey, temp_address) + + # This will raise an exception for attempting to get the private key of an invalid Dash address + assert_raises_rpc_error(-5, "Invalid Dash address", self.nodes[0].dumpprivkey, "invalid") + + # This will raise an exception for attempting to set a label for an invalid Dash address + assert_raises_rpc_error(-5, "Invalid Dash address", self.nodes[0].setlabel, "invalid address", "label") + + # This will raise an exception for importing an invalid address + assert_raises_rpc_error(-5, "Invalid Dash address or script", self.nodes[0].importaddress, "invalid") + + # This will raise an exception for attempting to import a pubkey that isn't in hex + assert_raises_rpc_error(-5, "Pubkey must be a hex string", self.nodes[0].importpubkey, "not hex") + + # This will raise an exception for importing an invalid pubkey + assert_raises_rpc_error(-5, "Pubkey is not a valid public key", self.nodes[0].importpubkey, "5361746f736869204e616b616d6f746f") + # Import address and private key to check correct behavior of spendable unspents # 1. Send some coins to generate new UTXO address_to_import = self.nodes[2].getnewaddress() From 184fa2a6a01f4fb3cc575441505fce54c0f5682c Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 12 Feb 2019 11:09:57 -0500 Subject: [PATCH 05/13] Merge #14543: [QA] minor p2p_sendheaders fix of height in coinbase 1cdb9bb51f minor p2p_sendheaders fix of height in coinbase (Gregory Sanders) Pull request description: > \# Now announce a header that forks the last two blocks Doesn't effect any behavior since BIP34 isn't active in regtest for many blocks. Tree-SHA512: 3f214b956a94250bb640f63b6ff707930e1d4cb8df1bbf0fef4012d89a94bafbde0d7b42bbe7113ec33810169281c22c6e389445921d99decb74aa56e87a0f27 --- test/functional/p2p_sendheaders.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/p2p_sendheaders.py b/test/functional/p2p_sendheaders.py index 6860cac538..41158de1e9 100755 --- a/test/functional/p2p_sendheaders.py +++ b/test/functional/p2p_sendheaders.py @@ -494,7 +494,7 @@ class SendHeadersTest(BitcoinTestFramework): # Now announce a header that forks the last two blocks tip = blocks[0].sha256 - height -= 1 + height -= 2 blocks = [] # Create extra blocks for later From 80f54e30672d61d0cc06fa9a36909a1683074cf5 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 11 Feb 2019 09:59:58 -0500 Subject: [PATCH 06/13] Merge #15353: docs: Minor textual improvements in translation_strings_policy.md a94e470921 A few textual improvements (Martin Erlandsson) Pull request description: Found a few places where the reading flow was interrupted by minor grammar and punctuation issues. Tree-SHA512: 50640288449eb035f463bce563d259efbe97c14517d92c916e1bc52b7c54961ba2fc6fca1272470153803397f20ae7570bd090c16850ebd0180ebcf6bb2415d1 --- doc/translation_strings_policy.md | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/translation_strings_policy.md b/doc/translation_strings_policy.md index e0f792ac64..b13ce3c47b 100644 --- a/doc/translation_strings_policy.md +++ b/doc/translation_strings_policy.md @@ -33,25 +33,25 @@ General recommendations Try not to burden translators with translating messages that are e.g. slight variations of other messages. In the GUI, avoid the use of text where an icon or symbol will do. -Make sure that placeholder texts in forms don't end up in the list of strings to be translated (use ``). +Make sure that placeholder texts in forms do not end up in the list of strings to be translated (use ``). ### Make translated strings understandable -Try to write translation strings in an understandable way, for both the user and the translator. Avoid overly technical or detailed messages +Try to write translation strings in an understandable way, for both the user and the translator. Avoid overly technical or detailed messages. ### Do not translate internal errors -Do not translate internal errors, or log messages, or messages that appear on the RPC interface. If an error is to be shown to the user, -use a translatable generic message, then log the detailed message to the log. E.g. "A fatal internal error occurred, see debug.log for details". +Do not translate internal errors, log messages, or messages that appear on the RPC interface. If an error is to be shown to the user, +use a translatable generic message, then log the detailed message to the log. E.g., "A fatal internal error occurred, see debug.log for details". This helps troubleshooting; if the error is the same for everyone, the likelihood is increased that it can be found using a search engine. ### Avoid fragments -Avoid dividing up a message into fragments. Translators see every string separately, so may misunderstand the context if the messages are not self-contained. +Avoid dividing up a message into fragments. Translators see every string separately, so they may misunderstand the context if the messages are not self-contained. ### Avoid HTML in translation strings -There have been difficulties with use of HTML in translation strings; translators should not be able to accidentally affect the formatting of messages. +There have been difficulties with the use of HTML in translation strings; translators should not be able to accidentally affect the formatting of messages. This may sometimes be at conflict with the recommendation in the previous section. ### Plurals @@ -66,7 +66,7 @@ Plurals can be complex in some languages. A quote from the gettext documentation 25-31 pliko'w and so on -In Qt code use tr's third argument for optional plurality. For example: +In Qt code, use tr's third argument for optional plurality. For example: tr("%n hour(s)","",secs/HOUR_IN_SECONDS); tr("%n day(s)","",secs/DAY_IN_SECONDS); @@ -82,7 +82,7 @@ This adds ``s to the respective `.ts` file, which can be translated -Where it is possible try to avoid embedding numbers into the flow of the string at all. e.g. +Where possible, try to avoid embedding numbers into the flow of the string at all. E.g., WARNING: check your network connection, %d blocks received in the last %d hours (%d expected) From f27f036d0b2478f10cb7087c8f4d6b23d9fa6a7f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 11 Feb 2019 08:20:25 -0500 Subject: [PATCH 07/13] Merge #15380: trivial: correct parameter name in comments 1a0139cbaf trivial: correct parameter name in comments (andrewtoth) Pull request description: Tree-SHA512: 029b5ca5406cd7bf704b4d7611dac072cdc46a8659041bf631d77372ed4c16fa9ddf02c754044e310b16ea9bdd0803d051bef6ef6a86815d523826666134c649 --- src/scheduler.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/scheduler.h b/src/scheduler.h index 596b57959d..0a46c52482 100644 --- a/src/scheduler.h +++ b/src/scheduler.h @@ -45,13 +45,13 @@ public: // Call func at/after time t void schedule(Function f, boost::chrono::system_clock::time_point t=boost::chrono::system_clock::now()); - // Convenience method: call f once deltaSeconds from now + // Convenience method: call f once deltaMilliSeconds from now void scheduleFromNow(Function f, int64_t deltaMilliSeconds); // Another convenience method: call f approximately - // every deltaSeconds forever, starting deltaSeconds from now. + // every deltaMilliSeconds forever, starting deltaMilliSeconds from now. // To be more precise: every time f is finished, it - // is rescheduled to run deltaSeconds later. If you + // is rescheduled to run deltaMilliSeconds later. If you // need more accurate scheduling, don't use this method. void scheduleEvery(Function f, int64_t deltaMilliSeconds); From 4eafb4dea60ddaa2b4ccb06077230bf912690086 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 5 Feb 2019 17:14:29 -0500 Subject: [PATCH 08/13] Merge #15327: tests: Make test updatecoins_simulation_test deterministic ef0b01217a tests: Make updatecoins_simulation_test deterministic (practicalswift) Pull request description: Make test `updatecoins_simulation_test` deterministic. Can be verified using `contrib/test_deterministic_coverage.sh` introduced in #15296. Related: * #15296: "tests: Add script checking for deterministic line coverage in unit tests" * #15324: "test: Make bloom tests deterministic" * #14343: "coverage reports non-deterministic" Tree-SHA512: 3466e28a42dd3735effb8542044d88e8350a470729d4a4f02abce9d6367de6568d698131469ba154d3dc76d448bacb360b7aefd066bb5b91408c0be375dd3ecb --- src/test/coins_tests.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp index 2b23a62b96..9eeef26db3 100644 --- a/src/test/coins_tests.cpp +++ b/src/test/coins_tests.cpp @@ -278,6 +278,8 @@ UtxoData::iterator FindRandomFrom(const std::set &utxoSet) { // has the expected effect (the other duplicate is overwritten at all cache levels) BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) { + SeedInsecureRand(/* deterministic */ true); + bool spent_a_duplicate_coinbase = false; // A simple map to track what we expect the cache stack to represent. std::map result; From bc186bba3003a785c462fa9c0f2f4047fe958fa8 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 5 Feb 2019 16:39:16 -0500 Subject: [PATCH 09/13] Merge #15203: Fix issue #9683 "gui, wallet: random abort (segmentation fault) 364cff1cab Fix issue #9683 "gui, wallet: random abort (segmentation fault) running master/HEAD". (Chris Moore) Pull request description: Patch taken from @ryanofsky's comment https://github.com/bitcoin/bitcoin/issues/9683#issuecomment-448035913. [MarcoFalke wrote](https://github.com/bitcoin/bitcoin/issues/9683#issuecomment-454066004): > Mind to submit this patch as a pull request? So that's what I'm doing. I was regularly seeing crashes on startup before applying this patch and haven't seen a single crash on startup since applying it almost a month ago. Tree-SHA512: 3bbb2291cdf03ab7e7b5b796df68d76272491e35d473a89f4550065554c092f867659a7b8d7a1a91461ae4dc9a3b13b72541eafdbd732536463e9f3cf82300c8 --- src/qt/transactionview.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 155ee90c7d..c8199cbab8 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -216,8 +216,8 @@ void TransactionView::setModel(WalletModel *_model) transactionView->setAlternatingRowColors(true); transactionView->setSelectionBehavior(QAbstractItemView::SelectRows); transactionView->setSelectionMode(QAbstractItemView::ExtendedSelection); + transactionView->horizontalHeader()->setSortIndicator(TransactionTableModel::Date, Qt::DescendingOrder); transactionView->setSortingEnabled(true); - transactionView->sortByColumn(TransactionTableModel::Date, Qt::DescendingOrder); transactionView->verticalHeader()->hide(); transactionView->setColumnWidth(TransactionTableModel::Status, STATUS_COLUMN_WIDTH); From 8846f79220281345a48fa1921a73b0260dc94f95 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 31 Jan 2019 19:08:03 +0100 Subject: [PATCH 10/13] Merge #15299: Fix assertion in CKey::SignCompact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3617f117394285c87c395a0ccc92941977f97019 Fix assertion in CKey::SignCompact (João Barbosa) Pull request description: Fixes #15286. Tree-SHA512: b39b6f26f87cf1850b13f625ab6de963937b6ecb5b6d4ac4932134f0491a6c0fa61c6d6e6980e8b1770775578dc365fdd1b6ba426bba1f7c23430f68b3a2339a --- src/key.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/key.cpp b/src/key.cpp index 860f1ded8d..b43d4fa188 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -226,7 +226,7 @@ bool CKey::SignCompact(const uint256 &hash, std::vector& vchSig) secp256k1_ecdsa_recoverable_signature sig; int ret = secp256k1_ecdsa_sign_recoverable(secp256k1_context_sign, &sig, hash.begin(), begin(), secp256k1_nonce_function_rfc6979, nullptr); assert(ret); - secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &sig); + ret = secp256k1_ecdsa_recoverable_signature_serialize_compact(secp256k1_context_sign, &vchSig[1], &rec, &sig); assert(ret); assert(rec != -1); vchSig[0] = 27 + rec + (fCompressed ? 4 : 0); From 63272f5b942241061b055e5608d3b4f2daf3ce56 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 31 Jan 2019 15:15:17 +0100 Subject: [PATCH 11/13] Merge #15244: gdb attaching to process during tests has non-sudo solution f96dbd1bbeeea82f07bc71c695fb17e8d7c9f1aa gdb attaching to process during tests has non-sudo solution (Gregory Sanders) Pull request description: There are some security considerations, so a link is attached. Tree-SHA512: 67dd9c4b26b1e6d8e9a9fe766d309c0af69b752f6f544f3dce4bdcc95ae85feb9a49ac600c3f70d100629505d2340ab43932ded53b1485f80b97981e6df6a527 --- test/README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/README.md b/test/README.md index f10f4cca07..d189239cd6 100644 --- a/test/README.md +++ b/test/README.md @@ -179,7 +179,8 @@ cat /tmp/user/1000/testo9vsdjo3/node1/regtest/dashd.pid gdb /home/example/dashd ``` -Note: gdb attach step may require `sudo` +Note: gdb attach step may require ptrace_scope to be modified, or `sudo` preceding the `gdb`. +See this link for considerations: https://www.kernel.org/doc/Documentation/security/Yama.txt ### Util tests From 90fc61450047f81f2eb46d4d9ba57d48ad8457c9 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 30 Jan 2019 16:17:58 +0100 Subject: [PATCH 12/13] Merge #15243: [doc] add notes on release notes 65bc38d1c1f666e2c2d773111921b115d4249563 [doc] add notes on release notes (John Newbery) Pull request description: Explains when and how release notes should be written. Tree-SHA512: 94085d5a30499f41e6d1821b9f157aea40b3cff61a8ba606fed1b239e794ffe6769f985f53400715d712d12aadaa8db8cfca08dd1700a1fe17df86e0e554eac2 --- doc/developer-notes.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index c8d0c327f3..c7b10769fc 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -33,6 +33,7 @@ Developer Notes - [Subtrees](#subtrees) - [Git and GitHub tips](#git-and-github-tips) - [Scripted diffs](#scripted-diffs) + - [Release notes](#release-notes) - [RPC interface guidelines](#rpc-interface-guidelines) @@ -805,6 +806,21 @@ The scripted-diff is verified by the tool `test/lint/commit-script-check.sh` Commit [`bb81e173`](https://github.com/bitcoin/bitcoin/commit/bb81e173) is an example of a scripted-diff. +Release notes +------------- + +Release notes should be written for any PR that: + +- introduces a notable new feature +- fixes a significant bug +- changes an API or configuration model +- makes any other visible change to the end-user experience. + +Release notes should be added to a PR-specific release note file at +`/doc/release-notes-.md` to avoid conflicts between multiple PRs. +All `release-notes*` files are merged into a single +[/doc/release-notes.md](/doc/release-notes.md) file prior to the release. + RPC interface guidelines -------------------------- From 0f648122cde8a7bb235b39616c4f39e01057dd28 Mon Sep 17 00:00:00 2001 From: PastaPastaPasta <6443210+PastaPastaPasta@users.noreply.github.com> Date: Wed, 30 Jun 2021 18:12:37 -0500 Subject: [PATCH 13/13] Update test/functional/wallet_basic.py Co-authored-by: UdjinM6 --- test/functional/wallet_basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index b2af98ac39..f32d835481 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -340,7 +340,7 @@ class WalletTest(BitcoinTestFramework): assert_raises_rpc_error(-5, "Cannot use the p2sh flag with an address - use a script instead", self.nodes[0].importaddress, temp_address, "label", False, True) # This will raise an exception for attempting to dump the private key of an address you do not own - assert_raises_rpc_error(-3, "Address does not refer to a key", self.nodes[0].dumpprivkey, temp_address) + assert_raises_rpc_error(-4, "Private key for address %s is not known" % temp_address, self.nodes[0].dumpprivkey, temp_address) # This will raise an exception for attempting to get the private key of an invalid Dash address assert_raises_rpc_error(-5, "Invalid Dash address", self.nodes[0].dumpprivkey, "invalid")