33df8d46bb ci: Build Arm64 on Travis without functional tests (Fabian Jahr)
Pull request description:
The Travis Arm64 env is a pretty big PITA for quite a while now. It simply seems to time out due to a lack of resources as far as I can tell from my research into the matter.
I have reported the issue in August and received no response: https://travis-ci.community/t/arm64-failing-without-message/9775. Others have complained about similar issues with Arm64 over the past year. The explanation for this is probably that this env is still considered to be in beta: https://docs.travis-ci.com/user/multi-cpu-architectures.
Considering the high frequency of failures (I didn't run any numbers, just based on my personal observations) and the beta status I would suggest that we shorten the runtime by removing part of the test suite until the env is more stable. I would suggest removing functional tests since it should help for sure but maybe there are other ideas.
ACKs for top commit:
MarcoFalke:
ACK 33df8d46bb
Tree-SHA512: 55b0c658b526611d16a26e9508611ddeecbfbd842ec064ada61d188a8be2f473e080c2a35746a9aae8006b11b0c2ab9dd4639b0f3004ecf821506e26c7345972
3491bf358a test: Mention commit id in scripted diff error (Wladimir J. van der Laan)
Pull request description:
Add commit id to make spotting the issue easier.
ACKs for top commit:
robot-dreams:
ACK 3491bf358a
sipa:
utACK 3491bf358a
hebasto:
~ACK~ Concept ACK 3491bf358a, should help in situations like https://travis-ci.org/github/bitcoin/bitcoin/jobs/732481553
Tree-SHA512: 1ae66fa760f9e5d52e029bae71f6b5863f1efd7b95de3723ea09290944c9d7687f5ec6927aa115a3aebd6f2b993baa0c2433975c6ad5cd2858089013362eb599
a91ab86fae lint: Use TRAVIS_BRANCH in lint-git-commit-check.sh (Fabian Jahr)
c11dc995c9 lint: Don't use TRAVIS_COMMIT_RANGE in whitespace linter (Fabian Jahr)
1b41ce8f5f lint: Don't use TRAVIS_COMMIT_RANGE for commit-script-check (Fabian Jahr)
Pull request description:
This is causing problems again, very similar to #19654.
UPDATE: This now removes all remaining usages of TRAVIS_COMMIT_RANGE and instead uses TRAVIS_BRANCH for the range, including `lint-git-commit-check` where TRAVIS_COMMIT_RANGE had already been removed. For builds triggered by a pull request, TRAVIS_BRANCH is the name of the branch targeted by the pull request. In the linters there is still a fallback that assumes master as the target branch.
ACKs for top commit:
sipa:
ACK a91ab86fae. See test I tried in #20075.
Tree-SHA512: 1378bdebd5d8787a83fbda5d9999cc9447209423e7f0218fe5eb240e6a32dc1b51d1cd53b4f8cd1f71574d935ac5e22e203dfe09cce17e9976a48416038e1263
f471a3be00 scripted diff: Improve invalid vout value rpc error message (Nima Yazdanmehr)
Pull request description:
Since the `vout` value can start at `0`, the error message for *negative* values can be improved to something like: `vout cannot be negative`.
ACKs for top commit:
fanquake:
ACK f471a3be00
promag:
Code review ACK f471a3be00.
Tree-SHA512: fbdee3d0ddd5b58eb93934a1217b44e125a9ad39e672b1f35c7609c6c5fcf45ae1b731d3d6135b7225d98792dbfc34a50907b8c41274a5b029d7b5c59f886560
c1585bca8d test: Get rid of default wallet hacks (Russell Yanofsky)
ed3acda33b test, refactor: add default_wallet_name and wallet_data_filename variables (Russell Yanofsky)
Pull request description:
Changes:
- Get rid of setup_nodes (`-wallet`, `-nowallet`, `-disablewallet`) argument rewriting
- Get rid of hardcoded wallet `""` names and `-wallet=""` args
Motivation:
- Simplify test framework behavior so it's easier to write new tests without having arguments mangled by the framework
- Make tests more readable, replacing unexplained `""` string literals with `default_wallet_name` references
- Make it trivial to update default wallet name and wallet data filename for sqlite #19077 testing
- Stop relying on `-wallet` arguments to create wallets, so it is easy to change `-wallet` option in the future to only load existing wallets not create new ones (to avoid accidental wallet creation, and encourage use of wallet encryption and descriptor features)
ACKs for top commit:
MarcoFalke:
ACK c1585bca8d, only effective change is adding documentation 🎵
Tree-SHA512: f62dec7cbdacb5f330aa0e1eec89ab4d065540d91495bbedcb375eda1c080b45ce9edb310ce253c44c4839f1b4cc2c7df9816c58402d5d43f94a437e301ea8bc
d76925478e [doc] Clarify semantic of peer's m_protect w.r.t to outbound eviction logics (Antoine Riard)
ac71fe936d [doc] Clarify scope of eviction protection of outbound block-relay peers (Antoine Riard)
Pull request description:
Block-relay-only peers were introduced by #15759. According to its
author, it was intented to make them only immune to outbound peer
rotation-based eviction and not from all eviction as modified comment
leans to think of.
Clearly indicate that outbound block-relay peers aren't protected
from eviction by the bad/lagging chain logic.
Fix#19863
ACKs for top commit:
naumenkogs:
ACK d76925478e
jonatack:
ACK d76925478e
Tree-SHA512: 597fbd62838a6e39276024165b11514cad20a2e9d33cf9202d261cbadcb62b2df427c858e0cb57e585840d4c1d4600104aa53916bb868541f2580e4eed9b4b52
96571b3d4c doc: Update onion service target port numbers in tor.md (Hennadii Stepanov)
bb145c9050 net: Extend -bind config option with optional network type (Hennadii Stepanov)
92bd3c1da4 net, refactor: Move AddLocal call one level up (Hennadii Stepanov)
57f17e57c8 net: Pass onion service target to Tor controller (Hennadii Stepanov)
e3f07851f0 refactor: Rename TorController::target to m_tor_control_center (Hennadii Stepanov)
fdd3ae4d26 net, refactor: Refactor CBaseChainParams::RPCPort function (Hennadii Stepanov)
a5266d4546 net: Add alternative port for onion service (Hennadii Stepanov)
b3273cf403 net: Use network byte order for in_addr.s_addr (Hennadii Stepanov)
Pull request description:
This PR adds ability to label incoming Tor connections as different from normal localhost connections.
Closes#8973.
Closes#16693.
Default onion service target ports are:
- 8334 on mainnnet
- 18334 on testnet
- 38334 on signet
- 18445 on regtest
To set the onion service target socket manually the extended `-bind` config option could be used:
```
$ src/bitcoind -help | grep -A 6 -e '-bind'
-bind=<addr>[:<port>][=onion]
Bind to given address and always listen on it (default: 0.0.0.0). Use
[host]:port notation for IPv6. Append =onion to tag any incoming
connections to that address and port as incoming Tor connections
(default: 127.0.0.1:8334=onion, testnet: 127.0.0.1:18334=onion,
signet: 127.0.0.1:38334=onion, regtest: 127.0.0.1:18445=onion)
```
Since [pr19991.02 update](https://github.com/bitcoin/bitcoin/pull/19991#issuecomment-698882284) this PR is an alternative to #19043.
ACKs for top commit:
Sjors:
re-utACK 96571b3d4c
vasild:
ACK 96571b3d4
laanwj:
Re-ACK 96571b3d4c
Tree-SHA512: cb0eade80f4b3395f405f775e1b89c086a1f09d5a4464df6cb4faf808d9c2245474e1720b2b538f203f6c1996507f69b09f5a6e35ea42633c10e22bd733d4438
7eab781a14 rpc: Set HTTP Content-Type in bitcoin-cli (Wladimir J. van der Laan)
Pull request description:
We don't set any `Content-Type` in the client. It is more consistent with our other JSON-RPC use to set it to `application/json`.
Note that our server doesn't enforce content types, so it doesn't make a difference in practice. But it is fairly strange HTTP behavior to not set it at all for a POST request.
This came up in #18950.
ACKs for top commit:
promag:
ACK 7eab781a14.
jonatack:
Tested ACK 7eab781a14
practicalswift:
ACK 7eab781a14: patch looks correct
fanquake:
ACK 7eab781a14 - Looks fine to me.
Tree-SHA512: a9fa155324d0f7bff955585a336ead6bb60b721039f424521a435e4bb0fad3f4532e5cc7b7a9acc4e93585e8d3db3082c010138810f22c0e92b8f749b86ef653
7532c9a221 qt: Periodic translations update (Wladimir J. van der Laan)
598e3b1ed3 tx: Update transifex slug for 0.21 (Wladimir J. van der Laan)
Pull request description:
Update transifex slug for 0.21 (as part of #18947), and do a periodic translations update.
ACKs for top commit:
fanquake:
ACK 7532c9a221
Tree-SHA512: 7e396c9a7bcf69088ec89b604332b44cfc2f1949665a7d464ad7cd1c6156cd32ca4a6f7e0b70e4782966f0904c82c4c9286b25ada98052adeaf97e1f408489f6
e66870c5a4 zmq: Append address to notify log output (nthumann)
241803da21 test: Add zmq test to support multiple interfaces (nthumann)
a0b2e5cb6a doc: Add release notes to support multiple interfaces (nthumann)
b1c3f180ec doc: Adjust ZMQ usage to support multiple interfaces (nthumann)
347c94f551 zmq: Add support to listen on multiple interfaces (Nicolas Thumann)
Pull request description:
This PR adds support for ZeroMQ to listen on multiple interfaces, just like the RPC server.
Currently, if you specify more than one e.g. `zmqpubhashblock` paramter, only the first one will be used. Therefore a user may be forced to listen on all interfaces (e.g. `zmqpubhashblock=0.0.0.0:28332`), which can result in an increased attack surface.
With this PR a user can specify multiple interfaces to listen on, e.g.
`-zmqpubhashblock=tcp://127.0.0.1:28332 -zmqpubhashblock=tcp://192.168.1.123:28332`.
ACKs for top commit:
laanwj:
Code review ACK e66870c5a4
instagibbs:
reACK e66870c5a4
Tree-SHA512: f38ab4a6ff00dc821e5f4842508cefadb701e70bb3893992c1b32049be20247c8aa9476a1f886050c5f17fe7f2ce99ee30193ce2c81a7482a5a51f8fc22300c7
We don't set any `Content-Type` in the client. It is more
consistent with our other JSON-RPC use to set it to `application/json`.
Note that our server doesn't enforce content types, so it doesn't make a
difference in practice. But it is fairly strange HTTP behavior to not set it.
This came up in #18950.
6fccad7f71 signet: do not log signet startup messages for other chains (Jon Atack)
Pull request description:
The following signet startup messages are printed to the debug log immediately on node startup for all chains. This behavior occurs on master as a side effect after the merge of #20014. This PR removes the first message and moves the signet derived magic logging to `init.cpp`.
```
$ ./src/bitcoind
2020-09-30T14:25:15Z Using default signet network
2020-09-30T14:25:15Z Signet derived magic (message start): 0a03cf40
2020-09-30T14:25:15Z Bitcoin Core version v0.20.99.0 ...
```
ACKs for top commit:
MarcoFalke:
ACK 6fccad7f71
kallewoof:
utACK 6fccad7f71
hebasto:
ACK 6fccad7f71
Tree-SHA512: 33821dce89b24caf7b7c1ecb41e572ecfb26e6958a1316d359ff240e6ef97c4a1f2cf1b4b974596b252815f9df23960ce385c132ebdbc855bbe6123c3b0b003a
-BEGIN VERIFY SCRIPT-
r() { sed -i 's/vout must be positive/vout cannot be negative/g' $1 }
r $(git grep -l 'vout must be positive')
-END VERIFY SCRIPT-
fa710a6d67 doc: Add 19501 release notes (MarcoFalke)
faf60dee34 doc: Remove double-whitespace from help string, other whitespace fixups (MarcoFalke)
Pull request description:
Adds release notes and fixes up some whitespace nits for the touched RPCs
ACKs for top commit:
fanquake:
ACK fa710a6d67
laanwj:
Code review ACK fa710a6d67
Tree-SHA512: b84a96386a9a8ed69f464c7dffdd600cf9a8b33a06120798b141b300991baed369ab91ae48df6446e89e1d62534ccd8ae721454e7a19b48900b317e9192afc47
af57766182 Fix misleading error message: Clean stack rule (sanket1729)
Pull request description:
Error messages in clean stack is misleading as it lets the user believe that there are extra
elements on the stack which is incorrect if the stack is empty.
Let me know if this requires additional test.
ACKs for top commit:
instagibbs:
re-ACK af57766182
gzhao408:
reACK af57766182
theStack:
re-ACK af57766182
darosior:
re ACK af57766182
Tree-SHA512: 88e77416e220b080246fec368f5552a891d102d072b7bee62ac560d5e31c4a8c2ee9cbe569740b253e9df177d21dc788d10d856b2a542ab47761bb81698e4082
fac966142e signet: Add assumed values for default signet (MarcoFalke)
Pull request description:
Doesn't matter much right now, but when the default signet is bigger, this might come in handy
ACKs for top commit:
jsarenik:
Tested ACK fac966142e
laanwj:
Tested ACK fac966142e (did a new re-sync)
kallewoof:
utACK fac966142e
Tree-SHA512: ed2692f5896350f8dc81f9bc5d79fbf1a4544b8f724c5c667fcadec3a37e26e9833ac189a3067a0731fd7b17a0c94f6b44a641fffe448e42259f7b7b44910db1
69cf5d4eeb [test] Make sure send rpc returns fee reason (Sishir Giri)
d5863c0b3e [send] Make send RPCs return fee reason (Sishir Giri)
Pull request description:
Whenever a wallet funds a transaction, the fee reason is reported to the user only if the verbose is set to true. I added an extra parameter to `CreateTransaction` function in wallet.cpp. Then I implemented the fee reason return logic in `SendMoney` in rpcwallet.cpp, followed by verbose parameter in `sendtoaddress` and `sendmany` functions. I also added a fee reason test case in walletbasic.py.
link to the issue: https://github.com/MarcoFalke/bitcoin-core/issues/22#issue-616251578
ACKs for top commit:
instagibbs:
ACK 69cf5d4eeb
meshcollider:
utACK 69cf5d4eeb
Tree-SHA512: 2e3af32dcfbd5511ba95f8bc8edca7acfe709a8430ff03e43172e5d0af3dfa4b2f57906978e7f272d878043b9ed8c6004674cf47d7496b005d5f612e9a58aa0e
712f95d332 Update msvc build to use new vcpkg manifest (Aaron Clauson)
Pull request description:
The vcpkg tool has introduced a proper way to use [manifests](https://devblogs.microsoft.com/cppblog/vcpkg-accelerate-your-team-development-environment-with-binary-caching-and-manifests/). This PR replaces the custom text file mechanism with the new manifest approach.
It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI.
ACKs for top commit:
fanquake:
ACK 712f95d332 - This is a nice simplification. I tested this in a Windows VM; packages were downloaded and installed automatically as required:
hebasto:
Approach ACK 712f95d332, I've verified that changes comply MS docs:
Tree-SHA512: ff9b3d6ad3cacabcbec6566fd289b179af163dc0c4545f8ba666fc14ba07527557f72bc84ba8abfa3bdffb22e2b8ff0b180f41d909c6de76894ac50ddcf8646b
2ea62cae48 Improve docs about feeler connections (Gleb Naumenko)
Pull request description:
"feeler" and "test-before-evict" are two different strategies suggest in [Eclipse Attacks on Bitcoin’s Peer-to-Peer Network](https://www.usenix.org/system/files/conference/usenixsecurity15/sec15-paper-heilman.pdf). In our codebase, we use `ConnType::FEELER` to implement both.
It is confusing, up to the point that our documentation was just incorrect.
This PR:
- ~clarifies this aspect by renaming "ConnType::FEELER" to "ConnType::PROBE", meaning that this connections only probes that the node is operational, and then disconnects.~
- fixes the documentation
ACKs for top commit:
amitiuttarwar:
ACK 2ea62cae48. thank you!
practicalswift:
ACK 2ea62cae48
Tree-SHA512: c9c03c09eefeacec28ea199cc3f697b0a98723f2f849f7a8115edc43791f8165e296e0e25a82f0b5a4a781a7de38c8954b48bf74c714eba02cdc21f7460673e5
8a4dcda414 doc: Added default signet config for linearize script (gr0kchain)
Pull request description:
Updated the example-linearize.cfg file to include support for the signet chain network config which is used by the ./linearize-data.py
Problem:
Without the signet magic, genesis hash and path config, the linearize-data.py script cannot generate a bootstrap.dat file.
Example:
```
./linearize-data.py ./linearize.cfg
Read 4776 hashes
Genesis block not found in hashlist
```
Solution:
Added netmagic, genesis and input example parameters to file. Netmagic in terms of signet is derived from the signet-challenge and not static as with other networks. The provided signet magic is based on the default public signet.
Resolution
```
./linearize-hashes.py ./linearize.cfg > ./hashlist.txt
./linearize-data.py ./linearize.cfg
$ ./linearize-data.py ./linearize.cfg
Read 4776 hashes
Input file /Users/gr0kchain/.bitcoin/signet/blocks/blk00000.dat
Output file /Users/gr0kchain/Downloads/bootstrap.dat
1000 blocks scanned, 1000 blocks written (of 4776, 20.9% complete)
2000 blocks scanned, 2000 blocks written (of 4776, 41.9% complete)
3011 blocks scanned, 3000 blocks written (of 4776, 62.8% complete)
4010 blocks scanned, 4000 blocks written (of 4776, 83.8% complete)
Done (4776 blocks written)
```
ACKs for top commit:
laanwj:
Tested ACK 8a4dcda414
Tree-SHA512: ad4d330358cf67e7424fb1d97ca828c28ca2758102e45747f4059c11a8acce801162da024a20cfb892f997fd4c3f4d5af988d1ca67c74efa78bf7d4ed27dd421
f22d6a1142 log: Remove static log message "Initializing chainstate Chainstate [ibd] @ height -1 (null)" (practicalswift)
Pull request description:
Remove static log message `Initializing chainstate Chainstate [ibd] @ height -1 (null)`.
AFAICT `chainstate->ToString()` will always equal `"Chainstate [ibd] @ height -1 (null)"` here which makes the log message neither relevant nor interesting :)
ACKs for top commit:
laanwj:
ACK f22d6a1142
promag:
ACK f22d6a1142, just get rid of it.
hebasto:
ACK f22d6a1142, I agree that the removed log message in its current state is cryptic and useless.
Tree-SHA512: 1a65c0d14c9a433afcdaadef9bfcdd5d63276d5d2caee1bf3c48ac477e54fa28138f64020e6e26ca5e67872954a1e7d93fa24a12accc7c7211bc6e7a6039051d
9b4fa0af40 net: Print error message if -proxy is specified without arguments (instead of continuing without proxy server) (practicalswift)
Pull request description:
Exit with error message if `-proxy` is specified without arguments (instead of continuing without proxy server).
Continuing without a proxy server when the end-user has specified `-proxy` may result in accidental loss of privacy. (The end-user might think he/she is using a proxy when he/she is not.)
Before this patch:
```
$ src/bitcoind -proxy
…
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -listen=0
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -upnp=0
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -proxy set -> setting -discover=0
2020-09-23T00:24:33Z InitParameterInteraction: parameter interaction: -listen=0 -> setting -listenonion=0
…
2020-09-23T00:24:33Z init message: Starting network threads...
```
`bitcoind` is now running *without* a proxy server (`GetProxy(…, …) == false`, `HaveNameProxy() == false`, etc.).
Note that the "-proxy set" log messages above which the end-user might interpret as "good, my traffic is now routed via the proxy".
After this patch:
```
$ src/bitcoind -proxy
Error: No proxy server specified. Use -proxy=<ip> or -proxy=<ip:port>.
$ echo $?
1
```
ACKs for top commit:
laanwj:
re-ACK 9b4fa0af40
kristapsk:
ACK 9b4fa0af40, I have tested the code.
hebasto:
re-ACK 9b4fa0af40
Tree-SHA512: 4ba7a011991699a54b5bb87ec68367c681231bf5dcd36f8c89ff9ddc2e8d29df453817b7e362597e652ad6b341a22b7274be0fd78d435e5f0fd8058e5221c4ce
The vcpkg tool has introduced a proper way to use manifests, https://devblogs.microsoft.com/cppblog/vcpkg-accelerate-your-team-development-environment-with-binary-caching-and-manifests/. This PR replaces the custom text file mechanism with the new manifest approach.
It is planned that vckpg manifests will include the ability to version dependencies in the future. Dependency versions would solve a number of issues that currently require workarounds with the appveyor CI.
Set vcpkg manifest version to 1 to avoid any perception it's related to any release or other version numbering.
