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.
faa06ecc9c357428f3fa7a8f86b41b1220809951 build: Bump minimum Qt version to 5.9.5 (Hennadii Stepanov)
Pull request description:
Close#20104.
ACKs for top commit:
laanwj:
Code review ACK faa06ecc9c357428f3fa7a8f86b41b1220809951
jarolrod:
ACK faa06ecc9c357428f3fa7a8f86b41b1220809951
fanquake:
ACK faa06ecc9c357428f3fa7a8f86b41b1220809951 - this should be ok to do now.
Tree-SHA512: 7295472b5fd37ffb30f044e88c39d375a5a5187d3f2d44d4e73d0eb0c7fd923cf9949c2ddab6cddd8c5da7e375fff38112b6ea9779da4fecce6f024d05ba9c08
88c4b9b761964d0b3011126796f272cfca2a9fa2 test: remove unneeded node from feature_blockfilterindex_prune.py (Jon Atack)
ace3f4cbdf476ea8b954d98dfaa931076f87a080 test: improve assertions in feature_blockfilterindex_prune.py (Jon Atack)
Pull request description:
- improves the assertions
- removes an unneeded node, reducing from two to one, and some unneeded `extra_arg` code
ACKs for top commit:
MarcoFalke:
ACK 88c4b9b761964d0b3011126796f272cfca2a9fa2
brunoerg:
Tested ACK 88c4b9b761964d0b3011126796f272cfca2a9fa2
Tree-SHA512: 295700da3a5f583ee02ae2d184db93cce0e13aba69115d5db07f83e96a66b4b850adaff2c3725b6585799565b9ee654b1fee8a6245eaba8c21e1cb5ce524eb2b
13a9fd11a507fd3398bc2c0a0575bdc81579243f guix: Passthrough SDK_PATH into container (Carl Dong)
Pull request description:
This is a usability improvement for Guix builders so that they don't have to extract the Xcode tarball into `depends/SDKs` every time.
Inspiration: https://github.com/bitcoin/bitcoin/pull/21089#issuecomment-778639698
ACKs for top commit:
laanwj:
Tested ACK 13a9fd11a507fd3398bc2c0a0575bdc81579243f
Tree-SHA512: 63392d537e48a0da9f0ee04a929613b139bef1ac5643187871c9ea5376afd2a3d95df0f5e0950ae0eccd2813b166667be98401e5a248ae9c187fe4e84e54d427
70d3c5d0b987411873d557440607aa4f3f65b1ec gui: add "Last Block" (CNodeStats::nLastBlockTime) to peer details (Jon Atack)
a21be7c401e1b885bb338645248f40023af7a88f gui: add "Last Tx" (CNodeStats::nLastTXTime) to peer details (Jon Atack)
4dc2fd6c37f8d85c2e9f19d5f03c72c5a1707455 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 70d3c5d0b987411873d557440607aa4f3f65b1ec
Tree-SHA512: 2611b71fd358ba9ffb6a6206275c08ecb5e683b6f87d022faaaba9802a15030430113afdb434814a9ae2681d04429aa733164bc110b64337ceaae12a0420f4f1
fa730e915766b012ff145e3c2af782668defb21f test: Avoid connecting to real network when running tests (MarcoFalke)
fa1b713941f71b077da07e9dedb743c7e99c3ca8 test: Assume node is running in subtests (MarcoFalke)
Pull request description:
Introduced in #19884
ACKs for top commit:
Sjors:
ACK fa730e915766b012ff145e3c2af782668defb21f
Tree-SHA512: fe132a9ffe2fae1ab16857a3dec9839526fdf74d27a1ae794fbffca8356f639c4b916dc888b260281e9cc793916706c18d1687ebb5a076d4e1c481d218d308d3
fae216a73d71441432cf07212313d18771ce7deb scripted-diff: Rename MakeFuzzingContext to MakeNoLogFileContext (MarcoFalke)
fa4fbec03e4395a6ab75ea98a696bee6d768e97e 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 fae216a73d71441432cf07212313d18771ce7deb: scripted-diff looks correct
Tree-SHA512: e5d347746f5da72b0c86fd4f07ac2e4b3016e88e8c97a830c73bd79d0af6d0245fe7712487fc20344d6cc25958941716c1678124a123930407e3a437265b71df
fa560cc6c493764efc2f2f1702d45286060fe15f test: Intermittent issue in feature_blockfilterindex_prune (MarcoFalke)
Pull request description:
https://cirrus-ci.com/task/4962244553342976?command=ci#L5131
The index is built in a background thread, so we have to wait for it.
ACKs for top commit:
jonatack:
ACK fa560cc6c493764efc2f2f1702d45286060fe15f
Tree-SHA512: e7a246fe43a28511581fe34b1f5a85303b1874b2535330afc0405269cce7306984ecc6af389791321e3aa4b224819e89d9b89dd5bc080d60baa20bd007412787
faa137eb9eac5554504b062a6dc865ca87fd572b test: Speed up rpc_blockchain.py by removing miniwallet.generate() (MarcoFalke)
fa1fe80c757df0adcbfaf41b5c5c8a468bc07b6f test: Change address type from P2PKH to P2WSH in rpc_blockchain (MarcoFalke)
fa4d8f3169e38cbdbae20258efebe7070c49f522 test: Cache 25 mature coins for ADDRESS_BCRT1_P2WSH_OP_TRUE (MarcoFalke)
fad25153f5c8e88f72cf666b16b0b0dbdc45d3b1 test: Remove unused bug workaround (MarcoFalke)
faabce7d07c5776e4116b1a7ad1f6c408a4a4e46 test: Start only the number of nodes that are needed (MarcoFalke)
Pull request description:
Speed up various tests:
* Remove unused nodes, which only consume time on start/stop
* Remove unused "bug workarounds"
* Remove the need for `miniwallet.generate()` by adding `miniwallet.scan_blocks()`. (On my system, with valgrind, generating 105 blocks takes 3.31 seconds. Rescanning 5 blocks takes 0.11 seconds.)
ACKs for top commit:
laanwj:
Code review ACK faa137eb9eac5554504b062a6dc865ca87fd572b
Tree-SHA512: ead1988d5aaa748ef9f8520af1e0bf812cf1d72e281ad22fbd172b7306d850053040526f8adbcec0b9a971c697a0ee7ee8962684644d65b791663eedd505a025
5440c074579f5588cdcff3d33bd1350cee0b7bff qt: Rename "Edit label" to "Edit address label" (Wladimir J. van der Laan)
22664d6287dda243ebf06df4409b50ab509603e1 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 5440c074579f5588cdcff3d33bd1350cee0b7bff, verified that 22664d6287dda243ebf06df4409b50ab509603e1 is a clean revert of 8f9644890a167a093d95ecef1f12a20dce1bc581.
Tree-SHA512: 3a86a730279bc454d0bd25d874dbfb6b1c0492480e66c3164e7c60d8658d622d4522de11bf8564876dc3ee056b53db71ecbe8a37281bf25d41a27e6e0d72ad8f
bb3da8fe410285887f22679c6f08a1f40bcfac8f 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 bb3da8fe410285887f22679c6f08a1f40bcfac8f
Tree-SHA512: d98f1a6e05ebf6d9d056bc5754aca78ca7ecda93f497dba88187b947ca4a261eb7dc5e8c872956315acaa0008cc39320fb5806e17117e0c873090ad917ca4a3d
ca5bd1c8e5fee29a3c2f0a3fbc879ee74f892266 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 ca5bd1c8e5fee29a3c2f0a3fbc879ee74f892266, 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
d98f4593cf00ab2973f8113e30506861b24383bc guix: Explicitly set umask in build container (Carl Dong)
Pull request description:
Opened as a separate PR to fix non-reproducibility found through testing here: https://github.com/bitcoin/bitcoin/pull/21089#issuecomment-783549633
Many thanks to everyone who helped find this!
ACKs for top commit:
laanwj:
ACK d98f4593cf00ab2973f8113e30506861b24383bc
fanquake:
ACK d98f4593cf00ab2973f8113e30506861b24383bc - I'm seeing matching hashes.
Tree-SHA512: ea339c3902f2f4dea32e8ef5cc675a1df0679530881260ae999aaaf7339d5b12c46e01e58677cbb079f33e573ad105e2b443a835f3e944ef8e943a25f83027f1
8b08d0f2a5161296d809a154ccde403a33a0f3ea build, doc: Fix configure script output indentation and typos (Hennadii Stepanov)
Pull request description:
This PR is follow up of #16546, that breaks the `configure` script output indentation for gui/qt/qr lines:
```
Options used to compile and link:
external signer = no
multiprocess = no
with libs = yes
with wallet = yes
with sqlite = yes
with bdb = yes
with gui / qt = yes
with qr = yes
with zmq = yes
with test = yes
...
```
With this PR:
```
Options used to compile and link:
external signer = no
multiprocess = no
with libs = yes
with wallet = yes
with sqlite = yes
with bdb = yes
with gui / qt = yes
with qr = yes
with zmq = yes
with test = yes
...
```
Also typos are fixed.
ACKs for top commit:
Sjors:
utACK 8b08d0f2a5161296d809a154ccde403a33a0f3ea
vasild:
ACK 8b08d0f2a5161296d809a154ccde403a33a0f3ea
Tree-SHA512: 46dfcfb754192dbcc080348781327d1714e2f9a696f3ed9252746b426e3afc628d84adb197ba3b8080eacaa6053ccac07e670998930ae92cef8ed713dd457c10
df8f2a11dcadd08840c5311631132a522650a83e test: Replace accidentally placed bit-OR with logical-OR (Hennadii Stepanov)
Pull request description:
This PR is a follow up of #19698.
ACKs for top commit:
glozow:
utACK df8f2a11dc
Tree-SHA512: 36aba3e952850deafe78dd39775a568e89e867c8a352f511f152bce7062f614f5bb4f23266dbb33da5292c9ee6da5ccefce117e3168621c71d2140c8e7f58460
fd6580e405699ccb051fd2a34525e48d3253673d [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 fd6580e405699ccb051fd2a34525e48d3253673d 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 fd6580e405699ccb051fd2a34525e48d3253673d, since my [previous](https://github.com/bitcoin/bitcoin/pull/18017#pullrequestreview-569619362) review:
Tree-SHA512: 7004623faa02b56639aa05ab7a078320a6d8d54ec62d8022876221e33f350f47df51ddff056c0de5be798f8eb39b5c03c2d3f035698555d70abc218e950f2f8c
9bac71350d98580cc7441957fc7c3fa2f4158553 build: make HAVE_O_CLOEXEC available outside LevelDB (bugfix) (Sebastian Falbesoner)
584fd91d2d294883e6896dbd64a2176528e94581 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 9bac71350d98580cc7441957fc7c3fa2f4158553
Tree-SHA512: aec89faf6ba52b6f014c610ebef7b725d9e967207d58b42a4a71afc9f1268fcb673ecc85b33a2a3debba8105a304dd7edaba4208c5373fcef2ab83e48a170051
