6242beeb06 Hoist repeated translated strings to RPCConsole struct members (Jon Atack)
0f035c12fb RPCConsole::updateDetailWidget: convert strings to translated strings (Jon Atack)
Pull request description:
- fixups from #206 review feedback (thanks!), see commit message for details
- hoists repeatedly used translatable strings to the `RPCConsole` class for reuse
ACKs for top commit:
hebasto:
re-ACK 6242beeb06
Talkless:
tACK 6242beeb06, tested on Debian Sid with Qt 5.15.2. I see "Ban for.." translated to my native language as before, "To/From/Yes/No" are not but that's expected, as `.ts` files are not updated.
jarolrod:
ACK 6242beeb06
Tree-SHA512: 20a296511c5ac03a816766237fa2731b0360dedebf1bea02711eb21d7e4eae2a63a051fe48f4726052edc3e6318952f01fef920cd4b22a8196c39c23d8e5cc3a
1d5d832d5c qt, refactor: Use enum type as switch argument in TransactionTableModel (Hennadii Stepanov)
52f122c11f qt, refactor: Use enum type as switch argument in PeerTableModel (Hennadii Stepanov)
a35223f1cd qt, refactor: Use enum type as switch argument in BanTableModel (Hennadii Stepanov)
ab8a747d1c qt, refactor: Use enum type as switch argument in AddressTableModel (Hennadii Stepanov)
Pull request description:
This PR makes code more maintainable by leveraging `-Wswitch` compiler warnings.
Only the `RecentRequestsTableModel` is not refactored, because its `enum ColumnIndex` contains additional `NUMBER_OF_COLUMNS` value.
No behavior change.
ACKs for top commit:
hebasto:
Do you mind mentioning the _top_ pr commit with your ACK, i.e., 1d5d832d5c, not ab8a747d1ced9f20ca32f9898418be70670da71a?
jarolrod:
ACK 1d5d832d5c, tested on macOS 11.1 Qt 5.15.2
leonardojobim:
ACK 1d5d832d5c, tested on Ubuntu 20.04 Qt 5.12.8
promag:
Code review ACK 1d5d832d5c.
Tree-SHA512: 0d474d226a2fa0069495d1aa5ec13b2470708ec7b8a6ab35402236c7bf57cb9939577677a30dfa54f5e3bc0477c6cfffd20ed6f19e4eb394a938569cc9347851
0c471a5f30 tests: check never active versionbits (Anthony Towns)
3ba9283a47 tests: more helpful errors for failing versionbits tests (Anthony Towns)
Pull request description:
Extracted from #19573 to make review easier. I also reviewed it myself.
I added some comments to the test: bae9a45219 (r585486781)
I also moved some `TestState` changes from the second to the first commit, to reduce the latter diff.
ACKs for top commit:
ajtowns:
ACK 0c471a5f30
MarcoFalke:
review ACK 0c471a5f30🔓
Tree-SHA512: 61f8d1ecaf38a6cd13db1cf71c89b8c4d2f5852ef77c5e7ecb9bd78eb216545037411641bb101cf0740c5c47845ac327954ee25b676d63779c5f148719ac5caf
The qt tests don't need to bind to the regtest port. By not binding, it
will no longer conflict with existing regtest instances and the tests
will run as normal.
7af25024e9 build: compile libnatpmp with -DNATPMP_STATICLIB on Windows (fanquake)
ee35745754 build: use newer source for libnatpmp (fanquake)
Pull request description:
The source we are currently using is from 2015. The upstream repo has
received a small number of bug fixes and improvements since then.
Including one that fixes an issue for Windows users: https://github.com/miniupnp/libnatpmp/pull/13.
The source we are currently using is the most recent "official" release,
however I don't think it's worth waiting for a new one. The maintainer
was prompted to do so in Oct 2020, then again in Jan of this year, and
no release has eventuated. Given libnatpmp is a new inclusion into our
repository, I think we should be using this newer source.
This also cleans up a few warnings we currently see in Windows depends builds:
```bash
Extracting libnatpmp...
/home/ubuntu/bitcoin/depends/sources/libnatpmp-20150609.tar.gz: OK
Preprocessing libnatpmp...
Configuring libnatpmp...
Building libnatpmp...
make[1]: Entering directory '/home/ubuntu/bitcoin/depends/work/build/x86_64-w64-mingw32/libnatpmp/20150609-13efa1beb87'
x86_64-w64-mingw32-gcc -Os -fPIC -Wall -DENABLE_STRNATPMPERR -c -o natpmp.o natpmp.c
x86_64-w64-mingw32-gcc -Os -fPIC -Wall -DENABLE_STRNATPMPERR -c -o getgateway.o getgateway.c
natpmp.c:42: warning: "EWOULDBLOCK" redefined
42 | #define EWOULDBLOCK WSAEWOULDBLOCK
|
In file included from natpmp.c:38:
/usr/share/mingw-w64/include/errno.h:166: note: this is the location of the previous definition
166 | #define EWOULDBLOCK 140
|
natpmp.c:43: warning: "ECONNREFUSED" redefined
43 | #define ECONNREFUSED WSAECONNREFUSED
|
In file included from natpmp.c:38:
/usr/share/mingw-w64/include/errno.h:110: note: this is the location of the previous definition
110 | #define ECONNREFUSED 107
|
natpmp.c:271:5: warning: ‘readnatpmpresponseorretry’ redeclared without dllimport attribute: previous dllimport ignored [-Wattributes]
271 | int readnatpmpresponseorretry(natpmp_t * p, natpmpresp_t * response)
| ^~~~~~~~~~~~~~~~~~~~~~~~~
ar crs libnatpmp.a natpmp.o getgateway.o
make[1]: Leaving directory '/home/ubuntu/bitcoin/depends/work/build/x86_64-w64-mingw32/libnatpmp/20150609-13efa1beb87'
Staging libnatpmp...
Postprocessing libnatpmp...
Caching libnatpmp...
```
ACKs for top commit:
hebasto:
ACK 7af25024e9
Tree-SHA512: 6939014ea986149a5bfdd42b516d563a65ae643516e234579d3f28e7c2f877b0270cc4305ae7c7cb131d6d946a6e0aedc84b4cc880a412612a878a333398b9d7
67c59ae479 qt: Make warning label look clickable (Jarol Rodriguez)
Pull request description:
The warning icon on the overview page indicates that there is something important the user should know about, but a user may not be aware that they can click it because, on `master`, the warning label does not look clickable. As detailed in issue #23, the reason to make it look clickable is that it if they "had a more clickable-appearance (borders or beveled button edges) it could help users more quickly understand what they are being alerted to."
This PR removes the `flat` property from both `QPushButton`'s to make them look like a button, and therefore clickable. Furthermore, it updates the `Maximum Width` to `45` to fix the small hit-box issue outlined in issue #215.
Below are screenshots showing how the warning icon looks under `master` and this `PR`:
**macOS 11.1: Qt 5.15**
| Master | PR |
| ----------- | ----------- |
| <img width="754" alt="Screen Shot 2021-02-22 at 5 00 40 PM" src="https://user-images.githubusercontent.com/23396902/108776135-f6d50380-752f-11eb-9f96-25163c6a2a02.png"> | <img width="754" alt="Screen Shot 2021-02-22 at 3 08 40 PM" src="https://user-images.githubusercontent.com/23396902/108776068-e0c74300-752f-11eb-9545-3580e2b8f187.png"> |
**Ubuntu 20.04: Qt 5.12**
| Master | PR |
| ----------- | ----------- |
| <img width="783" alt="Screen Shot 2021-02-22 at 4 57 32 PM" src="https://user-images.githubusercontent.com/23396902/108776249-284dcf00-7530-11eb-8325-7fe13a9243a7.png"> |  |
Closes#23Closes#215
ACKs for top commit:
Talkless:
tACK 67c59ae479, tested on Debian Sid. Does look as expected.
Tree-SHA512: 2b7302fb990ea49e2f01df6f4a23e2bc3de0797da89deaeb299742e6b285a0c21ea80d8259dc0222640cccc2bccc4ea09df443b9a11bf8b88a828e5fb2aec12c
6a0a6e7d05 Correction for VerifyTaprootCommitment comments (Russell O'Connor)
Pull request description:
According to BIP-341, 'p' is called the taproot *internal* key, not inner key.
ACKs for top commit:
sipa:
ACK 6a0a6e7d05
benthecarman:
ACK 6a0a6e7d05
theStack:
ACK 6a0a6e7d05
Tree-SHA512: 94f553476a8404bff4b2d5724a1a54c5f530b987a616cd00a3800095f245c06e3c7a9066c729976f32069a56029406859a70ba523151d333dc1ed874f242bce8
This adds a `-daemonwait` flag that does the same as `-daemon` except
it, from a user perspective, backgrounds the process only after
initialization is complete.
This can be useful when the process launching bitcoind wants to
guarantee that either the RPC server is running, or that initialization
failed, before continuing. The exit code indicates the initialization
result.
This replaces the use of the libc function `daemon()` by a custom
implementation which is inspired by the glibc implementation, but also
creates a pipe from the child to the parent process for communication.
An additional advantage of having our own `daemon()` implementation is
that no MACOS-specific pragmas are needed anymore to silence a
deprecation warning.
e11b649650 validation: CVerifyDB::VerifyDB: Use locking annotation (Carl Dong)
03f75c42e1 validation: Use existing chain member in CChainState::LoadGenesisBlock (Carl Dong)
5e4af77380 validation: Use existing chain member in CChainState::AcceptBlock (Carl Dong)
fee73347c0 validation: Pass in chain to FindBlockPos+SaveBlockToDisk (Carl Dong)
a9d28bcd8d validation: Use *this in CChainState::ActivateBestChainStep (Carl Dong)
4744efc9ba validation: Pass in chainstate to CTxMemPool::check (Carl Dong)
1fb7b2c595 validation: Use *this in CChainState::InvalidateBlock (Carl Dong)
8cdb2f7e58 validation: Move LoadBlockIndexDB to CChainState (Carl Dong)
8b99efbcc0 validation: Move invalid block handling to CChainState (Carl Dong)
2bdf37fe18 validation: Pass in chainstate to CVerifyDB::VerifyDB (Carl Dong)
31eac50c72 validation: Remove global ::VersionBitsTip{State,SinceHeight,Statistics} (Carl Dong)
63e4c7316a validation: Pass in chainstate to ::PruneBlockFilesManual (Carl Dong)
4bada76237 validation: Pass in chainstate to UpdateTip (Carl Dong)
a3ba08ba7d validation: Remove global ::{{Precious,Invalidate}Block,ResetBlockFailureFlags} (Carl Dong)
4927c9e699 validation: Remove global ::LoadGenesisBlock (Carl Dong)
9da106be4d validation: Check chain tip is non-null in CheckFinalTx (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
Based on:
- [x] #20750 | [Bundle 2/n] Prune g_chainman usage in mempool-related validation functions
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
Note to self:
- [x] Address: https://github.com/bitcoin/bitcoin/pull/20750#discussion_r579400663
ACKs for top commit:
laanwj:
Code review ACK e11b649650
Tree-SHA512: 205a451a741e32f17d5966de289f2f5a3f0817738c0087b70ff4755ddd217b53d01050ed396669bda2b1d216a88d927b9778777f9ff95ab1fe20e59c5f341776
fa59ad5130 fuzz: Add missing include (test/util/setup_common.h) (MarcoFalke)
Pull request description:
`src/test/fuzz/socks5.cpp` is using the symbol `BasicTestingSetup`, which is defined in `src/test/util/setup_common.h`.
Currently compilation happens to succeed because the needed dependency is indirectly included. Compilation will break as soon as the indirect dependency is broken. According to the dev notes, everything that is used must be included.
Fix the issue by including the missing include.
ACKs for top commit:
fanquake:
ACK fa59ad5130
Tree-SHA512: 9359d5d288ebc5a53d753ebed1ee8d49ddcfe12aeb56054ea43654c0d915337bb0dce7c8a7178e94711ff8dacd1b3ea0a2871b21b1709cd9786efc0c1ef532b3
0eaea66e8b Make tx relay data structure use std::chrono types (Pieter Wuille)
55e82881a1 Make all Poisson delays use std::chrono types (Pieter Wuille)
c733ac4d8a Convert block/header sync timeouts to std::chrono types (Pieter Wuille)
4d98b401fb Change all ping times to std::chrono types (Pieter Wuille)
Pull request description:
(Picking up #20044. Rebased against master.)
This changes various uses of integers to represent timestamps and durations to `std::chrono` duration types with type-safe conversions, getting rid of various `.count()`, constructors, and conversion factors.
ACKs for top commit:
jnewbery:
utACK 0eaea66e8b
vasild:
ACK 0eaea66e8b
MarcoFalke:
re-ACK 0eaea66e8b, only changes: minor rename, using C++11 member initializer, using 2min chrono literal, rebase 🤚
ajtowns:
utACK 0eaea66e8b
Tree-SHA512: 2dbd8d53bf82e98f9b4611e61dc14c448e8957d1a02575b837fadfd59f80e98614d0ccf890fc351f960ade76a6fb8051b282e252e81675a8ee753dba8b1d7f57
We don't mark RelayTransaction as const. Even though it doesn't mutate
PeerManagerImpl state, it _is_ mutating the internal state of a CNode
object, by updating setInventoryTxToSend. In a subsequent commit, that
field will be moved to the Peer object, which is owned by
PeerMangerImpl.
This requires PeerManagerImpl::ReattemptInitialBroadcast() to no longer
be const.
1f05dbd06d util: Avoid invalid integer negation in ValueFromAmount: make ValueFromAmount(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)
7cc75c9ba3 util: Avoid invalid integer negation in FormatMoney: make FormatMoney(const CAmount& n) well-defined also when n is std::numeric_limits<CAmount>::min() (practicalswift)
Pull request description:
Avoid invalid integer negation in `FormatMoney` and `ValueFromAmount`.
Fixes#20402.
Before this patch:
```
$ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
$ make -C src/ test/test_bitcoin
$ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
core_write.cpp:21:29: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
(aka 'long'); cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior core_write.cpp:21:29 in
test/rpc_tests.cpp(186): error: in "rpc_tests/rpc_format_monetary_values":
check ValueFromAmount(std::numeric_limits<CAmount>::min()).write() == "-92233720368.54775808" has failed
[--92233720368.-54775808 != -92233720368.54775808]
util/moneystr.cpp:16:34: runtime error: negation of -9223372036854775808 cannot be represented in type 'CAmount'
(aka 'long'); cast to an unsigned type to negate this value to itself
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior util/moneystr.cpp:16:34 in
test/util_tests.cpp(1188): error: in "util_tests/util_FormatMoney":
check FormatMoney(std::numeric_limits<CAmount>::min()) == "-92233720368.54775808" has failed
[--92233720368.-54775808 != -92233720368.54775808]
```
After this patch:
```
$ CC=clang CXX=clang++ ./configure --with-sanitizers=undefined
$ make -C src/ test/test_bitcoin
$ src/test/test_bitcoin -t rpc_tests/rpc_format_monetary_values -t util_tests/util_FormatMoney
```
ACKs for top commit:
laanwj:
re-ACK 1f05dbd06d
Tree-SHA512: 5aaeb8e2178f1597921f53c12bdfc2f3d5993d10c41658dcd25943e54e8cc2116a411bc71d928f890b33bc0b3761a8ee4449b0532bce41125b6c60692808c8c3
7d3343fb8e cli: update -netinfo help doc following the merge of 882ce251 (Jon Atack)
ef614bb408 cli: small -netinfo simplification and performance improvement (Jon Atack)
6b45ef3233 cli: improve -netinfo invalid argument error message (Jon Atack)
3732404afa cli: warn in help that -netinfo is not intended to be a stable API (Jon Atack)
7afdd72258 cli: enable -netinfo help to run without a remote server (Jon Atack)
Pull request description:
A few updates, some per IRC discussion today at http://www.erisian.com.au/bitcoin-core-dev/log-2021-01-07.html#l-87 with respect to -netinfo:
- enable `-netinfo help` to run without a remote server
- warn in `-netinfo help` that -netinfo is not intended to be a stable API
- improve the -netinfo invalid argument error message
- make a performance improvement and simplification I noticed after the merge of #20764
- update the -netinfo help doc following the merge of #21192
-----
How to test manually: 🔬🧪📈
1. check out and build this branch locally; if you need help, don't hesitate to refer to https://jonatack.github.io/articles/how-to-review-pull-requests-in-bitcoin-core#pull-down-the-code-locally or https://jonatack.github.io/articles/how-to-compile-bitcoin-core-and-run-the-tests
2. while it is compiling, look at the code changes
3. stop signet (if it is running) with `./src/bitcoin-cli -signet stop`
4. once the build is completed, run `./src/bitcoin-cli -signet -netinfo help`
5. the help should be printed even though the signet server is not running
6. near the top you should see the new warning, "This human-readable interface will change regularly and is not intended to be a stable API" as well as a bit more description about the integer argument values.
7. start signet with `./src/bitcoind -signet`
8. test the improved invalid argument error message if you run `./src/bitcoin-cli -signet -netinfo 256` or `./src/bitcoin-cli -signet -netinfo a` (valid values are from 0 to 255)
9. leave review feedback or `ACK <commit hash>` -- done 🍻
ACKs for top commit:
michaelfolkson:
Re-ACK 7d3343fb8e
pinheadmz:
RE-ACK 7d3343fb8e
Tree-SHA512: 28c5e9f295ffccba5c2a70faac4987d45f35d4758cf8f10daa767e83212316c4cfc65930e4066f7ad627e9d15b92d43439d1ba9c2f755dfde61885c6a70aa155
366e3e1f89 fuzz: Add FUZZED_SOCKET_FAKE_LATENCY mode to FuzzedSock to allow for fuzzing timeout logic (practicalswift)
b22d4c1607 fuzz: Add fuzzing harness for Socks5(...) (practicalswift)
Pull request description:
Add [regression fuzz harness](https://twitter.com/kayseesee/status/1205287895923212289) for CVE-2017-18350. This fuzzing harness would have found CVE-2017-18350 within a minute of fuzzing :)
See [`doc/fuzzing.md`](https://github.com/bitcoin/bitcoin/blob/master/doc/fuzzing.md) for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the [Bitcoin Core fuzzing corpus repo](https://github.com/bitcoin-core/qa-assets).
Happy fuzzing :)
ACKs for top commit:
vasild:
ACK 366e3e1f89
Tree-SHA512: 5d8e1863b635efd10ccb11678b71472ba1523c3ef16affa7f9cd638635c1a9c307e28f432d5b87eb0c9cd1c3c1aeafbb24fa7ae86fe4e5090fda2e20d542b6ca
a701fcf01f net: Do not skip the I2P network from GetNetworkNames() (Vasil Dimov)
0181e24439 net: recognize I2P from ParseNetwork() so that -onlynet=i2p works (Vasil Dimov)
b905363fa8 net: accept incoming I2P connections from CConnman (Vasil Dimov)
0635233a1e net: make outgoing I2P connections from CConnman (Vasil Dimov)
9559bd1404 net: add I2P to the reachability map (Vasil Dimov)
76c35c60f3 init: introduce I2P connectivity options (Vasil Dimov)
c22daa2ecf net: implement the necessary parts of the I2P SAM protocol (Vasil Dimov)
5bac7e45e1 net: extend Sock with a method to check whether connected (Vasil Dimov)
42c779f503 net: extend Sock with methods for robust send & read until terminator (Vasil Dimov)
ea1845315a net: extend Sock::Wait() to report a timeout (Vasil Dimov)
78fdfbea66 net: dedup MSG_NOSIGNAL and MSG_DONTWAIT definitions (Vasil Dimov)
34bcfab562 net: move the constant maxWait out of InterruptibleRecv() (Vasil Dimov)
cff65c4a27 net: extend CNetAddr::SetSpecial() to support I2P (Vasil Dimov)
f6c267db3b net: avoid unnecessary GetBindAddress() call (Vasil Dimov)
7c224fdac4 net: isolate the protocol-agnostic part of CConnman::AcceptConnection() (Vasil Dimov)
1f75a653dd net: get the bind address earlier in CConnman::AcceptConnection() (Vasil Dimov)
25605895af net: check for invalid socket earlier in CConnman::AcceptConnection() (Vasil Dimov)
545bc5f81d util: fix WriteBinaryFile() claiming success even if error occurred (Vasil Dimov)
8b6e4b3b23 util: fix ReadBinaryFile() returning partial contents (Vasil Dimov)
4cba2fdafa util: extract {Read,Write}BinaryFile() to its own files (Vasil Dimov)
Pull request description:
Add I2P support by using the [I2P SAM](https://geti2p.net/en/docs/api/samv3) protocol. Unlike Tor, for incoming connections we get the I2P address of the peer (and they also receive ours when we are the connection initiator).
Two new options are added:
```
-i2psam=<ip:port>
I2P SAM proxy to reach I2P peers and accept I2P connections (default:
none)
-i2pacceptincoming
If set and -i2psam is also set then incoming I2P connections are
accepted via the SAM proxy. If this is not set but -i2psam is set
then only outgoing connections will be made to the I2P network.
Ignored if -i2psam is not set. Notice that listening for incoming
I2P connections is done through the SAM proxy, not by binding to
a local address and port (default: true)
```
# Overview of the changes
## Make `ReadBinary()` and `WriteBinary()` reusable
We would need to dump the I2P private key to a file and read it back later. Move those two functions out of `torcontrol.cpp`.
```
util: extract {Read,Write}BinaryFile() to its own files
util: fix ReadBinaryFile() returning partial contents
util: fix WriteBinaryFile() claiming success even if error occurred
```
## Split `CConnman::AcceptConnection()`
Most of `CConnman::AcceptConnection()` is agnostic of how the socket was accepted. The other part of it deals with the details of the `accept(2)` system call. Split those so that the protocol-agnostic part can be reused if we accept a socket by other means.
```
net: check for invalid socket earlier in CConnman::AcceptConnection()
net: get the bind address earlier in CConnman::AcceptConnection()
net: isolate the protocol-agnostic part of CConnman::AcceptConnection()
net: avoid unnecessary GetBindAddress() call
```
## Implement the I2P [SAM](https://geti2p.net/en/docs/api/samv3) protocol (not all of it)
Just the parts that would enable us to make outgoing and accept incoming I2P connections.
```
net: extend CNetAddr::SetSpecial() to support I2P
net: move the constant maxWait out of InterruptibleRecv()
net: dedup MSG_NOSIGNAL and MSG_DONTWAIT definitions
net: extend Sock::Wait() to report a timeout
net: extend Sock with methods for robust send & read until terminator
net: extend Sock with a method to check whether connected
net: implement the necessary parts of the I2P SAM protocol
```
## Use I2P SAM to connect to and accept connections from I2P peers
Profit from all of the preceding commits.
```
init: introduce I2P connectivity options
net: add I2P to the reachability map
net: make outgoing I2P connections from CConnman
net: accept incoming I2P connections from CConnman
net: recognize I2P from ParseNetwork() so that -onlynet=i2p works
net: Do not skip the I2P network from GetNetworkNames()
```
ACKs for top commit:
laanwj:
re-ACK a701fcf01f
jonatack:
re-ACK a701fcf01f reviewed diff per `git range-diff ad89812 2a7bb34 a701fcf`, debug built and launched bitcoind with i2pd v2.35 running a dual I2P+Torv3 service with the I2P config settings listed below (did not test `onlynet=i2p`); operation appears nominal (same as it has been these past weeks), and tested the bitcoind help outputs grepping for `-i i2p` and the rpc getpeerinfo and getnetworkinfo helps
Tree-SHA512: de42090c9c0bf23b43b5839f5b4fc4b3a2657bde1e45c796b5f3c7bf83cb8ec6ca4278f8a89e45108ece92f9b573cafea3b42a06bc09076b40a196c909b6610e
e3e0a2432c Add benchmark to write JSON into a string (Martin Ankerl)
Pull request description:
The benchmark `BlockToJsonVerbose` only tests generating (and destroying)
the JSON data structure, but serializing into a string is also a
performance critical aspect of the RPC calls.
Extracts test setup into a `struct TestBlockAndIndex`, and uses it in
both `BlockToJsonVerbose` and `BlockToJsonVerboseWrite`.
Also, use `ankerl::nanobench::doNotOptimizeAway` to make sure the compiler
can't optimize the result of the calls away.
Here are benchmark results on my Intel i7-8700:
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:----------
| 71,807,017.00 | 13.93 | 0.4% | 555,782,961.00 | 220,788,645.00 | 2.517 | 102,279,341.00 | 0.4% | 0.80 | `BlockToJsonVerbose`
| 27,916,835.00 | 35.82 | 0.1% | 235,084,034.00 | 89,033,525.00 | 2.640 | 42,911,139.00 | 0.3% | 0.32 | `BlockToJsonVerboseWrite`
ACKs for top commit:
laanwj:
Code review ACK e3e0a2432c
Tree-SHA512: bc4d6d1588d47d4bd7af8e7908e44b8561bc391a2d73eccd7c0aa37fc40e8a9ce1fa1f3c29b416eef24a73c6bce3036839c0bbfe1b8dbd6d1bba3718b7ca5383
So that help texts include "i2p" in:
* `./bitcoind -help` (in `-onlynet` description)
* `getpeerinfo` RPC
* `getnetworkinfo` RPC
Co-authored-by: Jon Atack <jon@atack.com>
Introduce two new options to reach the I2P network:
* `-i2psam=<ip:port>` point to the I2P SAM proxy. If this is set then
the I2P network is considered reachable and we can make outgoing
connections to I2P peers via that proxy. We listen for and accept
incoming connections from I2P peers if the below is set in addition to
`-i2psam=<ip:port>`
* `-i2pacceptincoming` if this is set together with `-i2psam=<ip:port>`
then we accept incoming I2P connections via the I2P SAM proxy.
Implement the following commands from the I2P SAM protocol:
* HELLO: needed for all of the remaining ones
* DEST GENERATE: to generate our private key and destination
* NAMING LOOKUP: to convert .i2p addresses to destinations
* SESSION CREATE: needed for STREAM CONNECT and STREAM ACCEPT
* STREAM CONNECT: to make outgoing connections
* STREAM ACCEPT: to accept incoming connections
Introduce two high level, convenience methods in the `Sock` class:
* `SendComplete()`: keep trying to send the specified data until either
successfully sent all of it, timeout or interrupted.
* `RecvUntilTerminator()`: read until a terminator is encountered (never
after it), timeout or interrupted.
These will be convenient in the I2P SAM implementation.
`SendComplete()` can also be used in the SOCKS5 implementation instead
of calling `send()` directly.
Previously `Sock::Wait()` would not have signaled to the caller whether
a timeout or one of the requested events occurred since that was not
needed by any of the callers.
Such functionality will be needed in the I2P implementation, thus extend
the `Sock::Wait()` method.
Recognize also I2P addresses in the form `base32hashofpublickey.b32.i2p`
from `CNetAddr::SetSpecial()`.
This makes `Lookup()` support them, which in turn makes it possible to
manually connect to an I2P node by using
`-proxy=i2p_socks5_proxy:port -addnode=i2p_address.b32.i2p:port`
Co-authored-by: Lucas Ontivero <lucasontivero@gmail.com>
Our local (bind) address is already saved in `CNode::addrBind` and there
is no need to re-retrieve it again with `GetBindAddress()`.
Also, for I2P connections `CNode::addrBind` would contain our I2P
address, but `GetBindAddress()` would return something like
`127.0.0.1:RANDOM_PORT`.
Isolate the second half of `CConnman::AcceptConnection()` into a new
separate method, which could be reused if we accept incoming connections
by other means than `accept()` (first half of
`CConnman::AcceptConnection()`).
Call `GetBindAddress()` earlier in `CConnman::AcceptConnection()`. That
is specific to the TCP protocol and makes the code below it reusable for
other protocols, if the caller provides `addr_bind`, retrieved by other
means.
This check is related to an `accept()` failure. So do the check earlier,
closer to the `accept()` call.
This will allow to isolate the `accept()`-specific code at the beginning
of `CConnman::AcceptConnection()` and reuse the code that follows it.
`fclose()` is flushing any buffered data to disk, so if it fails then
that could mean that the data was not completely written to disk.
Thus, check if `fclose()` succeeds and only then claim success from
`WriteBinaryFile()`.
If an error occurs and `fread()` returns `0` (nothing was read) then the
code before this patch would have returned "success" with a partially
read contents of the file.
Extract `ReadBinaryFile()` and `WriteBinaryFile()` from `torcontrol.cpp`
to its own `readwritefile.{h,cpp}` files, so that it can be reused from
other modules.
faa06ecc9c build: Bump minimum Qt version to 5.9.5 (Hennadii Stepanov)
Pull request description:
Close#20104.
ACKs for top commit:
laanwj:
Code review ACK faa06ecc9c
jarolrod:
ACK faa06ecc9c
fanquake:
ACK faa06ecc9c - this should be ok to do now.
Tree-SHA512: 7295472b5fd37ffb30f044e88c39d375a5a5187d3f2d44d4e73d0eb0c7fd923cf9949c2ddab6cddd8c5da7e375fff38112b6ea9779da4fecce6f024d05ba9c08
Collects all the orphan handling globals into a single member var in
net_processing, and ensures access is encapuslated into the interface
functions. Also adds doxygen comments for methods.
All the interesting functionality of AddOrphanTx is already in other
functions, so call those functions directly in the one place that
AddOrphanTx was used.
Rather than checking net_processing's internal implementation of
AddOrphanTx, test txorphanage's exported AddTx interface. Note that
this means AddToCompactExtraTransactions is no longer tested here.
EraseOrphansFor was called both with and without g_cs_orphans held,
correct that so that it's always called with it already held.
LimitOrphanTxSize was always called with g_cs_orphans held, so
add annotations and don't lock it a second time.
70d3c5d0b9 gui: add "Last Block" (CNodeStats::nLastBlockTime) to peer details (Jon Atack)
a21be7c401 gui: add "Last Tx" (CNodeStats::nLastTXTime) to peer details (Jon Atack)
4dc2fd6c37 qt: add RPCConsole::TimeDurationField helper, call systime only once (Jon Atack)
Pull request description:
- add `RPCConsole::TimeDurationField` helper to replace repeated code and call system time only once in `RPCConsole::updateDetailWidget`
- add "Last Tx" (`CNodeStats::nLastTXTime`) field to peer details
- add "Last Block" (`CNodeStats::nLastBlockTime`) field to peer details
ACKs for top commit:
hebasto:
re-ACK 70d3c5d0b9
Tree-SHA512: 2611b71fd358ba9ffb6a6206275c08ecb5e683b6f87d022faaaba9802a15030430113afdb434814a9ae2681d04429aa733164bc110b64337ceaae12a0420f4f1
fae216a73d scripted-diff: Rename MakeFuzzingContext to MakeNoLogFileContext (MarcoFalke)
fa4fbec03e scripted-diff: Rename PROVIDE_MAIN_FUNCTION -> PROVIDE_FUZZ_MAIN_FUNCTION (MarcoFalke)
Pull request description:
Split out two renames from #21003:
* `PROVIDE_FUZZ_MAIN_FUNCTION`. *Reason*: This in only used by fuzzing, so the name should indicate that.
* `MakeNoLogFileContext`. *Reason*: Better reflects what the helper does. Also, prepares it to be used in non-fuzz tests in the future.
ACKs for top commit:
practicalswift:
cr ACK fae216a73d: scripted-diff looks correct
Tree-SHA512: e5d347746f5da72b0c86fd4f07ac2e4b3016e88e8c97a830c73bd79d0af6d0245fe7712487fc20344d6cc25958941716c1678124a123930407e3a437265b71df
5440c07457 qt: Rename "Edit label" to "Edit address label" (Wladimir J. van der Laan)
22664d6287 Revert "qt: Remove Transactionview Edit Label Action" (Wladimir J. van der Laan)
Pull request description:
This reverts PR #211.
I disagree with this change, I use the functionality a lot, it was the primary way I used to organize and edit transactions labels and am sad to see this go.
> you can edit a sending address in the send tab Address Book
Using the address book should not be encouraged at all! A while ago it was even proposed to remove it. There's rarely need to scroll through all historical addresses used and unused. The transaction list does just fine for this.
> While all other actions apply directly to the selected transaction, the Edit Label action applies to the selected transaction's address.
**In practice** when bitcoin is used in the commonly advised way, generate a new address for each transaction, those are equivalent though.
I doubt I (and **luke-jr**) will be the only users that will stumblle on this. Further discussion here: https://github.com/bitcoin-core/gui/pull/211#issuecomment-784755998
ACKs for top commit:
hebasto:
ACK 5440c07457, verified that 22664d6287 is a clean revert of 8f9644890a.
Tree-SHA512: 3a86a730279bc454d0bd25d874dbfb6b1c0492480e66c3164e7c60d8658d622d4522de11bf8564876dc3ee056b53db71ecbe8a37281bf25d41a27e6e0d72ad8f
bb3da8fe41 qt: Disable requests context menu actions when appropriate (Jarol Rodriguez)
Pull request description:
The recent requests table will allow you to copy data points even if they do not exist. This PR implements checks to disable the `copy label`, `copy message`, and `copy amount` context menu actions if the respective fields are empty. This brings the recent requests table context menu behavior closer to the behavior seen in the transaction view.
On a payment request entry which does not have a value for label, message, or amount:
| Master | PR |
| ----------- | ----------- |
|<img width="169" alt="Screen Shot 2021-02-19 at 1 22 28 AM" src="https://user-images.githubusercontent.com/23396902/108466086-167adc00-7251-11eb-8bd6-13984042bdb3.png">| <img width="169" alt="Screen Shot 2021-02-19 at 1 21 49 AM" src="https://user-images.githubusercontent.com/23396902/108466185-3e6a3f80-7251-11eb-9dd8-492ed07fd638.png">|
`copy URI` never needs to be disabled as an entry in the recent requests table must have a URI even if it doesn't have a label, message, or amount. #213 will add a `copy address` context menu action. This also does not need a check as an entry must be associated with an address.
Below are some more examples of how this PR will behave:
**Has Label, Message, and Amount**
<img width="780" alt="Screen Shot 2021-02-19 at 12 05 38 AM" src="https://user-images.githubusercontent.com/23396902/108466507-c18b9580-7251-11eb-8875-f3aeb9c4c8e9.png">
**Has Label and Amount, but no Message**
<img width="780" alt="Screen Shot 2021-02-19 at 12 05 58 AM" src="https://user-images.githubusercontent.com/23396902/108466421-9b65f580-7251-11eb-97eb-a3bfaa21fa7d.png">
ACKs for top commit:
hebasto:
re-ACK bb3da8fe41
Tree-SHA512: d98f1a6e05ebf6d9d056bc5754aca78ca7ecda93f497dba88187b947ca4a261eb7dc5e8c872956315acaa0008cc39320fb5806e17117e0c873090ad917ca4a3d
ca5bd1c8e5 qt: Prevent the main window popup menu (Hennadii Stepanov)
Pull request description:
https://github.com/bitcoin/bitcoin/issues/11168 is not fixed by https://github.com/bitcoin/bitcoin/pull/11169 completely, as users are allowed to right click on the menu bar:
This PR moves the context menu prohibition from `QToolBar` instance to its parent `BitcoinGUI` instance, which is derived from `QMainWindow`.
ACKs for top commit:
jarolrod:
ACK ca5bd1c8e5, tested on Ubuntu 20.04 Qt 5.12. Confirming that I can replicate the behavior described on `master` and this `PR` fixes it.
leonardojobim:
tACK ca5bd1c8e5
Tree-SHA512: a654ecf7ee35bb271df039be77077c1e1f9514e332587ba8622cea18da6a5b3ae8a7eb421e404ec5993c31a2f4d028e0e456fcc01facdbf61a2bc3b1e8423982
fd6580e405 [refactor] txmempool: split epoch logic into class (Anthony Towns)
Pull request description:
Splits the epoch logic introduced in #17925 into a separate class.
Uses clang's thread safety annotations and encapsulates the data more strongly to reduce chances of bugs from API misuse.
ACKs for top commit:
jonatack:
ACK fd6580e405 using clang thread safety annotations looks like a very good idea, and the encapsulation this change adds should improve robustness (and possible unit test-ability) of the code. Verified that changing some of the locking duly provoked build-time warnings with Clang 9 on Debian and that small changes in the new `Epoch` class were covered by failing functional test assertions in `mempool_updatefromblock.py`, `mempool_resurrect.py`, and `mempool_reorg.py`
hebasto:
re-ACK fd6580e405, since my [previous](https://github.com/bitcoin/bitcoin/pull/18017#pullrequestreview-569619362) review:
Tree-SHA512: 7004623faa02b56639aa05ab7a078320a6d8d54ec62d8022876221e33f350f47df51ddff056c0de5be798f8eb39b5c03c2d3f035698555d70abc218e950f2f8c
9bac71350d build: make HAVE_O_CLOEXEC available outside LevelDB (bugfix) (Sebastian Falbesoner)
584fd91d2d init: only use pipe2 if availabile, check in configure (Sebastian Falbesoner)
Pull request description:
The result of the O_CLOEXEC availability check is currently only set in the Makefile and passed to LevelDB (see `LEVELDB_CPPFLAGS_INT` in `src/Makefile.leveldb.include`), but not defined to be used in our codebase. This means that code within the preprocessor conditional `#if HAVE_O_CLOEXEC` was actually never compiled. On the master branch this is currently used for pipe creation in `src/shutdown.cpp`, PR #21007 moves this part to a new module (I found the issue while testing that PR).
The fix is similar to the one in #19803, which solved the same problem for HAVE_FDATASYNC.
In the course of working on the PR it turned out that pipe2 is not available an all platforms, hence a configure check and a corresponding define HAVE_PIPE2 is introduced and used.
The PR can be tested by anyone with a system that has pipe2 and O_CLOEXEC available by putting gibberish into the HAVE_O_CLOEXEC block: on master, everything should compile fine, on PR, the compiler should abort with an error. At least that's my naive way of testing preprocessor logic, happy to hear more sophisticated ways :-)
ACKs for top commit:
laanwj:
Code review ACK 9bac71350d
Tree-SHA512: aec89faf6ba52b6f014c610ebef7b725d9e967207d58b42a4a71afc9f1268fcb673ecc85b33a2a3debba8105a304dd7edaba4208c5373fcef2ab83e48a170051
ba7e17e073 rpc, test: document {previous,next}blockhash as optional (Sebastian Falbesoner)
Pull request description:
This PR updates the result help of the following RPCs w.r.t. the `previousblockhash` and `nextblockhash` fields:
- getblockheader
- getblock
Also adds trivial tests on genesis block (should not contain "previousblockhash") and best block (should not contain "nextblockhash").
Top commit has no ACKs.
Tree-SHA512: ef42c5c773fc436e1b4a67be14e2532e800e1e30e45e54a57431c6abb714d2c069c70d40ea4012d549293b823a1973b3f569484b3273679683b28ed40abf46bb
f75e0c1edd doc: add external-signer.md (Sjors Provoost)
d4b0107d68 rpc: send: support external signer (Sjors Provoost)
245b4457cf rpc: signerdisplayaddress (Sjors Provoost)
7ebc7c0215 wallet: ExternalSigner: add GetDescriptors method (Sjors Provoost)
fc5da520f5 wallet: add GetExternalSigner() (Sjors Provoost)
259f52cc33 test: external_signer wallet flag is immutable (Sjors Provoost)
2655197e1c rpc: add external_signer option to createwallet (Sjors Provoost)
2700f09c41 rpc: signer: add enumeratesigners to list external signers (Sjors Provoost)
07b7c940a7 rpc: add external signer RPC files (Sjors Provoost)
8ce7767071 wallet: add ExternalSignerScriptPubKeyMan (Sjors Provoost)
157ea7c614 wallet: add external_signer flag (Sjors Provoost)
f3e6ce78fb test: add external signer test (Sjors Provoost)
8cf543f96d wallet: add -signer argument for external signer command (Sjors Provoost)
f7eb7ecc67 test: framework: add skip_if_no_external_signer (Sjors Provoost)
87a97941f6 configure: add --enable-external-signer (Sjors Provoost)
Pull request description:
Big picture overview in [this gist](https://gist.github.com/Sjors/29d06728c685e6182828c1ce9b74483d).
This PR lets `bitcoind` call an arbitrary command `-signer=<cmd>`, e.g. a hardware wallet driver, where it can fetch public keys, ask to display an address, and sign a transaction (using PSBT under the hood).
It's design to work with https://github.com/bitcoin-core/HWI, which supports multiple hardware wallets. Any command with the same arguments and return values will work. It simplifies the manual procedure described [here](https://github.com/bitcoin-core/HWI/blob/master/docs/bitcoin-core-usage.md).
Usage is documented in [doc/external-signer.md](
https://github.com/Sjors/bitcoin/blob/2019/08/hww-box2/doc/external-signer.md), which also describes what protocol a different signer binary should conform to.
Use `--enable-external-signer` to opt in, requires Boost::Process:
```
Options used to compile and link:
with wallet = yes
with gui / qt = no
external signer = yes
```
It adds the following RPC methods:
* `enumeratesigners`: asks <cmd> for a list of signers (e.g. devices) and their master key fingerprint
* `signerdisplayaddress <address>`: asks <cmd> to display an address
It enhances the following RPC methods:
* `createwallet`: takes an additional `external_signer` argument and fetches keys from device
* `send`: automatically sends transaction to device and waits
Usage TL&DR:
* clone HWI repo somewhere and launch `bitcoind -signer=../HWI/hwi.py`
* check if you can see your hardware device: `bitcoin-cli enumeratesigners`
* create wallet and auto import keys `bitcoin-cli createwallet "hww" true true "" true true true`
* display address on device: `bitcoin-cli signerdisplayaddress ...`
* to spend, use `send` RPC and approve transaction on device
Prerequisites:
- [x] #21127 load wallet flags before everything else
- [x] #21182 remove mostly pointless BOOST_PROCESS macro
Potentially useful followups:
- GUI support: bitcoin-core/gui#4
- bumpfee support
- (automatically) verify (a subset of) keys on the device after import, through message signing
ACKs for top commit:
laanwj:
re-ACK f75e0c1edd
Tree-SHA512: 7db8afd54762295c1424c3f01d8c587ec256a72f34bd5256e04b21832dabd5dc212be8ab975ae3b67de75259fd569a561491945750492f417111dc7b6641e77f
The recent requests table will allow you to copy data points even if they do not exist.
This PR implements checks to disable the 'copy label', 'copy message', and 'copy amount' context menu action if the respective fields are empty.
faf48f20f1 log: Clarify log message when file does not exist (MarcoFalke)
Pull request description:
Shorter and broader alternative to #21181
Rendered diff:
```diff
@@ -1,4 +1,4 @@
-Bitcoin Core version v21.99.0-db656db2ed5a (release build)
+Bitcoin Core version v21.99.0-faf48f20f196 (release build)
Qt 5.15.2 (dynamic), plugin=wayland (dynamic)
No static plugins.
Style: adwaita / Adwaita::Style
@@ -24,8 +24,8 @@ scheduler thread start
Using wallet directory /tmp/test_001/regtest/wallets
init message: Verifying wallet(s)...
init message: Loading banlist...
-ERROR: DeserializeFileDB: Failed to open file /tmp/test_001/regtest/banlist.dat
-Invalid or missing banlist.dat; recreating
+Missing or invalid file /tmp/test_001/regtest/banlist.dat
+Recreating banlist.dat
SetNetworkActive: true
Failed to read fee estimates from /tmp/test_001/regtest/fee_estimates.dat. Continue anyway.
Using /16 prefix for IP bucketing
@@ -63,9 +63,9 @@ Bound to [::]:18444
Bound to 0.0.0.0:18444
Bound to 127.0.0.1:18445
init message: Loading P2P addresses...
-ERROR: DeserializeFileDB: Failed to open file /tmp/test_001/regtest/peers.dat
-Invalid or missing peers.dat; recreating
-ERROR: DeserializeFileDB: Failed to open file /tmp/test_001/regtest/anchors.dat
+Missing or invalid file /tmp/test_001/regtest/peers.dat
+Recreating peers.dat
+Missing or invalid file /tmp/test_001/regtest/anchors.dat
0 block-relay-only anchors will be tried for connections.
init message: Starting network threads...
net thread start
ACKs for top commit:
jnewbery:
utACK faf48f20f1
amitiuttarwar:
utACK faf48f20f1, 👍 for consistency. also checked where we create / load other `.dat` files, looks good to me.
practicalswift:
cr ACK faf48f20f1
Tree-SHA512: 697a728ef2b9f203363ac00b03eaf23ddf80bee043ecd3719265a0d884e8cfe88cd39afe946c86ab849edd1c836f05ec51125f052bdc14fe184b84447567756f
5e531e6beb assumptions: check C++17 assumption with MSVC (fanquake)
c7b46489f8 assumptions: assume a C++17 compiler (fanquake)
Pull request description:
This has been the case since #20413.
This should also enable the check for MSVC. From my reading of https://docs.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-160 and https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/ if we set the `/Zc:__cplusplus` switch in additional options, MSVC will report the correct value for `__cplusplus`. However I have not tested this.
ACKs for top commit:
laanwj:
Code review ACK 5e531e6beb
hebasto:
ACK 5e531e6beb, checked the MS docs, and AppVeyor build is green.
practicalswift:
ACK 5e531e6beb
Tree-SHA512: a4fb525cf5c33abc944c614edb0313a39c8a39a1637a03c09342c15ba0925f4eb037062e65e51b42ade667506b7e554c7159acf86e6b8c35d0a87dd79a6f239b
By default, a popup menu contains checkable entries for the toolbars
and dock widgets present in the main window. This allows users to
accidentally hide the toolbar.
5786a818e1 Verify that all validation flags are backward compatible (gzhao408)
b10ce9aa48 [test] check verification flags are minimal/maximal (gzhao408)
a260c22cad [test] Check for invalid flag combinations (gzhao408)
a7098a2a8d [refactor] use CheckTxScripts, TrimFlags, FillFlags (gzhao408)
7a77727b2f Apply minimal validation flags to tx_invalid tests (gzhao408)
9532591bed [test] add BADTX setting for invalid txns that fail CheckTransaction (gzhao408)
4c06ebf128 [test] fix two witness tests in invalid tests with empty vout (gzhao408)
158a0b268c Apply maximal validation flags to tx_valid tests (gzhao408)
0a76a39b63 [test] fix CSV test missing OP_ADD (gzhao408)
19db590d04 [test] remove unnecessary OP_1s from CSV and CLTV tests (gzhao408)
Pull request description:
This uses the first 4 commits of #15045, rebased and added some comments. The diff is quite large already and I want to make it easy to review, so I'm splitting it into 2 PRs (transaction and script). Script one is WIP, I'll link it when I open it.
Interpretation of scripts is dependent on the script verification flags passed in.
In tests, we should always apply **maximal** verification flags when checking that a transaction is **valid**; any additional flags should invalidate the transaction. A transaction should not be valid because we forgot to include a flag, and we should apply all flags by default.
We should apply **minimal** verification flags when asserting that a transaction is **invalid**; if verification flags are applied, removing any one of them should mean the transaction is valid.
New verify flags must be backwards compatible; tests should check backwards compatibility and apply the new flags by default. All `tx_invalid` tests should continue to be invalid with the exact same verify flags. All `tx_valid` tests that don't pass with new flags should _explicitly_ indicate that the flags need to be excluded, and fail otherwise.
1. Flip the meaning of `verifyFlags` in tx_valid.json to mean _excluded_ verification flags instead of included flags. Edit the test data accordingly.
2. Trim unneeded flags from tx_invalid.json.
3. Add check to verify that tx_valid tests have maximal flags and tx_invalid tests have minimal flags.
4. Add checks to verify that flags are soft forks (#10699) i.e. adding any flag should only decrease the number of acceptable scripts. Test by adding/removing random flags.
ACKs for top commit:
achow101:
ACK 5786a818e1
laanwj:
ACK 5786a818e1
Tree-SHA512: 19195d8cf3299e62f47dd3443ae4a95430c5c9d497993a18ab80de9e24b1869787af972774993bf05717784879bc4592fdabaae0fddebd437963d8f3c96d9a73
Currently, the only way to copy the address of a payment request is to double-click on the payment request and then click on the copy address button. This commit adds a convenient context menu action to copy the address of a payment request.
The warning label shown on the overview page does not look clickable.
This PR makes the warning label look clickable by removing the 'flat' property.
Additionally, the Maximum Width is updated to fix the small hit-box issue.
...also update comments to remove mention of ::ChainActive()
From: https://github.com/bitcoin/bitcoin/pull/20750#discussion_r579400663
> Also, what about passing a const reference instead of a pointer? I
> know this is only theoretical, but previously if the tip was nullptr,
> then Height() evaluated to -1, now it evaluates to UB
-BEGIN VERIFY SCRIPT-
# Rename
sed -i -e 's/MakeFuzzingContext/MakeNoLogFileContext/g' $(git grep -l MakeFuzzingContext)
# Bump the copyright of touched files in this scripted diff to avoid touching them again later
./contrib/devtools/copyright_header.py update ./src/test/fuzz/
-END VERIFY SCRIPT-
