💬 mzumsande commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1633934145)
Although I'm not sure if this is an actual problem in practice, because with our DoS protections for low-work blocks and headers we shouldn't really get into this situation with a divergent chain in the first place.
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1633934145)
Although I'm not sure if this is an actual problem in practice, because with our DoS protections for low-work blocks and headers we shouldn't really get into this situation with a divergent chain in the first place.
💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2159465142)
@maflcko Thanks, I can also reproduce that way.
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2159465142)
@maflcko Thanks, I can also reproduce that way.
⚠️ edilmedeiros opened an issue: "Won't compile with miniupnpc 2.2.8"
(https://github.com/bitcoin/bitcoin/issues/30266)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Bitcoin-core will not compile if using miniupnpc 2.2.8.
### Expected behaviour
Compile successfully.
### Steps to reproduce
1. Install miniupnpc 2.2.8 from http://miniupnp.free.fr/.
2. `./configure`
3. `make`
### Relevant log output
<details>
<summary>configure script output</summary>
```bash
❯ ./configure --with-boost=/opt/local/libexec/boost/1.78 --with-gui=no
checking for
...
(https://github.com/bitcoin/bitcoin/issues/30266)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
Bitcoin-core will not compile if using miniupnpc 2.2.8.
### Expected behaviour
Compile successfully.
### Steps to reproduce
1. Install miniupnpc 2.2.8 from http://miniupnp.free.fr/.
2. `./configure`
3. `make`
### Relevant log output
<details>
<summary>configure script output</summary>
```bash
❯ ./configure --with-boost=/opt/local/libexec/boost/1.78 --with-gui=no
checking for
...
💬 edilmedeiros commented on issue "Add bitcoind and bitcoin-cli to macOS release":
(https://github.com/bitcoin/bitcoin/issues/30262#issuecomment-2159613627)
I second this, never understood why not expose all the CLI tools in the binaries distribution.
(https://github.com/bitcoin/bitcoin/issues/30262#issuecomment-2159613627)
I second this, never understood why not expose all the CLI tools in the binaries distribution.
👋 ryanofsky's pull request is ready for review: "multiprocess: Add capnp wrapper for Chain interface"
(https://github.com/bitcoin/bitcoin/pull/29409)
(https://github.com/bitcoin/bitcoin/pull/29409)
💬 edilmedeiros commented on issue "Won't compile with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/issues/30266#issuecomment-2159644988)
From miniupnpc changelog:
```
VERSION 2.2.8 : released 2024/06/09
2024/05/16:
IPv6: try site-local before link-local
2024/05/08:
upnpc.c: Add -f option to upnpc program (delete multiple port redirections)
UPNP_GetValidIGD(): distinguish between not connected and connected to a
"private" network (with a reserved IP address).
Increments API_VERSION to 18
```
Relevant line in the configure script: https://github.com/bitcoin/bitcoin/blob/b1ba1b178f501daa1afdd91f9efec34
...
(https://github.com/bitcoin/bitcoin/issues/30266#issuecomment-2159644988)
From miniupnpc changelog:
```
VERSION 2.2.8 : released 2024/06/09
2024/05/16:
IPv6: try site-local before link-local
2024/05/08:
upnpc.c: Add -f option to upnpc program (delete multiple port redirections)
UPNP_GetValidIGD(): distinguish between not connected and connected to a
"private" network (with a reserved IP address).
Increments API_VERSION to 18
```
Relevant line in the configure script: https://github.com/bitcoin/bitcoin/blob/b1ba1b178f501daa1afdd91f9efec34
...
💬 edilmedeiros commented on issue "Won't compile with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/issues/30266#issuecomment-2159651901)
Well, this can be fixed:
1. by adjusting the above checks (keep dependency of miniupnpc in version range 2.1 to 2.2.7).
Drawback: miss potential security updates on the upstream package.
2. by diving deeper and using the new API (backward incompatible, possibly increasing friction in a lot of systems until the main package managers maintainers decide to update miniupnpc version).
Note that when someone decide to upgrade the version in `depends`, the code base will have to support the new
...
(https://github.com/bitcoin/bitcoin/issues/30266#issuecomment-2159651901)
Well, this can be fixed:
1. by adjusting the above checks (keep dependency of miniupnpc in version range 2.1 to 2.2.7).
Drawback: miss potential security updates on the upstream package.
2. by diving deeper and using the new API (backward incompatible, possibly increasing friction in a lot of systems until the main package managers maintainers decide to update miniupnpc version).
Note that when someone decide to upgrade the version in `depends`, the code base will have to support the new
...
💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1634195691)
Will do if I retouch.
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1634195691)
Will do if I retouch.
💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1634196738)
I think that would be too complicated. Feel free to consider a followup, but I would restrain bloating this PR further.
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1634196738)
I think that would be too complicated. Feel free to consider a followup, but I would restrain bloating this PR further.
🤔 ajtowns reviewed a pull request: "Tr partial descriptors"
(https://github.com/bitcoin/bitcoin/pull/30243#pullrequestreview-2109323689)
Seems okay to me. Presumably should update doc/descriptors.md though, and might also be nice to have some functional tests demonstrating it works end-to-end? (Add some examples to `DESCS` in wallet_miniscript.py maybe?
(https://github.com/bitcoin/bitcoin/pull/30243#pullrequestreview-2109323689)
Seems okay to me. Presumably should update doc/descriptors.md though, and might also be nice to have some functional tests demonstrating it works end-to-end? (Add some examples to `DESCS` in wallet_miniscript.py maybe?
💬 ajtowns commented on pull request "Tr partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#discussion_r1634192469)
Having an `optional` when in one branch you ignore the value and in the other you assume it always has a value doesn't make a lot of sense to me.
Would it make sense to drop `leaf_version` from `TRNodeInfo` and store the leaf version as the first byte of the script? Alternatively, could just drop the optional and default it to 0.
(https://github.com/bitcoin/bitcoin/pull/30243#discussion_r1634192469)
Having an `optional` when in one branch you ignore the value and in the other you assume it always has a value doesn't make a lot of sense to me.
Would it make sense to drop `leaf_version` from `TRNodeInfo` and store the leaf version as the first byte of the script? Alternatively, could just drop the optional and default it to 0.
💬 vasild commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1634204476)
All these
https://github.com/bitcoin/bitcoin/blob/b1ba1b178f501daa1afdd91f9efec34e5ec1e294/test/functional/test_framework/util.py#L393-L423
can be considered as divergence from the default `bitcoind` behavior.
The suggested change adds a new `bitcoind` config option just for the test framework, I do not like that. I guess it may be possible to avoid Tor collisions by spreading the ports without using a new config option, by using `-bind=127.0.0.1:torport=onion`, but then, I do not think
...
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1634204476)
All these
https://github.com/bitcoin/bitcoin/blob/b1ba1b178f501daa1afdd91f9efec34e5ec1e294/test/functional/test_framework/util.py#L393-L423
can be considered as divergence from the default `bitcoind` behavior.
The suggested change adds a new `bitcoind` config option just for the test framework, I do not like that. I guess it may be possible to avoid Tor collisions by spreading the ports without using a new config option, by using `-bind=127.0.0.1:torport=onion`, but then, I do not think
...
💬 emc99 commented on issue "Add bitcoind and bitcoin-cli to macOS release":
(https://github.com/bitcoin/bitcoin/issues/30262#issuecomment-2159910695)
Most macs don't come with the storage that bitcoin core requires.
I was down in the store the other day having a look and the storage hdd capacity on macOS machines was limited to 256GiB.
So I suppose the only solution is to purchase extra storage @Sjors
(https://github.com/bitcoin/bitcoin/issues/30262#issuecomment-2159910695)
Most macs don't come with the storage that bitcoin core requires.
I was down in the store the other day having a look and the storage hdd capacity on macOS machines was limited to 256GiB.
So I suppose the only solution is to purchase extra storage @Sjors
💬 Self-Hosting-Group commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2159928038)
@laanwj Thank you for the explanation.
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2159928038)
@laanwj Thank you for the explanation.
💬 hebasto commented on pull request "Migrate legacy wallets that are not loaded":
(https://github.com/bitcoin-core/gui/pull/824#issuecomment-2159966884)
Concept ACK.
(https://github.com/bitcoin-core/gui/pull/824#issuecomment-2159966884)
Concept ACK.
👍 hebasto approved a pull request: "doc: add release note for 29091 and 29165"
(https://github.com/bitcoin/bitcoin/pull/30261#pullrequestreview-2109554265)
ACK 3d4ca62d883b17d983d6f62cdacbd67a483f7d64.
(https://github.com/bitcoin/bitcoin/pull/30261#pullrequestreview-2109554265)
ACK 3d4ca62d883b17d983d6f62cdacbd67a483f7d64.
💬 hebasto commented on pull request "doc: add release note for 29091 and 29165":
(https://github.com/bitcoin/bitcoin/pull/30261#discussion_r1634329701)
nit: In the phrase "Clang 15+ or later" the plus sign seems redundant.
(https://github.com/bitcoin/bitcoin/pull/30261#discussion_r1634329701)
nit: In the phrase "Clang 15+ or later" the plus sign seems redundant.
💬 Sjors commented on issue "Add bitcoind and bitcoin-cli to macOS release":
(https://github.com/bitcoin/bitcoin/issues/30262#issuecomment-2159998831)
> macOS machines was limited to 256GiB
You're confusing the size of the blockchain with the size of the application download. Once you've installed Bitcoin Core, you can throw away the downloaded zip file. So if you just use Bitcoin-QT nothing changes.
(https://github.com/bitcoin/bitcoin/issues/30262#issuecomment-2159998831)
> macOS machines was limited to 256GiB
You're confusing the size of the blockchain with the size of the application download. Once you've installed Bitcoin Core, you can throw away the downloaded zip file. So if you just use Bitcoin-QT nothing changes.
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160080256)
@ryanofsky do you have any thoughts on this (for this PR or a followup)?
> Additionally the template provider needs to wait, to see if there's a new tip, with g_best_block_cv.wait_until. How would I go about moving that into this interface?
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160080256)
@ryanofsky do you have any thoughts on this (for this PR or a followup)?
> Additionally the template provider needs to wait, to see if there's a new tip, with g_best_block_cv.wait_until. How would I go about moving that into this interface?
📝 fjahr opened a pull request: "assumeutxo: Check snapshot base block is not in invalid chain"
(https://github.com/bitcoin/bitcoin/pull/30267)
This was discovered in a discussion in #29996
If the base block of the snapshot is marked invalid or part of an invalid chain, we currently still load the snapshot and get stuck in a weird state where we have the snapshot chainstate but it will never connect to our valid chain.
While this scenario is highly unlikely to occur on mainnet, it still seems good to prevent this inconsistent state.
(https://github.com/bitcoin/bitcoin/pull/30267)
This was discovered in a discussion in #29996
If the base block of the snapshot is marked invalid or part of an invalid chain, we currently still load the snapshot and get stuck in a weird state where we have the snapshot chainstate but it will never connect to our valid chain.
While this scenario is highly unlikely to occur on mainnet, it still seems good to prevent this inconsistent state.