🚀 MarcoFalke merged a pull request: "cli: include local ("unroutable") peers in -netinfo table"
(https://github.com/bitcoin/bitcoin/pull/26584)
(https://github.com/bitcoin/bitcoin/pull/26584)
💬 MarcoFalke commented on pull request "doc: Remove copyright years (headers only)":
(https://github.com/bitcoin/bitcoin/pull/26817#issuecomment-1430952811)
See also https://github.com/bitcoin/bitcoin/pull/27100
(https://github.com/bitcoin/bitcoin/pull/26817#issuecomment-1430952811)
See also https://github.com/bitcoin/bitcoin/pull/27100
💬 murrayn commented on pull request "doc: FreeBSD build doc should suggest db4 for legacy wallet support":
(https://github.com/bitcoin/bitcoin/pull/26773#issuecomment-1430998296)
I've updated the document to reflect the removal of `install_db4.sh` #26834
(https://github.com/bitcoin/bitcoin/pull/26773#issuecomment-1430998296)
I've updated the document to reflect the removal of `install_db4.sh` #26834
📝 ebarakos opened a pull request: "Fix minor typo"
(https://github.com/bitcoin/bitcoin/pull/27102)
<!--
Just a small typo fix
-->
(https://github.com/bitcoin/bitcoin/pull/27102)
<!--
Just a small typo fix
-->
💬 Sjors commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1431108853)
@darosior do I understand correctly that you need #26567 in order for [PSBT stuff](https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1183382594) to work? Can I just combine both PR's to continue the test I was doing, or are there more changes required?
(https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1431108853)
@darosior do I understand correctly that you need #26567 in order for [PSBT stuff](https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1183382594) to work? Can I just combine both PR's to continue the test I was doing, or are there more changes required?
📝 fanquake converted_to_draft a pull request: "test: autogenerate bash completion"
(https://github.com/bitcoin/bitcoin/pull/25243)
Added a functional test which parses all the RPC help commands then automatically generates the bitcoin-cli bash-completion file and makes sure that the original file matches the newly generated one.
In order to get the RPC help commands in the correct format needed for the test, I added the "format" RPC command.
Test using `python3 test/functional/tool_cli_completion.py --overwrite`
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a ration
...
(https://github.com/bitcoin/bitcoin/pull/25243)
Added a functional test which parses all the RPC help commands then automatically generates the bitcoin-cli bash-completion file and makes sure that the original file matches the newly generated one.
In order to get the RPC help commands in the correct format needed for the test, I added the "format" RPC command.
Test using `python3 test/functional/tool_cli_completion.py --overwrite`
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a ration
...
💬 Sjors commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1431116683)
Concept ACK, I second @prusnak.
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1431116683)
Concept ACK, I second @prusnak.
💬 darosior commented on pull request "Signing support for Miniscript Descriptors":
(https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1431118379)
@Sjors see https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1103625462. A minimal PSBT integration is part of this PR now. We treat Miniscript-related data when we are given a PSBT. However to fill Miniscript-related data to a PSBT from our wallet will need more work than just #26567, and also some design decisions to be taken.
(https://github.com/bitcoin/bitcoin/pull/24149#issuecomment-1431118379)
@Sjors see https://github.com/bitcoin/bitcoin/pull/24149#discussion_r1103625462. A minimal PSBT integration is part of this PR now. We treat Miniscript-related data when we are given a PSBT. However to fill Miniscript-related data to a PSBT from our wallet will need more work than just #26567, and also some design decisions to be taken.
💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1431124511)
Thanks @ryanofsky that was unintentional and is now fixed. The only behaviour change is now in `bitcoin-cli`.
This does still leave us with the (undesirable) behaviour described in https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1429434184. It seems likely a likely flow that a user might download bitcoin core, create `bitcoin.conf` and then run the application, which results in *not* creating a `wallets/` subdir on mainnet (and technically other networks, but it's unlikely a new us
...
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1431124511)
Thanks @ryanofsky that was unintentional and is now fixed. The only behaviour change is now in `bitcoin-cli`.
This does still leave us with the (undesirable) behaviour described in https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1429434184. It seems likely a likely flow that a user might download bitcoin core, create `bitcoin.conf` and then run the application, which results in *not* creating a `wallets/` subdir on mainnet (and technically other networks, but it's unlikely a new us
...
📝 TheCharlatan opened a pull request: "Blockstorage: Dont access gArgs to get blocks_dir"
(https://github.com/bitcoin/bitcoin/pull/27103)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/27103)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ TheCharlatan closed a pull request: "Blockstorage: Dont access gArgs to get blocks_dir"
(https://github.com/bitcoin/bitcoin/pull/27103)
(https://github.com/bitcoin/bitcoin/pull/27103)
💬 fanquake commented on pull request "wallet: Guard against undefined behaviour":
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1431174607)
@achow101 would be good to get your conceptual thoughts here.
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1431174607)
@achow101 would be good to get your conceptual thoughts here.
💬 MarcoFalke commented on pull request "wallet: Guard against undefined behaviour":
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1431207007)
In any case: `test_bitcoin: wallet/coinselection.cpp:71: std::optional<wallet::SelectionResult> wallet::SelectCoinsBnB(std::__debug::vector<wallet::OutputGroup>&, const CAmount&, const CAmount&): Assertion `cost_of_change > 0' failed.`
(https://github.com/bitcoin/bitcoin/pull/25982#issuecomment-1431207007)
In any case: `test_bitcoin: wallet/coinselection.cpp:71: std::optional<wallet::SelectionResult> wallet::SelectCoinsBnB(std::__debug::vector<wallet::OutputGroup>&, const CAmount&, const CAmount&): Assertion `cost_of_change > 0' failed.`
💬 hebasto commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1107016808)
Why `GetConfigFile()` call dropped?
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1107016808)
Why `GetConfigFile()` call dropped?
💬 hebasto commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1107037794)
The behavior has been slightly changed:
- on master:
```
$ ./src/qt/bitcoin-qt -regtest -conf=nothing
Error: Error reading configuration file: specified config file "nothing" could not be opened.
```
- this PR (6207b73e508d6cfc7583483d00261c66b0c348c3):
```
$ ./src/qt/bitcoin-qt -regtest -conf=nothing
Error: Error reading configuration file: specified config file "/home/hebasto/.bitcoin/nothing" could not be opened.
```
If it is intended, maybe document it?
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1107037794)
The behavior has been slightly changed:
- on master:
```
$ ./src/qt/bitcoin-qt -regtest -conf=nothing
Error: Error reading configuration file: specified config file "nothing" could not be opened.
```
- this PR (6207b73e508d6cfc7583483d00261c66b0c348c3):
```
$ ./src/qt/bitcoin-qt -regtest -conf=nothing
Error: Error reading configuration file: specified config file "/home/hebasto/.bitcoin/nothing" could not be opened.
```
If it is intended, maybe document it?
🚀 hebasto merged a pull request: "Add settings.json prune-prev, proxy-prev, onion-prev settings "
(https://github.com/bitcoin-core/gui/pull/603)
(https://github.com/bitcoin-core/gui/pull/603)
✳️ hebasto pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/68e484afbbc2...e43ff4eab2fe)
Merge bitcoin-core/gui#603: Add settings.json prune-prev, proxy-prev, onion-prev settings
9d3127b11e34131409dab7c08bde5b444d90b2cb Add settings.json prune-prev, proxy-prev, onion-prev settings (Ryan Ofsky)
Pull request description:
With #602, if proxy and pruning settings are disabled in the GUI and the GUI is restarted, proxy and prune values are not stored anywhere. So if these settings are enabled in the future, default values will be shown, not previous values.
This PR stores previous values so they will preserved across restarts. I'm not sure I like this behavior because showing default values seems simpler and safer to me. Previous values may just have been set temporarily and may have never actually worked, and it adds some code complexity to store them.
This PR is one way of resolving #596. Other solutions are possible and could be implemented as alternatives.
ACKs for top commit:
hebasto:
ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb, tested on Ubuntu 22.04.
vasild:
ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb
jarolrod:
tACK 9d3127b11e34131409dab7c08bde5b444d90b2cb
Tree-SHA512: 1778d1819443490c880cfd5c1711d9c5ac75ea3ee8440e2f0ced81d293247163a78ae8aba6027215110aec6533bd7dc6472aeead6796bfbd51bf2354e28f24a9
(https://github.com/bitcoin/bitcoin/compare/68e484afbbc2...e43ff4eab2fe)
Merge bitcoin-core/gui#603: Add settings.json prune-prev, proxy-prev, onion-prev settings
9d3127b11e34131409dab7c08bde5b444d90b2cb Add settings.json prune-prev, proxy-prev, onion-prev settings (Ryan Ofsky)
Pull request description:
With #602, if proxy and pruning settings are disabled in the GUI and the GUI is restarted, proxy and prune values are not stored anywhere. So if these settings are enabled in the future, default values will be shown, not previous values.
This PR stores previous values so they will preserved across restarts. I'm not sure I like this behavior because showing default values seems simpler and safer to me. Previous values may just have been set temporarily and may have never actually worked, and it adds some code complexity to store them.
This PR is one way of resolving #596. Other solutions are possible and could be implemented as alternatives.
ACKs for top commit:
hebasto:
ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb, tested on Ubuntu 22.04.
vasild:
ACK 9d3127b11e34131409dab7c08bde5b444d90b2cb
jarolrod:
tACK 9d3127b11e34131409dab7c08bde5b444d90b2cb
Tree-SHA512: 1778d1819443490c880cfd5c1711d9c5ac75ea3ee8440e2f0ced81d293247163a78ae8aba6027215110aec6533bd7dc6472aeead6796bfbd51bf2354e28f24a9
✳️ fanquake pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/e43ff4eab2fe...2b0cd7679f82)
Merge bitcoin/bitcoin#27076: verify-commits: Bump trusted git root to after most recent laanwj merge
6ada37d44cce7fa3a729de72cede4c1f3bc675ce verify-commits: Bump trusted git root to after most recent laanwj merge (Andrew Chow)
Pull request description:
To prepare for the removal of laanwj's key from trusted key (#27054), the trusted git root needs to be newer than the most recent merge commit signed by his key.
This can be tested by removing the laanwj's key from trusted keys (e.g. by merging with #27054) and running `verify-commits.py` with `--clean-merge 0`: `./contrib/verify-commits/verify-commits.py --clean-merge 0 HEAD~`. (`--clean-merge 0` disables the clean merge check which will checkout some commits, which results in the `trusted-keys` used in checking of subsequent commits to be different than expected).
ACKs for top commit:
fanquake:
ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce
hebasto:
ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce, I've verified the history of laanwj's merge commits.
Tree-SHA512: 55cafeddd54aa2b62d7b7cd41c542f4fd974b322a0405de546600d88658575714ebc893b087eb31f28c205559a7b213f88d9038de431271fca00be866610df74
(https://github.com/bitcoin/bitcoin/compare/e43ff4eab2fe...2b0cd7679f82)
Merge bitcoin/bitcoin#27076: verify-commits: Bump trusted git root to after most recent laanwj merge
6ada37d44cce7fa3a729de72cede4c1f3bc675ce verify-commits: Bump trusted git root to after most recent laanwj merge (Andrew Chow)
Pull request description:
To prepare for the removal of laanwj's key from trusted key (#27054), the trusted git root needs to be newer than the most recent merge commit signed by his key.
This can be tested by removing the laanwj's key from trusted keys (e.g. by merging with #27054) and running `verify-commits.py` with `--clean-merge 0`: `./contrib/verify-commits/verify-commits.py --clean-merge 0 HEAD~`. (`--clean-merge 0` disables the clean merge check which will checkout some commits, which results in the `trusted-keys` used in checking of subsequent commits to be different than expected).
ACKs for top commit:
fanquake:
ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce
hebasto:
ACK 6ada37d44cce7fa3a729de72cede4c1f3bc675ce, I've verified the history of laanwj's merge commits.
Tree-SHA512: 55cafeddd54aa2b62d7b7cd41c542f4fd974b322a0405de546600d88658575714ebc893b087eb31f28c205559a7b213f88d9038de431271fca00be866610df74
🚀 fanquake merged a pull request: "verify-commits: Bump trusted git root to after most recent laanwj merge"
(https://github.com/bitcoin/bitcoin/pull/27076)
(https://github.com/bitcoin/bitcoin/pull/27076)
👍 MarcoFalke approved a pull request: "Remove laanwj from trusted-keys"
(https://github.com/bitcoin/bitcoin/pull/27054)
(https://github.com/bitcoin/bitcoin/pull/27054)