sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-26 04:22:55 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #4586 from Munkybooty/backports-0.19-pr8

Browse Source 
Backports 0.19 pr8
This commit is contained in:
UdjinM6 2021-11-30 14:12:24 +03:00 committed by GitHub
commit d6b9958310
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
18 changed files with 246 additions and 196 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
doc/benchmarking.md
View File

@ -2,10 +2,17 @@ Benchmarking
============ ============
Dash Core has an internal benchmarking framework, with benchmarks Dash Core has an internal benchmarking framework, with benchmarks
for cryptographic algorithms (e.g. SHA1, SHA256, SHA512, RIPEMD160), as well as the rolling bloom filter. for cryptographic algorithms (e.g. SHA1, SHA256, SHA512, RIPEMD160, Poly1305, ChaCha20), rolling bloom filter, coins selection,
thread queue, wallet balance.
Running Running
--------------------- ---------------------
For benchmarks purposes you only need to compile `dash_bench`. Beware of configuring without `--enable-debug` as this would impact
benchmarking by unlatching log printers and lock analysis.
make -C src bench_dash
After compiling Dash Core, the benchmarks can be run with: After compiling Dash Core, the benchmarks can be run with:
src/bench/bench_dash src/bench/bench_dash
@ -21,15 +28,23 @@ The output will look similar to:
Help Help
--------------------- ---------------------
`-?` will print a list of options and exit:
src/bench/bench_dash -? src/bench/bench_dash --help
To print options like scaling factor or per-benchmark filter.
Notes Notes
--------------------- ---------------------
More benchmarks are needed for, in no particular order: More benchmarks are needed for, in no particular order:
- Script Validation - Script Validation
- CCoinDBView caching
- Coins database - Coins database
- Memory pool - Memory pool
- Wallet coin selection - Cuckoo Cache
- P2P throughput
Going Further
--------------------
To monitor Bitcoin Core performance more in depth (like reindex or IBD): https://github.com/chaincodelabs/bitcoinperf
To generate Flame Graphs for Bitcoin Core: https://github.com/eklitzke/bitcoin/blob/flamegraphs/doc/flamegraphs.md

147
doc/developer-notes.md
View File

@ -63,7 +63,7 @@ tool to clean up patches automatically before submission.
- Braces on the same line for everything else. - Braces on the same line for everything else.
- 4 space indentation (no tabs) for every block except namespaces. - 4 space indentation (no tabs) for every block except namespaces.
- No indentation for `public`/`protected`/`private` or for `namespace`. - No indentation for `public`/`protected`/`private` or for `namespace`.
- No extra spaces inside parenthesis; don't do ( this ) - No extra spaces inside parenthesis; don't do ( this ).
- No space after function names; one space after `if`, `for` and `while`. - No space after function names; one space after `if`, `for` and `while`.
- If an `if` only has a single-statement `then`-clause, it can appear - If an `if` only has a single-statement `then`-clause, it can appear
on the same line as the `if`, without braces. In every other case, on the same line as the `if`, without braces. In every other case,
@ -78,12 +78,12 @@ tool to clean up patches automatically before submission.
- **Symbol naming conventions**. These are preferred in new code, but are not - **Symbol naming conventions**. These are preferred in new code, but are not
required when doing so would need changes to significant pieces of existing required when doing so would need changes to significant pieces of existing
code. code.
- Variable (including function arguments) and namespace names are all lowercase, and may use `_` to - Variable (including function arguments) and namespace names are all lowercase and may use `_` to
separate words (snake_case). separate words (snake_case).
- Class member variables have a `m_` prefix. - Class member variables have a `m_` prefix.
- Global variables have a `g_` prefix. - Global variables have a `g_` prefix.
- Constant names are all uppercase, and use `_` to separate words. - Constant names are all uppercase, and use `_` to separate words.
- Class names, function names and method names are UpperCamelCase - Class names, function names, and method names are UpperCamelCase
(PascalCase). Do not prefix class names with `C`. (PascalCase). Do not prefix class names with `C`.
- Test suite naming convention: The Boost test suite in file - Test suite naming convention: The Boost test suite in file
`src/test/foo_tests.cpp` should be named `foo_tests`. Test suite names `src/test/foo_tests.cpp` should be named `foo_tests`. Test suite names
@ -141,6 +141,7 @@ Dash Core uses [Doxygen](http://www.doxygen.nl/) to generate its official docume
Use Doxygen-compatible comment blocks for functions, methods, and fields. Use Doxygen-compatible comment blocks for functions, methods, and fields.
For example, to describe a function use: For example, to describe a function use:
```c++ ```c++
/** /**
* ... text ... * ... text ...
@ -150,11 +151,12 @@ For example, to describe a function use:
*/ */
bool function(int arg1, const char *arg2) bool function(int arg1, const char *arg2)
``` ```
A complete list of `@xxx` commands can be found at http://www.stack.nl/~dimitri/doxygen/manual/commands.html. A complete list of `@xxx` commands can be found at http://www.stack.nl/~dimitri/doxygen/manual/commands.html.
As Doxygen recognizes the comments by the delimiters (`/**` and `*/` in this case), you don't As Doxygen recognizes the comments by the delimiters (`/**` and `*/` in this case), you don't
*need* to provide any commands for a comment to be valid; just a description text is fine. *need* to provide any commands for a comment to be valid; just a description text is fine.
To describe a class use the same construct above the class definition: To describe a class, use the same construct above the class definition:
```c++ ```c++
/** /**
* Alerts are for notifying old versions if they become too obsolete and * Alerts are for notifying old versions if they become too obsolete and
@ -196,7 +198,7 @@ but the above styles are favored.
Documentation can be generated with `make docs` and cleaned up with `make clean-docs`. The resulting files are located in `doc/doxygen/html`; open `index.html` to view the homepage. Documentation can be generated with `make docs` and cleaned up with `make clean-docs`. The resulting files are located in `doc/doxygen/html`; open `index.html` to view the homepage.
Before running `make docs`, you will need to install dependencies `doxygen` and `dot`. For example, on MacOS via Homebrew: Before running `make docs`, you will need to install dependencies `doxygen` and `dot`. For example, on macOS via Homebrew:
``` ```
brew install graphviz doxygen brew install graphviz doxygen
``` ```
@ -238,7 +240,7 @@ that run in `-regtest` mode.
Dash Core is a multi-threaded application, and deadlocks or other Dash Core is a multi-threaded application, and deadlocks or other
multi-threading bugs can be very difficult to track down. The `--enable-debug` multi-threading bugs can be very difficult to track down. The `--enable-debug`
configure option adds `-DDEBUG_LOCKORDER` to the compiler flags. This inserts configure option adds `-DDEBUG_LOCKORDER` to the compiler flags. This inserts
run-time checks to keep track of which locks are held, and adds warnings to the run-time checks to keep track of which locks are held and adds warnings to the
debug.log file if inconsistencies are detected. debug.log file if inconsistencies are detected.
### Valgrind suppressions file ### Valgrind suppressions file
@ -306,7 +308,7 @@ $ perf record \
-p `pgrep dashd` -- sleep 60 -p `pgrep dashd` -- sleep 60
``` ```
You could then analyze the results by running You could then analyze the results by running:
```sh ```sh
perf report --stdio | c++filt | less perf report --stdio | c++filt | less
@ -371,7 +373,7 @@ Additional resources:
Locking/mutex usage notes Locking/mutex usage notes
------------------------- -------------------------
The code is multi-threaded, and uses mutexes and the The code is multi-threaded and uses mutexes and the
`LOCK` and `TRY_LOCK` macros to protect data structures. `LOCK` and `TRY_LOCK` macros to protect data structures.
Deadlocks due to inconsistent lock ordering (thread 1 locks `cs_main` and then Deadlocks due to inconsistent lock ordering (thread 1 locks `cs_main` and then
@ -396,7 +398,7 @@ Threads
- ThreadDNSAddressSeed : Loads addresses of peers from the DNS. - ThreadDNSAddressSeed : Loads addresses of peers from the DNS.
- ThreadMapPort : Universal plug-and-play startup/shutdown - ThreadMapPort : Universal plug-and-play startup/shutdown.
- ThreadSocketHandler : Sends/Receives data from peers on port 9999. - ThreadSocketHandler : Sends/Receives data from peers on port 9999.
@ -428,7 +430,7 @@ Thread pools
Ignoring IDE/editor files Ignoring IDE/editor files
-------------------------- --------------------------
In closed-source environments in which everyone uses the same IDE it is common In closed-source environments in which everyone uses the same IDE, it is common
to add temporary files it produces to the project-wide `.gitignore` file. to add temporary files it produces to the project-wide `.gitignore` file.
However, in open source software such as Dash Core, where everyone uses However, in open source software such as Dash Core, where everyone uses
@ -466,19 +468,19 @@ pay attention to for reviewers of Dash Core code.
General Dash Core General Dash Core
---------------------- ----------------------
- New features should be exposed on RPC first, then can be made available in the GUI - New features should be exposed on RPC first, then can be made available in the GUI.
- *Rationale*: RPC allows for better automatic testing. The test suite for - *Rationale*: RPC allows for better automatic testing. The test suite for
the GUI is very limited the GUI is very limited.
- Make sure pull requests pass Travis CI before merging - Make sure pull requests pass Travis CI before merging.
- *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing - *Rationale*: Makes sure that they pass thorough testing, and that the tester will keep passing
on the master branch. Otherwise all new pull requests will start failing the tests, resulting in on the master branch. Otherwise, all new pull requests will start failing the tests, resulting in
confusion and mayhem confusion and mayhem.
- *Explanation*: If the test suite is to be updated for a change, this has to - *Explanation*: If the test suite is to be updated for a change, this has to
be done first be done first.
Wallet Wallet
------- -------
@ -486,13 +488,13 @@ Wallet
- Make sure that no crashes happen with run-time option `-disablewallet`. - Make sure that no crashes happen with run-time option `-disablewallet`.
- *Rationale*: In RPC code that conditionally uses the wallet (such as - *Rationale*: In RPC code that conditionally uses the wallet (such as
`validateaddress`) it is easy to forget that global pointer `pwalletMain` `validateaddress`), it is easy to forget that global pointer `pwalletMain`
can be nullptr. See `test/functional/disablewallet.py` for functional tests can be nullptr. See `test/functional/disablewallet.py` for functional tests
exercising the API with `-disablewallet` exercising the API with `-disablewallet`.
- Include `db_cxx.h` (BerkeleyDB header) only when `ENABLE_WALLET` is set - Include `db_cxx.h` (BerkeleyDB header) only when `ENABLE_WALLET` is set.
- *Rationale*: Otherwise compilation of the disable-wallet build will fail in environments without BerkeleyDB - *Rationale*: Otherwise compilation of the disable-wallet build will fail in environments without BerkeleyDB.
General C++ General C++
------------- -------------
@ -503,26 +505,26 @@ Guidelines](https://isocpp.github.io/CppCoreGuidelines/).
Common misconceptions are clarified in those sections: Common misconceptions are clarified in those sections:
- Passing (non-)fundamental types in the [C++ Core - Passing (non-)fundamental types in the [C++ Core
Guideline](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-conventional) Guideline](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-conventional).
- Assertions should not have side-effects - Assertions should not have side-effects.
- *Rationale*: Even though the source code is set to refuse to compile - *Rationale*: Even though the source code is set to refuse to compile
with assertions disabled, having side-effects in assertions is unexpected and with assertions disabled, having side-effects in assertions is unexpected and
makes the code harder to understand makes the code harder to understand.
- If you use the `.h`, you must link the `.cpp` - If you use the `.h`, you must link the `.cpp`.
- *Rationale*: Include files define the interface for the code in implementation files. Including one but - *Rationale*: Include files define the interface for the code in implementation files. Including one but
not linking the other is confusing. Please avoid that. Moving functions from not linking the other is confusing. Please avoid that. Moving functions from
the `.h` to the `.cpp` should not result in build errors the `.h` to the `.cpp` should not result in build errors.
- Use the RAII (Resource Acquisition Is Initialization) paradigm where possible. For example by using - Use the RAII (Resource Acquisition Is Initialization) paradigm where possible. For example, by using
`unique_ptr` for allocations in a function. `unique_ptr` for allocations in a function.
- *Rationale*: This avoids memory and resource leaks, and ensures exception safety - *Rationale*: This avoids memory and resource leaks, and ensures exception safety.
- Use `MakeUnique()` to construct objects owned by `unique_ptr`s - Use `MakeUnique()` to construct objects owned by `unique_ptr`s.
- *Rationale*: `MakeUnique` is concise and ensures exception safety in complex expressions. - *Rationale*: `MakeUnique` is concise and ensures exception safety in complex expressions.
`MakeUnique` is a temporary project local implementation of `std::make_unique` (C++14). `MakeUnique` is a temporary project local implementation of `std::make_unique` (C++14).
@ -530,27 +532,27 @@ Common misconceptions are clarified in those sections:
C++ data structures C++ data structures
-------------------- --------------------
- Never use the `std::map []` syntax when reading from a map, but instead use `.find()` - Never use the `std::map []` syntax when reading from a map, but instead use `.find()`.
- *Rationale*: `[]` does an insert (of the default element) if the item doesn't - *Rationale*: `[]` does an insert (of the default element) if the item doesn't
exist in the map yet. This has resulted in memory leaks in the past, as well as exist in the map yet. This has resulted in memory leaks in the past, as well as
race conditions (expecting read-read behavior). Using `[]` is fine for *writing* to a map race conditions (expecting read-read behavior). Using `[]` is fine for *writing* to a map.
- Do not compare an iterator from one data structure with an iterator of - Do not compare an iterator from one data structure with an iterator of
another data structure (even if of the same type) another data structure (even if of the same type).
- *Rationale*: Behavior is undefined. In C++ parlor this means "may reformat - *Rationale*: Behavior is undefined. In C++ parlor this means "may reformat
the universe", in practice this has resulted in at least one hard-to-debug crash bug the universe", in practice this has resulted in at least one hard-to-debug crash bug.
- Watch out for out-of-bounds vector access. `&vch[vch.size()]` is illegal, - Watch out for out-of-bounds vector access. `&vch[vch.size()]` is illegal,
including `&vch[0]` for an empty vector. Use `vch.data()` and `vch.data() + including `&vch[0]` for an empty vector. Use `vch.data()` and `vch.data() +
vch.size()` instead. vch.size()` instead.
- Vector bounds checking is only enabled in debug mode. Do not rely on it - Vector bounds checking is only enabled in debug mode. Do not rely on it.
- Initialize all non-static class members where they are defined. - Initialize all non-static class members where they are defined.
If this is skipped for a good reason (i.e., optimization on the critical If this is skipped for a good reason (i.e., optimization on the critical
path), add an explicit comment about this path), add an explicit comment about this.
- *Rationale*: Ensure determinism by avoiding accidental use of uninitialized - *Rationale*: Ensure determinism by avoiding accidental use of uninitialized
values. Also, static analyzers balk about this. values. Also, static analyzers balk about this.
@ -574,12 +576,12 @@ class A
`int8_t`. Do not use bare `char` unless it is to pass to a third-party API. `int8_t`. Do not use bare `char` unless it is to pass to a third-party API.
This type can be signed or unsigned depending on the architecture, which can This type can be signed or unsigned depending on the architecture, which can
lead to interoperability problems or dangerous conditions such as lead to interoperability problems or dangerous conditions such as
out-of-bounds array accesses out-of-bounds array accesses.
- Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior - Prefer explicit constructions over implicit ones that rely on 'magical' C++ behavior.
- *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those - *Rationale*: Easier to understand what is happening, thus easier to spot mistakes, even for those
that are not language lawyers that are not language lawyers.
- Prefer signed ints and do not mix signed and unsigned integers. If an unsigned int is used, it should have a good - Prefer signed ints and do not mix signed and unsigned integers. If an unsigned int is used, it should have a good
reason. The fact a value will never be negative is not a good reason. The most common reason will be that mod two reason. The fact a value will never be negative is not a good reason. The most common reason will be that mod two
arithmetic is needed, such as in cryptographic primitives. If you need to make sure that some value is always arithmetic is needed, such as in cryptographic primitives. If you need to make sure that some value is always
@ -594,17 +596,17 @@ Strings and formatting
- Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not. - Be careful of `LogPrint` versus `LogPrintf`. `LogPrint` takes a `category` argument, `LogPrintf` does not.
- *Rationale*: Confusion of these can result in runtime exceptions due to - *Rationale*: Confusion of these can result in runtime exceptions due to
formatting mismatch, and it is easy to get wrong because of subtly similar naming formatting mismatch, and it is easy to get wrong because of subtly similar naming.
- Use `std::string`, avoid C string manipulation functions - Use `std::string`, avoid C string manipulation functions.
- *Rationale*: C++ string handling is marginally safer, less scope for - *Rationale*: C++ string handling is marginally safer, less scope for
buffer overflows and surprises with `\0` characters. Also some C string manipulations buffer overflows, and surprises with `\0` characters. Also, some C string manipulations
tend to act differently depending on platform, or even the user locale tend to act differently depending on platform, or even the user locale.
- Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing - Use `ParseInt32`, `ParseInt64`, `ParseUInt32`, `ParseUInt64`, `ParseDouble` from `utilstrencodings.h` for number parsing.
- *Rationale*: These functions do overflow checking, and avoid pesky locale issues. - *Rationale*: These functions do overflow checking and avoid pesky locale issues.
- Avoid using locale dependent functions if possible. You can use the provided - Avoid using locale dependent functions if possible. You can use the provided
[`lint-locale-dependence.sh`](/test/lint/lint-locale-dependence.sh) [`lint-locale-dependence.sh`](/test/lint/lint-locale-dependence.sh)
@ -634,9 +636,9 @@ Strings and formatting
`wcstoll`, `wcstombs`, `wcstoul`, `wcstoull`, `wcstoumax`, `wcswidth`, `wcstoll`, `wcstombs`, `wcstoul`, `wcstoull`, `wcstoumax`, `wcswidth`,
`wcsxfrm`, `wctob`, `wctomb`, `wctrans`, `wctype`, `wcwidth`, `wprintf` `wcsxfrm`, `wctob`, `wctomb`, `wctrans`, `wctype`, `wcwidth`, `wprintf`
- For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers - For `strprintf`, `LogPrint`, `LogPrintf` formatting characters don't need size specifiers.
- *Rationale*: Dash Core uses tinyformat, which is type safe. Leave them out to avoid confusion - *Rationale*: Dash Core uses tinyformat, which is type safe. Leave them out to avoid confusion.
- Use `.c_str()` sparingly. Its only valid use is to pass C++ strings to C functions that take NULL-terminated - Use `.c_str()` sparingly. Its only valid use is to pass C++ strings to C functions that take NULL-terminated
strings. strings.
@ -663,13 +665,12 @@ Strings and formatting
Shadowing Shadowing
-------------- --------------
Although the shadowing warning (`-Wshadow`) is not enabled by default (it prevents issues rising Although the shadowing warning (`-Wshadow`) is not enabled by default (it prevents issues arising
from using a different variable with the same name), from using a different variable with the same name),
please name variables so that their names do not shadow variables defined in the source code. please name variables so that their names do not shadow variables defined in the source code.
When using nested cycles, do not name the inner cycle variable the same as in When using nested cycles, do not name the inner cycle variable the same as in
upper cycle etc. the upper cycle, etc.
Threads and synchronization Threads and synchronization
---------------------------- ----------------------------
@ -679,10 +680,9 @@ Threads and synchronization
- When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of - When using `LOCK`/`TRY_LOCK` be aware that the lock exists in the context of
the current scope, so surround the statement and the code that needs the lock the current scope, so surround the statement and the code that needs the lock
with braces with braces.
OK: OK:
```c++ ```c++
{ {
TRY_LOCK(cs_vNodes, lockNodes); TRY_LOCK(cs_vNodes, lockNodes);
@ -691,7 +691,6 @@ Threads and synchronization
``` ```
Wrong: Wrong:
```c++ ```c++
TRY_LOCK(cs_vNodes, lockNodes); TRY_LOCK(cs_vNodes, lockNodes);
{ {
@ -713,13 +712,11 @@ Scripts
`#!/usr/bin/env bash` searches the user's PATH to find the bash binary. `#!/usr/bin/env bash` searches the user's PATH to find the bash binary.
OK: OK:
```bash ```bash
#!/usr/bin/env bash #!/usr/bin/env bash
``` ```
Wrong: Wrong:
```bash ```bash
#!/bin/bash #!/bin/bash
``` ```
@ -728,9 +725,9 @@ Source code organization
-------------------------- --------------------------
- Implementation code should go into the `.cpp` file and not the `.h`, unless necessary due to template usage or - Implementation code should go into the `.cpp` file and not the `.h`, unless necessary due to template usage or
when performance due to inlining is critical when performance due to inlining is critical.
- *Rationale*: Shorter and simpler header files are easier to read, and reduce compile time - *Rationale*: Shorter and simpler header files are easier to read and reduce compile time.
- Use only the lowercase alphanumerics (`a-z0-9`), underscore (`_`) and hyphen (`-`) in source code filenames. - Use only the lowercase alphanumerics (`a-z0-9`), underscore (`_`) and hyphen (`-`) in source code filenames.
@ -748,7 +745,7 @@ Source code organization
- Don't import anything into the global namespace (`using namespace ...`). Use - Don't import anything into the global namespace (`using namespace ...`). Use
fully specified types such as `std::string`. fully specified types such as `std::string`.
- *Rationale*: Avoids symbol conflicts - *Rationale*: Avoids symbol conflicts.
- Terminate namespaces with a comment (`// namespace mynamespace`). The comment - Terminate namespaces with a comment (`// namespace mynamespace`). The comment
should be placed on the same line as the brace closing the namespace, e.g. should be placed on the same line as the brace closing the namespace, e.g.
@ -763,7 +760,7 @@ namespace {
} // namespace } // namespace
``` ```
- *Rationale*: Avoids confusion about the namespace context - *Rationale*: Avoids confusion about the namespace context.
- Use `#include <primitives/transaction.h>` bracket syntax instead of - Use `#include <primitives/transaction.h>` bracket syntax instead of
`#include "primitives/transactions.h"` quote syntax. `#include "primitives/transactions.h"` quote syntax.
@ -786,13 +783,13 @@ namespace {
GUI GUI
----- -----
- Do not display or manipulate dialogs in model code (classes `*Model`) - Do not display or manipulate dialogs in model code (classes `*Model`).
- *Rationale*: Model classes pass through events and data from the core, they - *Rationale*: Model classes pass through events and data from the core, they
should not interact with the user. That's where View classes come in. The converse also should not interact with the user. That's where View classes come in. The converse also
holds: try to not directly access core data structures from Views. holds: try to not directly access core data structures from Views.
- Avoid adding slow or blocking code in the GUI thread. In particular do not - Avoid adding slow or blocking code in the GUI thread. In particular, do not
add new `interfaces::Node` and `interfaces::Wallet` method calls, even if they add new `interfaces::Node` and `interfaces::Wallet` method calls, even if they
may be fast now, in case they are changed to lock or communicate across may be fast now, in case they are changed to lock or communicate across
processes in the future. processes in the future.
@ -815,7 +812,7 @@ directly upstream without being PRed directly against the project. They will be
subtree merge. subtree merge.
Others are external projects without a tight relationship with our project. Changes to these should also Others are external projects without a tight relationship with our project. Changes to these should also
be sent upstream but bugfixes may also be prudent to PR against Dash Core so that they can be integrated be sent upstream, but bugfixes may also be prudent to PR against Dash Core so that they can be integrated
quickly. Cosmetic changes should be purely taken upstream. quickly. Cosmetic changes should be purely taken upstream.
There is a tool in `test/lint/git-subtree-check.sh` to check a subtree directory for consistency with There is a tool in `test/lint/git-subtree-check.sh` to check a subtree directory for consistency with
@ -850,11 +847,11 @@ you must be aware of.
### File Descriptor Counts ### File Descriptor Counts
In most configurations we use the default LevelDB value for `max_open_files`, In most configurations, we use the default LevelDB value for `max_open_files`,
which is 1000 at the time of this writing. If LevelDB actually uses this many which is 1000 at the time of this writing. If LevelDB actually uses this many
file descriptors it will cause problems with Bitcoin's `select()` loop, because file descriptors, it will cause problems with Bitcoin's `select()` loop, because
it may cause new sockets to be created where the fd value is >= 1024. For this it may cause new sockets to be created where the fd value is >= 1024. For this
reason, on 64-bit Unix systems we rely on an internal LevelDB optimization that reason, on 64-bit Unix systems, we rely on an internal LevelDB optimization that
uses `mmap()` + `close()` to open table files without actually retaining uses `mmap()` + `close()` to open table files without actually retaining
references to the table file descriptors. If you are upgrading LevelDB, you must references to the table file descriptors. If you are upgrading LevelDB, you must
sanity check the changes to make sure that this assumption remains valid. sanity check the changes to make sure that this assumption remains valid.
@ -879,14 +876,14 @@ details.
It is possible for LevelDB changes to inadvertently change consensus It is possible for LevelDB changes to inadvertently change consensus
compatibility between nodes. This happened in Bitcoin 0.8 (when LevelDB was compatibility between nodes. This happened in Bitcoin 0.8 (when LevelDB was
first introduced). When upgrading LevelDB you should review the upstream changes first introduced). When upgrading LevelDB, you should review the upstream changes
to check for issues affecting consensus compatibility. to check for issues affecting consensus compatibility.
For example, if LevelDB had a bug that accidentally prevented a key from being For example, if LevelDB had a bug that accidentally prevented a key from being
returned in an edge case, and that bug was fixed upstream, the bug "fix" would returned in an edge case, and that bug was fixed upstream, the bug "fix" would
be an incompatible consensus change. In this situation the correct behavior be an incompatible consensus change. In this situation, the correct behavior
would be to revert the upstream fix before applying the updates to Bitcoin's 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 copy of LevelDB. In general, you should be wary of any upstream changes affecting
what data is returned from LevelDB queries. what data is returned from LevelDB queries.
Scripted diffs Scripted diffs
@ -905,7 +902,7 @@ To create a scripted-diff:
- `-BEGIN VERIFY SCRIPT-` - `-BEGIN VERIFY SCRIPT-`
- `-END VERIFY SCRIPT-` - `-END VERIFY SCRIPT-`
The scripted-diff is verified by the tool `test/lint/commit-script-check.sh`. The tool's default behavior when supplied The scripted-diff is verified by the tool `test/lint/commit-script-check.sh`. The tool's default behavior, when supplied
with a commit is to verify all scripted-diffs from the beginning of time up to said commit. Internally, the tool passes with a commit is to verify all scripted-diffs from the beginning of time up to said commit. Internally, the tool passes
the first supplied argument to `git rev-list --reverse` to determine which commits to verify script-diffs for, ignoring the first supplied argument to `git rev-list --reverse` to determine which commits to verify script-diffs for, ignoring
commits that don't conform to the commit message format described above. commits that don't conform to the commit message format described above.
@ -981,23 +978,23 @@ RPC interface guidelines
A few guidelines for introducing and reviewing new RPC interfaces: A few guidelines for introducing and reviewing new RPC interfaces:
- Method naming: use consecutive lower-case names such as `getrawtransaction` and `submitblock` - Method naming: use consecutive lower-case names such as `getrawtransaction` and `submitblock`.
- *Rationale*: Consistency with existing interface. - *Rationale*: Consistency with the existing interface.
- Argument naming: use snake case `fee_delta` (and not, e.g. camel case `feeDelta`) - Argument naming: use snake case `fee_delta` (and not, e.g. camel case `feeDelta`)
- *Rationale*: Consistency with existing interface. - *Rationale*: Consistency with the existing interface.
- Use the JSON parser for parsing, don't manually parse integers or strings from - Use the JSON parser for parsing, don't manually parse integers or strings from
arguments unless absolutely necessary. arguments unless absolutely necessary.
- *Rationale*: Introduces hand-rolled string manipulation code at both the caller and callee sites, - *Rationale*: Introduces hand-rolled string manipulation code at both the caller and callee sites,
which is error prone, and it is easy to get things such as escaping wrong. which is error-prone, and it is easy to get things such as escaping wrong.
JSON already supports nested data structures, no need to re-invent the wheel. JSON already supports nested data structures, no need to re-invent the wheel.
- *Exception*: AmountFromValue can parse amounts as string. This was introduced because many JSON - *Exception*: AmountFromValue can parse amounts as string. This was introduced because many JSON
parsers and formatters hard-code handling decimal numbers as floating point parsers and formatters hard-code handling decimal numbers as floating-point
values, resulting in potential loss of precision. This is unacceptable for values, resulting in potential loss of precision. This is unacceptable for
monetary values. **Always** use `AmountFromValue` and `ValueFromAmount` when monetary values. **Always** use `AmountFromValue` and `ValueFromAmount` when
inputting or outputting monetary values. The only exceptions to this are inputting or outputting monetary values. The only exceptions to this are
@ -1006,7 +1003,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
- Missing arguments and 'null' should be treated the same: as default values. If there is no - Missing arguments and 'null' should be treated the same: as default values. If there is no
default value, both cases should fail in the same way. The easiest way to follow this default value, both cases should fail in the same way. The easiest way to follow this
guideline is detect unspecified arguments with `params[x].isNull()` instead of guideline is to detect unspecified arguments with `params[x].isNull()` instead of
`params.size() <= x`. The former returns true if the argument is either null or missing, `params.size() <= x`. The former returns true if the argument is either null or missing,
while the latter returns true if is missing, and false if it is null. while the latter returns true if is missing, and false if it is null.
@ -1033,7 +1030,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
from there. from there.
- A RPC method must either be a wallet method or a non-wallet method. Do not - A RPC method must either be a wallet method or a non-wallet method. Do not
introduce new methods that differ in behavior based on presence of a wallet. introduce new methods that differ in behavior based on the presence of a wallet.
- *Rationale*: as well as complicating the implementation and interfering - *Rationale*: as well as complicating the implementation and interfering
with the introduction of multi-wallet, wallet and non-wallet code should be with the introduction of multi-wallet, wallet and non-wallet code should be
@ -1041,7 +1038,7 @@ A few guidelines for introducing and reviewing new RPC interfaces:
- Try to make the RPC response a JSON object. - Try to make the RPC response a JSON object.
- *Rationale*: If a RPC response is not a JSON object then it is harder to avoid API breakage if - *Rationale*: If a RPC response is not a JSON object, then it is harder to avoid API breakage if
new data in the response is needed. new data in the response is needed.
- Wallet RPCs call BlockUntilSyncedToCurrentChain to maintain consistency with - Wallet RPCs call BlockUntilSyncedToCurrentChain to maintain consistency with

4
src/interfaces/README.md
View File

@ -2,9 +2,9 @@
The following interfaces are defined here: The following interfaces are defined here:
* [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973). * [`Chain`](chain.h) — used by wallet to access blockchain and mempool state. Added in [#14437](https://github.com/bitcoin/bitcoin/pull/14437), [#14711](https://github.com/bitcoin/bitcoin/pull/14711), [#15288](https://github.com/bitcoin/bitcoin/pull/15288), and [#10973](https://github.com/bitcoin/bitcoin/pull/10973).
* [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#10973](https://github.com/bitcoin/bitcoin/pull/10973). * [`ChainClient`](chain.h) — used by node to start & stop `Chain` clients. Added in [#14437](https://github.com/bitcoin/bitcoin/pull/14437).
* [`Node`](node.h) — used by GUI to start & stop bitcoin node. Added in [#10244](https://github.com/bitcoin/bitcoin/pull/10244). * [`Node`](node.h) — used by GUI to start & stop bitcoin node. Added in [#10244](https://github.com/bitcoin/bitcoin/pull/10244).

8
src/interfaces/chain.cpp
View File

@ -170,7 +170,7 @@ class LockImpl : public Chain::Lock
LockAnnotation lock(::cs_main); LockAnnotation lock(::cs_main);
return CheckFinalTx(tx); return CheckFinalTx(tx);
} }
bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) override bool submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state) override
{ {
LockAnnotation lock(::cs_main); LockAnnotation lock(::cs_main);
return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */, return AcceptToMemoryPool(::mempool, state, tx, nullptr /* missing inputs */,
@ -321,8 +321,8 @@ public:
bool hasDescendantsInMempool(const uint256& txid) override bool hasDescendantsInMempool(const uint256& txid) override
{ {
LOCK(::mempool.cs); LOCK(::mempool.cs);
auto it_mp = ::mempool.mapTx.find(txid); auto it = ::mempool.GetIter(txid);
return it_mp != ::mempool.mapTx.end() && it_mp->GetCountWithDescendants() > 1; return it && (*it)->GetCountWithDescendants() > 1;
} }
void relayTransaction(const uint256& txid) override void relayTransaction(const uint256& txid) override
{ {
@ -333,7 +333,7 @@ public:
{ {
::mempool.GetTransactionAncestry(txid, ancestors, descendants); ::mempool.GetTransactionAncestry(txid, ancestors, descendants);
} }
bool checkChainLimits(CTransactionRef tx) override bool checkChainLimits(const CTransactionRef& tx) override
{ {
LockPoints lp; LockPoints lp;
CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp); CTxMemPoolEntry entry(tx, 0, 0, 0, false, 0, lp);

36
src/interfaces/chain.h
View File

@ -15,6 +15,7 @@
#include <vector> #include <vector>
class CBlock; class CBlock;
class CFeeRate;
class CRPCCommand; class CRPCCommand;
class CScheduler; class CScheduler;
class CValidationState; class CValidationState;
@ -38,7 +39,27 @@ namespace interfaces {
class Wallet; class Wallet;
class Handler; class Handler;
//! Interface for giving wallet processes access to blockchain state. //! Interface giving clients (wallet processes, maybe other analysis tools in
//! the future) ability to access to the chain state, receive notifications,
//! estimate fees, and submit transactions.
//!
//! TODO: Current chain methods are too low level, exposing too much of the
//! internal workings of the bitcoin node, and not being very convenient to use.
//! Chain methods should be cleaned up and simplified over time. Examples:
//!
//! * The Chain::lock() method, which lets clients delay chain tip updates
//! should be removed when clients are able to respond to updates
//! asynchronously
//! (https://github.com/bitcoin/bitcoin/pull/10973#issuecomment-380101269).
//!
//! * The relayTransactions() and submitToMemoryPool() methods could be replaced
//! with a higher-level broadcastTransaction method
//! (https://github.com/bitcoin/bitcoin/pull/14978#issuecomment-459373984).
//!
//! * The initMessages() and loadWallet() methods which the wallet uses to send
//! notifications to the GUI should go away when GUI and wallet can directly
//! communicate with each other without going through the node
//! (https://github.com/bitcoin/bitcoin/pull/15288#discussion_r253321096).
class Chain class Chain
{ {
public: public:
@ -126,8 +147,9 @@ public:
virtual bool checkFinalTx(const CTransaction& tx) = 0; virtual bool checkFinalTx(const CTransaction& tx) = 0;
//! Add transaction to memory pool if the transaction fee is below the //! Add transaction to memory pool if the transaction fee is below the
//! amount specified by absurd_fee (as a safeguard). */ //! amount specified by absurd_fee. Returns false if the transaction
virtual bool submitToMemoryPool(CTransactionRef tx, CAmount absurd_fee, CValidationState& state) = 0; //! could not be added due to the fee or for another reason.
virtual bool submitToMemoryPool(const CTransactionRef& tx, CAmount absurd_fee, CValidationState& state) = 0;
}; };
//! Return Lock interface. Chain is locked when this is called, and //! Return Lock interface. Chain is locked when this is called, and
@ -168,8 +190,8 @@ public:
//! Calculate mempool ancestor and descendant counts for the given transaction. //! Calculate mempool ancestor and descendant counts for the given transaction.
virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0; virtual void getTransactionAncestry(const uint256& txid, size_t& ancestors, size_t& descendants) = 0;
//! Check chain limits. //! Check if transaction will pass the mempool's chain limits.
virtual bool checkChainLimits(CTransactionRef tx) = 0; virtual bool checkChainLimits(const CTransactionRef& tx) = 0;
//! Estimate smart fee. //! Estimate smart fee.
virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0; virtual CFeeRate estimateSmartFee(int num_blocks, bool conservative, FeeCalculation* calc = nullptr) = 0;
@ -177,7 +199,7 @@ public:
//! Fee estimator max target. //! Fee estimator max target.
virtual unsigned int estimateMaxBlocks() = 0; virtual unsigned int estimateMaxBlocks() = 0;
//! Pool min fee. //! Mempool minimum fee.
virtual CFeeRate mempoolMinFee() = 0; virtual CFeeRate mempoolMinFee() = 0;
//! Relay current minimum fee (from -minrelaytxfee and -incrementalrelayfee settings). //! Relay current minimum fee (from -minrelaytxfee and -incrementalrelayfee settings).
@ -189,7 +211,7 @@ public:
//! Relay dust fee setting (-dustrelayfee), reflecting lowest rate it's economical to spend. //! Relay dust fee setting (-dustrelayfee), reflecting lowest rate it's economical to spend.
virtual CFeeRate relayDustFee() = 0; virtual CFeeRate relayDustFee() = 0;
//! Get node max tx fee setting (-maxtxfee). //! Node max tx fee setting (-maxtxfee).
//! This could be replaced by a per-wallet max fee, as proposed at //! This could be replaced by a per-wallet max fee, as proposed at
//! https://github.com/bitcoin/bitcoin/issues/15355 //! https://github.com/bitcoin/bitcoin/issues/15355
//! But for the time being, wallets call this to access the node setting. //! But for the time being, wallets call this to access the node setting.

9
src/qt/dash.cpp
View File

@ -195,8 +195,11 @@ void BitcoinCore::shutdown()
} }
} }
BitcoinApplication::BitcoinApplication(interfaces::Node& node, int &argc, char **argv): static int qt_argc = 1;
QApplication(argc, argv), static const char* qt_argv = "dash-qt";
BitcoinApplication::BitcoinApplication(interfaces::Node& node):
QApplication(qt_argc, const_cast<char **>(&qt_argv)),
coreThread(nullptr), coreThread(nullptr),
m_node(node), m_node(node),
optionsModel(nullptr), optionsModel(nullptr),
@ -468,7 +471,7 @@ int GuiMain(int argc, char* argv[])
QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling); QCoreApplication::setAttribute(Qt::AA_EnableHighDpiScaling);
#endif #endif
BitcoinApplication app(*node, argc, argv); BitcoinApplication app(*node);
// Register meta types used for QMetaObject::invokeMethod and Qt::QueuedConnection // Register meta types used for QMetaObject::invokeMethod and Qt::QueuedConnection
qRegisterMetaType<bool*>(); qRegisterMetaType<bool*>();

2
src/qt/dash.h
View File

@ -57,7 +57,7 @@ class BitcoinApplication: public QApplication
{ {
Q_OBJECT Q_OBJECT
public: public:
explicit BitcoinApplication(interfaces::Node& node, int &argc, char **argv); explicit BitcoinApplication(interfaces::Node& node);
~BitcoinApplication(); ~BitcoinApplication();
#ifdef ENABLE_WALLET #ifdef ENABLE_WALLET

5
src/qt/rpcconsole.cpp
View File

@ -1433,11 +1433,6 @@ void RPCConsole::showOrHideBanTableIfRequired()
ui->banHeading->setVisible(visible); ui->banHeading->setVisible(visible);
} }
RPCConsole::TabTypes RPCConsole::tabFocus() const
{
return (TabTypes) ui->stackedWidgetRPC->currentIndex();
}
void RPCConsole::setTabFocus(enum TabTypes tabType) void RPCConsole::setTabFocus(enum TabTypes tabType)
{ {
showPage(tabType); showPage(tabType);

1
src/qt/rpcconsole.h
View File

@ -69,7 +69,6 @@ public:
std::vector<TabTypes> tabs() const { return {TAB_INFO, TAB_CONSOLE, TAB_GRAPH, TAB_PEERS, TAB_REPAIR}; } std::vector<TabTypes> tabs() const { return {TAB_INFO, TAB_CONSOLE, TAB_GRAPH, TAB_PEERS, TAB_REPAIR}; }
TabTypes tabFocus() const;
QString tabTitle(TabTypes tab_type) const; QString tabTitle(TabTypes tab_type) const;
protected: protected:

2
src/qt/test/test_main.cpp
View File

@ -66,7 +66,7 @@ int main(int argc, char *argv[])
// Don't remove this, it's needed to access // Don't remove this, it's needed to access
// QApplication:: and QCoreApplication:: in the tests // QApplication:: and QCoreApplication:: in the tests
BitcoinApplication app(*node, argc, argv); BitcoinApplication app(*node);
app.setApplicationName("Dash-Qt-test"); app.setApplicationName("Dash-Qt-test");
SSL_library_init(); SSL_library_init();

19
src/qt/walletcontroller.cpp
View File

@ -98,6 +98,9 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
// Instantiate model and register it. // Instantiate model and register it.
WalletModel* wallet_model = new WalletModel(std::move(wallet), m_node, m_options_model, nullptr); WalletModel* wallet_model = new WalletModel(std::move(wallet), m_node, m_options_model, nullptr);
// Handler callback runs in a different thread so fix wallet model thread affinity.
wallet_model->moveToThread(thread());
wallet_model->setParent(this);
m_wallets.push_back(wallet_model); m_wallets.push_back(wallet_model);
// WalletModel::startPollBalance needs to be called in a thread managed by // WalletModel::startPollBalance needs to be called in a thread managed by
@ -124,25 +127,11 @@ WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wal
connect(wallet_model, &WalletModel::coinsSent, this, &WalletController::coinsSent); connect(wallet_model, &WalletModel::coinsSent, this, &WalletController::coinsSent);
// Notify walletAdded signal on the GUI thread. // Notify walletAdded signal on the GUI thread.
if (QThread::currentThread() == thread()) { Q_EMIT walletAdded(wallet_model);
addWallet(wallet_model);
} else {
// Handler callback runs in a different thread so fix wallet model thread affinity.
wallet_model->moveToThread(thread());
bool invoked = QMetaObject::invokeMethod(this, "addWallet", Qt::QueuedConnection, Q_ARG(WalletModel*, wallet_model));
assert(invoked);
}
return wallet_model; return wallet_model;
} }
void WalletController::addWallet(WalletModel* wallet_model)
{
// Take ownership of the wallet model and register it.
wallet_model->setParent(this);
Q_EMIT walletAdded(wallet_model);
}
void WalletController::removeAndDeleteWallet(WalletModel* wallet_model) void WalletController::removeAndDeleteWallet(WalletModel* wallet_model)
{ {
// Unregister wallet model. // Unregister wallet model.

3
src/qt/walletcontroller.h
View File

@ -50,9 +50,6 @@ public:
OpenWalletActivity* openWallet(const std::string& name, QWidget* parent = nullptr); OpenWalletActivity* openWallet(const std::string& name, QWidget* parent = nullptr);
void closeWallet(WalletModel* wallet_model, QWidget* parent = nullptr); void closeWallet(WalletModel* wallet_model, QWidget* parent = nullptr);
private Q_SLOTS:
void addWallet(WalletModel* wallet_model);
Q_SIGNALS: Q_SIGNALS:
void walletAdded(WalletModel* wallet_model); void walletAdded(WalletModel* wallet_model);
void walletRemoved(WalletModel* wallet_model); void walletRemoved(WalletModel* wallet_model);

32
src/script/descriptor.cpp
View File

@ -328,10 +328,12 @@ public:
/** Base class for all Descriptor implementations. */ /** Base class for all Descriptor implementations. */
class DescriptorImpl : public Descriptor class DescriptorImpl : public Descriptor
{ {
//! Public key arguments for this descriptor (size 1 for PK, PKH, WPKH; any size of Multisig). //! Public key arguments for this descriptor (size 1 for PK, PKH, WPKH; any size for Multisig).
const std::vector<std::unique_ptr<PubkeyProvider>> m_pubkey_args; const std::vector<std::unique_ptr<PubkeyProvider>> m_pubkey_args;
//! The sub-descriptor argument (nullptr for everything but SH and WSH). //! The sub-descriptor argument (nullptr for everything but SH and WSH).
const std::unique_ptr<DescriptorImpl> m_script_arg; //! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT)
//! and wsh(SCRIPT), and distinct from KEY expressions and ADDR expressions.
const std::unique_ptr<DescriptorImpl> m_subdescriptor_arg;
//! The string name of the descriptor function. //! The string name of the descriptor function.
const std::string m_name; const std::string m_name;
@ -342,10 +344,10 @@ protected:
/** A helper function to construct the scripts for this descriptor. /** A helper function to construct the scripts for this descriptor.
* *
* This function is invoked once for every CScript produced by evaluating * This function is invoked once for every CScript produced by evaluating
* m_script_arg, or just once in case m_script_arg is nullptr. * m_subdescriptor_arg, or just once in case m_subdescriptor_arg is nullptr.
* @param pubkeys The evaluations of the m_pubkey_args field. * @param pubkeys The evaluations of the m_pubkey_args field.
* @param script The evaluation of m_script_arg (or nullptr when m_script_arg is nullptr). * @param script The evaluation of m_subdescriptor_arg (or nullptr when m_subdescriptor_arg is nullptr).
* @param out A FlatSigningProvider to put scripts or public keys in that are necessary to the solver. * @param out A FlatSigningProvider to put scripts or public keys in that are necessary to the solver.
* The script arguments to this function are automatically added, as is the origin info of the provided pubkeys. * The script arguments to this function are automatically added, as is the origin info of the provided pubkeys.
* @return A vector with scriptPubKeys for this descriptor. * @return A vector with scriptPubKeys for this descriptor.
@ -353,12 +355,12 @@ protected:
virtual std::vector<CScript> MakeScripts(const std::vector<CPubKey>& pubkeys, const CScript* script, FlatSigningProvider& out) const = 0; virtual std::vector<CScript> MakeScripts(const std::vector<CPubKey>& pubkeys, const CScript* script, FlatSigningProvider& out) const = 0;
public: public:
DescriptorImpl(std::vector<std::unique_ptr<PubkeyProvider>> pubkeys, std::unique_ptr<DescriptorImpl> script, const std::string& name) : m_pubkey_args(std::move(pubkeys)), m_script_arg(std::move(script)), m_name(name) {} DescriptorImpl(std::vector<std::unique_ptr<PubkeyProvider>> pubkeys, std::unique_ptr<DescriptorImpl> script, const std::string& name) : m_pubkey_args(std::move(pubkeys)), m_subdescriptor_arg(std::move(script)), m_name(name) {}
bool IsSolvable() const override bool IsSolvable() const override
{ {
if (m_script_arg) { if (m_subdescriptor_arg) {
if (!m_script_arg->IsSolvable()) return false; if (!m_subdescriptor_arg->IsSolvable()) return false;
} }
return true; return true;
} }
@ -368,8 +370,8 @@ public:
for (const auto& pubkey : m_pubkey_args) { for (const auto& pubkey : m_pubkey_args) {
if (pubkey->IsRange()) return true; if (pubkey->IsRange()) return true;
} }
if (m_script_arg) { if (m_subdescriptor_arg) {
if (m_script_arg->IsRange()) return true; if (m_subdescriptor_arg->IsRange()) return true;
} }
return false; return false;
} }
@ -389,10 +391,10 @@ public:
} }
ret += std::move(tmp); ret += std::move(tmp);
} }
if (m_script_arg) { if (m_subdescriptor_arg) {
if (pos++) ret += ","; if (pos++) ret += ",";
std::string tmp; std::string tmp;
if (!m_script_arg->ToStringHelper(arg, tmp, priv)) return false; if (!m_subdescriptor_arg->ToStringHelper(arg, tmp, priv)) return false;
ret += std::move(tmp); ret += std::move(tmp);
} }
out = std::move(ret) + ")"; out = std::move(ret) + ")";
@ -421,6 +423,8 @@ public:
// Construct temporary data in `entries` and `subscripts`, to avoid producing output in case of failure. // Construct temporary data in `entries` and `subscripts`, to avoid producing output in case of failure.
for (const auto& p : m_pubkey_args) { for (const auto& p : m_pubkey_args) {
entries.emplace_back(); entries.emplace_back();
// If we have a cache, we don't need GetPubKey to compute the public key.
// Pass in nullptr to signify only origin info is desired.
if (!p->GetPubKey(pos, arg, cache_read ? nullptr : &entries.back().first, entries.back().second)) return false; if (!p->GetPubKey(pos, arg, cache_read ? nullptr : &entries.back().first, entries.back().second)) return false;
if (cache_read) { if (cache_read) {
// Cached expanded public key exists, use it. // Cached expanded public key exists, use it.
@ -437,9 +441,9 @@ public:
} }
} }
std::vector<CScript> subscripts; std::vector<CScript> subscripts;
if (m_script_arg) { if (m_subdescriptor_arg) {
FlatSigningProvider subprovider; FlatSigningProvider subprovider;
if (!m_script_arg->ExpandHelper(pos, arg, cache_read, subscripts, subprovider, cache_write)) return false; if (!m_subdescriptor_arg->ExpandHelper(pos, arg, cache_read, subscripts, subprovider, cache_write)) return false;
out = Merge(out, subprovider); out = Merge(out, subprovider);
} }
@ -449,7 +453,7 @@ public:
pubkeys.push_back(entry.first); pubkeys.push_back(entry.first);
out.origins.emplace(entry.first.GetID(), std::make_pair<CPubKey, KeyOriginInfo>(CPubKey(entry.first), std::move(entry.second))); out.origins.emplace(entry.first.GetID(), std::make_pair<CPubKey, KeyOriginInfo>(CPubKey(entry.first), std::move(entry.second)));
} }
if (m_script_arg) { if (m_subdescriptor_arg) {
for (const auto& subscript : subscripts) { for (const auto& subscript : subscripts) {
out.scripts.emplace(CScriptID(subscript), subscript); out.scripts.emplace(CScriptID(subscript), subscript);
std::vector<CScript> addscripts = MakeScripts(pubkeys, &subscript, out); std::vector<CScript> addscripts = MakeScripts(pubkeys, &subscript, out);

6
src/script/descriptor.h
View File

@ -46,9 +46,9 @@ struct Descriptor {
* *
* pos: the position at which to expand the descriptor. If IsRange() is false, this is ignored. * pos: the position at which to expand the descriptor. If IsRange() is false, this is ignored.
* provider: the provider to query for private keys in case of hardened derivation. * provider: the provider to query for private keys in case of hardened derivation.
* output_script: the expanded scriptPubKeys will be put here. * output_scripts: the expanded scriptPubKeys will be put here.
* out: scripts and public keys necessary for solving the expanded scriptPubKeys will be put here (may be equal to provider). * out: scripts and public keys necessary for solving the expanded scriptPubKeys will be put here (may be equal to provider).
* cache: vector which will be overwritten with cache data necessary to-evaluate the descriptor at this point without access to private keys. * cache: vector which will be overwritten with cache data necessary to evaluate the descriptor at this point without access to private keys.
*/ */
virtual bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, std::vector<unsigned char>* cache = nullptr) const = 0; virtual bool Expand(int pos, const SigningProvider& provider, std::vector<CScript>& output_scripts, FlatSigningProvider& out, std::vector<unsigned char>* cache = nullptr) const = 0;
@ -56,7 +56,7 @@ struct Descriptor {
* *
* pos: the position at which to expand the descriptor. If IsRange() is false, this is ignored. * pos: the position at which to expand the descriptor. If IsRange() is false, this is ignored.
* cache: vector from which cached expansion data will be read. * cache: vector from which cached expansion data will be read.
* output_script: the expanded scriptPubKeys will be put here. * output_scripts: the expanded scriptPubKeys will be put here.
* out: scripts and public keys necessary for solving the expanded scriptPubKeys will be put here (may be equal to provider). * out: scripts and public keys necessary for solving the expanded scriptPubKeys will be put here (may be equal to provider).
*/ */
virtual bool ExpandFromCache(int pos, const std::vector<unsigned char>& cache, std::vector<CScript>& output_scripts, FlatSigningProvider& out) const = 0; virtual bool ExpandFromCache(int pos, const std::vector<unsigned char>& cache, std::vector<CScript>& output_scripts, FlatSigningProvider& out) const = 0;

6
src/wallet/rpcwallet.cpp
View File

@ -361,7 +361,7 @@ static UniValue sendtoaddress(const JSONRPCRequest& request)
" The recipient will receive less amount of Dash than you enter in the amount field."}, " The recipient will receive less amount of Dash than you enter in the amount field."},
{"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"}, {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"},
{"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"}, {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"},
{"conf_target", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
{"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
" \"UNSET\"\n" " \"UNSET\"\n"
" \"ECONOMICAL\"\n" " \"ECONOMICAL\"\n"
@ -891,7 +891,7 @@ static UniValue sendmany(const JSONRPCRequest& request)
}, },
{"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"}, {"use_is", RPCArg::Type::BOOL, /* default */ "false", "Deprecated and ignored"},
{"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"}, {"use_cj", RPCArg::Type::BOOL, /* default */ "false", "Use CoinJoin funds only"},
{"conf_target", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
{"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
" \"UNSET\"\n" " \"UNSET\"\n"
" \"ECONOMICAL\"\n" " \"ECONOMICAL\"\n"
@ -3406,7 +3406,7 @@ static UniValue fundrawtransaction(const JSONRPCRequest& request)
{"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."}, {"vout_index", RPCArg::Type::NUM, RPCArg::Optional::OMITTED, "The zero-based output index, before a change output is added."},
}, },
}, },
{"conf_target", RPCArg::Type::NUM, /* default */ "fallback to wallet's default", "Confirmation target (in blocks)"}, {"conf_target", RPCArg::Type::NUM, /* default */ "wallet default", "Confirmation target (in blocks)"},
{"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n" {"estimate_mode", RPCArg::Type::STR, /* default */ "UNSET", "The fee estimate mode, must be one of:\n"
" \"UNSET\"\n" " \"UNSET\"\n"
" \"ECONOMICAL\"\n" " \"ECONOMICAL\"\n"

79
test/functional/feature_block.py
View File

@ -65,6 +65,10 @@ class CBrokenBlock(CBlock):
def normal_serialize(self): def normal_serialize(self):
return super().serialize() return super().serialize()
DUPLICATE_COINBASE_SCRIPT_SIG = b'\x01\x78' # Valid for block at height 120
class FullBlockTest(BitcoinTestFramework): class FullBlockTest(BitcoinTestFramework):
def set_test_params(self): def set_test_params(self):
self.num_nodes = 1 self.num_nodes = 1
@ -95,6 +99,13 @@ class FullBlockTest(BitcoinTestFramework):
self.spendable_outputs = [] self.spendable_outputs = []
# Create a new block # Create a new block
b_dup_cb = self.next_block('dup_cb')
b_dup_cb.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG
b_dup_cb.vtx[0].rehash()
duplicate_tx = b_dup_cb.vtx[0]
b_dup_cb = self.update_block('dup_cb', [])
self.send_blocks([b_dup_cb])
b0 = self.next_block(0) b0 = self.next_block(0)
self.save_spendable_output() self.save_spendable_output()
self.send_blocks([b0]) self.send_blocks([b0])
@ -717,7 +728,7 @@ class FullBlockTest(BitcoinTestFramework):
# Test a few invalid tx types # Test a few invalid tx types
# #
# -> b35 (10) -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) # -> b35 (10) -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 ()
# \-> ??? (17) # \-> ??? (17)
# #
@ -743,14 +754,14 @@ class FullBlockTest(BitcoinTestFramework):
# reset to good chain # reset to good chain
self.move_tip(57) self.move_tip(57)
b60 = self.next_block(60, spend=out[17]) b60 = self.next_block(60)
self.send_blocks([b60], True) self.send_blocks([b60], True)
self.save_spendable_output() self.save_spendable_output()
# Test BIP30 # Test BIP30 (reject duplicate)
# #
# -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) # -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 ()
# \-> b61 (18) # \-> b61 ()
# #
# Blocks are not allowed to contain a transaction whose id matches that of an earlier, # Blocks are not allowed to contain a transaction whose id matches that of an earlier,
# not-fully-spent transaction in the same chain. To test, make identical coinbases; # not-fully-spent transaction in the same chain. To test, make identical coinbases;
@ -758,20 +769,44 @@ class FullBlockTest(BitcoinTestFramework):
# #
self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)") self.log.info("Reject a block with a transaction with a duplicate hash of a previous transaction (BIP30)")
self.move_tip(60) self.move_tip(60)
b61 = self.next_block(61, spend=out[18]) b61 = self.next_block(61)
b61.vtx[0].vin[0].scriptSig = b60.vtx[0].vin[0].scriptSig # Equalize the coinbases b61.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG
b61.vtx[0].rehash() b61.vtx[0].rehash()
b61 = self.update_block(61, []) b61 = self.update_block(61, [])
assert_equal(b60.vtx[0].serialize(), b61.vtx[0].serialize()) assert_equal(duplicate_tx.serialize(), b61.vtx[0].serialize())
self.send_blocks([b61], success=False, reject_reason='bad-txns-BIP30', reconnect=True) self.send_blocks([b61], success=False, reject_reason='bad-txns-BIP30', reconnect=True)
# Test BIP30 (allow duplicate if spent)
#
# -> b57 (16) -> b60 ()
# \-> b_spend_dup_cb (b_dup_cb) -> b_dup_2 ()
#
self.move_tip(57)
b_spend_dup_cb = self.next_block('spend_dup_cb')
tx = CTransaction()
tx.vin.append(CTxIn(COutPoint(duplicate_tx.sha256, 0)))
tx.vout.append(CTxOut(0, CScript([OP_TRUE])))
self.sign_tx(tx, duplicate_tx)
tx.rehash()
b_spend_dup_cb = self.update_block('spend_dup_cb', [tx])
b_dup_2 = self.next_block('dup_2')
b_dup_2.vtx[0].vin[0].scriptSig = DUPLICATE_COINBASE_SCRIPT_SIG
b_dup_2.vtx[0].rehash()
b_dup_2 = self.update_block('dup_2', [])
assert_equal(duplicate_tx.serialize(), b_dup_2.vtx[0].serialize())
assert_equal(self.nodes[0].gettxout(txid=duplicate_tx.hash, n=0)['confirmations'], 119)
self.send_blocks([b_spend_dup_cb, b_dup_2], success=True)
# The duplicate has less confirmations
assert_equal(self.nodes[0].gettxout(txid=duplicate_tx.hash, n=0)['confirmations'], 1)
# Test tx.isFinal is properly rejected (not an exhaustive tx.isFinal test, that should be in data-driven transaction tests) # Test tx.isFinal is properly rejected (not an exhaustive tx.isFinal test, that should be in data-driven transaction tests)
# #
# -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) # -> b_spend_dup_cb (b_dup_cb) -> b_dup_2 ()
# \-> b62 (18) # \-> b62 (18)
# #
self.log.info("Reject a block with a transaction with a nonfinal locktime") self.log.info("Reject a block with a transaction with a nonfinal locktime")
self.move_tip(60) self.move_tip('dup_2')
b62 = self.next_block(62) b62 = self.next_block(62)
tx = CTransaction() tx = CTransaction()
tx.nLockTime = 0xffffffff # this locktime is non-final tx.nLockTime = 0xffffffff # this locktime is non-final
@ -784,11 +819,11 @@ class FullBlockTest(BitcoinTestFramework):
# Test a non-final coinbase is also rejected # Test a non-final coinbase is also rejected
# #
# -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) # -> b_spend_dup_cb (b_dup_cb) -> b_dup_2 ()
# \-> b63 (-) # \-> b63 (-)
# #
self.log.info("Reject a block with a coinbase transaction with a nonfinal locktime") self.log.info("Reject a block with a coinbase transaction with a nonfinal locktime")
self.move_tip(60) self.move_tip('dup_2')
b63 = self.next_block(63) b63 = self.next_block(63)
b63.vtx[0].nLockTime = 0xffffffff b63.vtx[0].nLockTime = 0xffffffff
b63.vtx[0].vin[0].nSequence = 0xDEADBEEF b63.vtx[0].vin[0].nSequence = 0xDEADBEEF
@ -804,14 +839,14 @@ class FullBlockTest(BitcoinTestFramework):
# What matters is that the receiving node should not reject the bloated block, and then reject the canonical # What matters is that the receiving node should not reject the bloated block, and then reject the canonical
# block on the basis that it's the same as an already-rejected block (which would be a consensus failure.) # block on the basis that it's the same as an already-rejected block (which would be a consensus failure.)
# #
# -> b39 (11) -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) # -> b_spend_dup_cb (b_dup_cb) -> b_dup_2 () -> b64 (18)
# \ # \
# b64a (18) # b64a (18)
# b64a is a bloated block (non-canonical varint) # b64a is a bloated block (non-canonical varint)
# b64 is a good block (same as b64 but w/ canonical varint) # b64 is a good block (same as b64 but w/ canonical varint)
# #
self.log.info("Accept a valid block even if a bloated version of the block has previously been sent") self.log.info("Accept a valid block even if a bloated version of the block has previously been sent")
self.move_tip(60) self.move_tip('dup_2')
regular_block = self.next_block("64a", spend=out[18]) regular_block = self.next_block("64a", spend=out[18])
# make it a "broken_block," with non-canonical serialization # make it a "broken_block," with non-canonical serialization
@ -837,7 +872,7 @@ class FullBlockTest(BitcoinTestFramework):
node.disconnect_p2ps() node.disconnect_p2ps()
self.reconnect_p2p() self.reconnect_p2p()
self.move_tip(60) self.move_tip('dup_2')
b64 = CBlock(b64a) b64 = CBlock(b64a)
b64.vtx = copy.deepcopy(b64a.vtx) b64.vtx = copy.deepcopy(b64a.vtx)
assert_equal(b64.hash, b64a.hash) assert_equal(b64.hash, b64a.hash)
@ -849,7 +884,7 @@ class FullBlockTest(BitcoinTestFramework):
# Spend an output created in the block itself # Spend an output created in the block itself
# #
# -> b42 (12) -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) # -> b_dup_2 () -> b64 (18) -> b65 (19)
# #
self.log.info("Accept a block with a transaction spending an output created in the same block") self.log.info("Accept a block with a transaction spending an output created in the same block")
self.move_tip(64) self.move_tip(64)
@ -862,7 +897,7 @@ class FullBlockTest(BitcoinTestFramework):
# Attempt to spend an output created later in the same block # Attempt to spend an output created later in the same block
# #
# -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) # -> b64 (18) -> b65 (19)
# \-> b66 (20) # \-> b66 (20)
self.log.info("Reject a block with a transaction spending an output created later in the same block") self.log.info("Reject a block with a transaction spending an output created later in the same block")
self.move_tip(65) self.move_tip(65)
@ -874,7 +909,7 @@ class FullBlockTest(BitcoinTestFramework):
# Attempt to double-spend a transaction created in a block # Attempt to double-spend a transaction created in a block
# #
# -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) # -> b64 (18) -> b65 (19)
# \-> b67 (20) # \-> b67 (20)
# #
# #
@ -889,7 +924,7 @@ class FullBlockTest(BitcoinTestFramework):
# More tests of block subsidy # More tests of block subsidy
# #
# -> b43 (13) -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) -> b69 (20) # -> b64 (18) -> b65 (19) -> b69 (20)
# \-> b68 (20) # \-> b68 (20)
# #
# b68 - coinbase with an extra 10 satoshis, # b68 - coinbase with an extra 10 satoshis,
@ -916,7 +951,7 @@ class FullBlockTest(BitcoinTestFramework):
# Test spending the outpoint of a non-existent transaction # Test spending the outpoint of a non-existent transaction
# #
# -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) -> b69 (20) # -> b65 (19) -> b69 (20)
# \-> b70 (21) # \-> b70 (21)
# #
self.log.info("Reject a block containing a transaction spending from a non-existent input") self.log.info("Reject a block containing a transaction spending from a non-existent input")
@ -932,7 +967,7 @@ class FullBlockTest(BitcoinTestFramework):
# Test accepting an invalid block which has the same hash as a valid one (via merkle tree tricks) # Test accepting an invalid block which has the same hash as a valid one (via merkle tree tricks)
# #
# -> b53 (14) -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) -> b69 (20) -> b72 (21) # -> b65 (19) -> b69 (20) -> b72 (21)
# \-> b71 (21) # \-> b71 (21)
# #
# b72 is a good block. # b72 is a good block.
@ -961,7 +996,7 @@ class FullBlockTest(BitcoinTestFramework):
# Test some invalid scripts and MAX_BLOCK_SIGOPS # Test some invalid scripts and MAX_BLOCK_SIGOPS
# #
# -> b55 (15) -> b57 (16) -> b60 (17) -> b64 (18) -> b65 (19) -> b69 (20) -> b72 (21) # -> b69 (20) -> b72 (21)
# \-> b** (22) # \-> b** (22)
# #

5
test/functional/rpc_psbt.py
View File

@ -17,7 +17,10 @@ class PSBTTest(BitcoinTestFramework):
self.setup_clean_chain = False self.setup_clean_chain = False
self.num_nodes = 3 self.num_nodes = 3
# TODO: remove -txindex. Currently required for getrawtransaction call. # TODO: remove -txindex. Currently required for getrawtransaction call.
self.extra_args = [[], ["-txindex"], ["-txindex"]] self.extra_args = [[],
["-txindex"],
["-txindex"]
]
def skip_test_if_missing_module(self): def skip_test_if_missing_module(self):
self.skip_if_no_wallet() self.skip_if_no_wallet()

27
test/functional/test_framework/blocktools.py
View File

@ -12,9 +12,8 @@ from .messages import (
CTransaction, CTransaction,
CTxIn, CTxIn,
CTxOut, CTxOut,
ser_string,
) )
from .script import CScript, OP_TRUE, OP_CHECKSIG from .script import CScript, CScriptNum, CScriptOp, OP_TRUE, OP_CHECKSIG
from .util import assert_equal, hex_str_to_bytes from .util import assert_equal, hex_str_to_bytes
from io import BytesIO from io import BytesIO
@ -33,20 +32,13 @@ def create_block(hashprev, coinbase, ntime=None):
block.calc_sha256() block.calc_sha256()
return block return block
def serialize_script_num(value): def script_BIP34_coinbase_height(height):
r = bytearray(0) if height <= 16:
if value == 0: res = CScriptOp.encode_op_n(height)
return r # Append dummy to increase scriptSig size above 2 (see bad-cb-length consensus rule)
neg = value < 0 return CScript([res, OP_TRUE])
absvalue = -value if neg else value return CScript([CScriptNum(height)])
while (absvalue):
r.append(int(absvalue & 0xff))
absvalue >>= 8
if r[-1] & 0x80:
r.append(0x80 if neg else 0)
elif neg:
r[-1] |= 0x80
return r
def create_coinbase(height, pubkey=None, dip4_activated=False): def create_coinbase(height, pubkey=None, dip4_activated=False):
"""Create a coinbase transaction, assuming no miner fees. """Create a coinbase transaction, assuming no miner fees.
@ -54,8 +46,7 @@ def create_coinbase(height, pubkey=None, dip4_activated=False):
If pubkey is passed in, the coinbase output will be a P2PK output; If pubkey is passed in, the coinbase output will be a P2PK output;
otherwise an anyone-can-spend output.""" otherwise an anyone-can-spend output."""
coinbase = CTransaction() coinbase = CTransaction()
coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), coinbase.vin.append(CTxIn(COutPoint(0, 0xffffffff), script_BIP34_coinbase_height(height), 0xffffffff))
ser_string(serialize_script_num(height)), 0xffffffff))
coinbaseoutput = CTxOut() coinbaseoutput = CTxOut()
coinbaseoutput.nValue = 500 * COIN coinbaseoutput.nValue = 500 * COIN
halvings = int(height / 150) # regtest halvings = int(height / 150) # regtest