✅ achow101 closed an issue: "Gracefully handle dropped UPnP support "
(https://github.com/bitcoin-core/gui/issues/843)
(https://github.com/bitcoin-core/gui/issues/843)
🚀 achow101 merged a pull request: "init: Handle dropped UPnP support more gracefully"
(https://github.com/bitcoin/bitcoin/pull/31916)
(https://github.com/bitcoin/bitcoin/pull/31916)
🤔 stickies-v reviewed a pull request: "validation: stricter internal handling of invalid blocks"
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2655396246)
Concept ACK for making `m_best_header` and `nStatus` more reliably correct, and the trade-offs seem reasonable. Even though the diff is small, I find reasoning about these changes fairly complex and require a lot of context, so I'll need to think through the implications more.
(https://github.com/bitcoin/bitcoin/pull/31405#pullrequestreview-2655396246)
Concept ACK for making `m_best_header` and `nStatus` more reliably correct, and the trade-offs seem reasonable. Even though the diff is small, I find reasoning about these changes fairly complex and require a lot of context, so I'll need to think through the implications more.
💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978292997)
It's not clear to me why we would only want to mark blocks with large-enough PoW as invalid descendants. If this is not a bug, I think behaviour is inconsistent with the commit message and should probably be document in the code too?
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978292997)
It's not clear to me why we would only want to mark blocks with large-enough PoW as invalid descendants. If this is not a bug, I think behaviour is inconsistent with the commit message and should probably be document in the code too?
💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978269926)
Is there a reason why we forcefully unset `BLOCK_FAILED_VALID`? Since both flags store orthogonal information in separate bits, intuitively I'd expect we could have both co-exist? If this breaks other assumptions, perhaps good to add to the documentation here?
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978269926)
Is there a reason why we forcefully unset `BLOCK_FAILED_VALID`? Since both flags store orthogonal information in separate bits, intuitively I'd expect we could have both co-exist? If this breaks other assumptions, perhaps good to add to the documentation here?
💬 stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978437444)
Since this map is a superset of `candidate_blocks_by_work `, I think we could just have one, and instead check for `IsValid(BLOCK_VALID_TRANSACTIONS) && HaveNumChainTxs()` further down when we update `setBlockIndexCandidates`? I suspect the memory impllications should be negligible either way, but I think it would clean up the code a bit?
E.g. something like:
<details>
<summary>git diff on 4ba2e480ff</summary>
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index f9c900e
...
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r1978437444)
Since this map is a superset of `candidate_blocks_by_work `, I think we could just have one, and instead check for `IsValid(BLOCK_VALID_TRANSACTIONS) && HaveNumChainTxs()` further down when we update `setBlockIndexCandidates`? I suspect the memory impllications should be negligible either way, but I think it would clean up the code a bit?
E.g. something like:
<details>
<summary>git diff on 4ba2e480ff</summary>
```diff
diff --git a/src/validation.cpp b/src/validation.cpp
index f9c900e
...
💬 asklokesh commented on issue "Bug: Non-Ranged Descriptors with Range [0,0] Trigger Unexpected Wallet Errors in `AddWalletDescriptor`":
(https://github.com/bitcoin/bitcoin/issues/31728#issuecomment-2696014125)
I've investigated this issue and have a fix for the uninitialized `subtype` variable in `LoadWalletFlags()`.
The bug occurs because `subtype` is being used for version compatibility checks but may remain uninitialized if the data stream doesn't contain subtype information.
**Fix approach:**
1. Initialize `subtype` with a safe default value (`DBWrapperSubType::UNKNOWN`) at declaration
2. Ensure all code paths properly set `subtype` with explicit handling for cases where:
- The stream contain
...
(https://github.com/bitcoin/bitcoin/issues/31728#issuecomment-2696014125)
I've investigated this issue and have a fix for the uninitialized `subtype` variable in `LoadWalletFlags()`.
The bug occurs because `subtype` is being used for version compatibility checks but may remain uninitialized if the data stream doesn't contain subtype information.
**Fix approach:**
1. Initialize `subtype` with a safe default value (`DBWrapperSubType::UNKNOWN`) at declaration
2. Ensure all code paths properly set `subtype` with explicit handling for cases where:
- The stream contain
...
💬 achow101 commented on pull request "Add mainnet assumeutxo param at height 880,000":
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2696072544)
ACK 14f16748557faf57cf4b0f4c91c162592557434c
> if you agree we should host it, can you add it to bitcoincore.org and give me the link? I'll add it as a binary seed.
That will require more discussion with the group.
(https://github.com/bitcoin/bitcoin/pull/31969#issuecomment-2696072544)
ACK 14f16748557faf57cf4b0f4c91c162592557434c
> if you agree we should host it, can you add it to bitcoincore.org and give me the link? I'll add it as a binary seed.
That will require more discussion with the group.
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2696652781)
Rebased to refresh the CI as all required actions are now enabled in this repo.
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2696652781)
Rebased to refresh the CI as all required actions are now enabled in this repo.
📝 eval-exec opened a pull request: "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds"
(https://github.com/bitcoin/bitcoin/pull/31979)
This PR introduces a maximum reconnect timeout of 600 seconds (10 minutes) to prevent excessive delays in reconnection attempts. It also updates the log message to display the retry delay in whole seconds for better readability.
(https://github.com/bitcoin/bitcoin/pull/31979)
This PR introduces a maximum reconnect timeout of 600 seconds (10 minutes) to prevent excessive delays in reconnection attempts. It also updates the log message to display the retry delay in whole seconds for better readability.
💬 eval-exec commented on pull request "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds":
(https://github.com/bitcoin/bitcoin/pull/31979#issuecomment-2696734781)
I'm working on fixing the CI and would like to kindly invite @laanwj to review this PR. Thank you!
(https://github.com/bitcoin/bitcoin/pull/31979#issuecomment-2696734781)
I'm working on fixing the CI and would like to kindly invite @laanwj to review this PR. Thank you!
💬 Sjors commented on pull request "Drop testnet3":
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2696820588)
Will rebase after #31649 lands.
(https://github.com/bitcoin/bitcoin/pull/31974#issuecomment-2696820588)
Will rebase after #31649 lands.
💬 hodlinator commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1979023367)
> Can we explicitly list out which things to take from the output?
While testing #31969 and using `dumptxoutset` I didn't feel the need for documentation on individual fields as long as there are prior `m_assumeutxo_data`-entries. If we keep `gettxoutsetinfo` though, the output seems to map less directly, so in that case I agree listing them might be useful.
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1979023367)
> Can we explicitly list out which things to take from the output?
While testing #31969 and using `dumptxoutset` I didn't feel the need for documentation on individual fields as long as there are prior `m_assumeutxo_data`-entries. If we keep `gettxoutsetinfo` though, the output seems to map less directly, so in that case I agree listing them might be useful.
💬 Sjors commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1979028647)
`dumptxoutset` was designed for this task (originally a contrib bash script), so it seems better to use it.
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1979028647)
`dumptxoutset` was designed for this task (originally a contrib bash script), so it seems better to use it.
💬 willcl-ark commented on pull request "Add assumeutxo chainparams to release-process.md":
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1979042477)
Updated to use `dumptxoutset` in 02fae3363511e96a76ff64a4513c7a7e8d8d4403
(https://github.com/bitcoin/bitcoin/pull/31940#discussion_r1979042477)
Updated to use `dumptxoutset` in 02fae3363511e96a76ff64a4513c7a7e8d8d4403
💬 Sjors commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979074753)
On my existing node `du -h` returns 671G for the blocks and 12G for the chainstate.
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979074753)
On my existing node `du -h` returns 671G for the blocks and 12G for the chainstate.
💬 fanquake commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979092800)
`no_warn_duplicate_libraries` isn't an lld option, so this can be dropped.
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979092800)
`no_warn_duplicate_libraries` isn't an lld option, so this can be dropped.
💬 fanquake commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979096453)
Rather than adding text that repeats the code below, we could just say something like: "Using lld is required to due to issues with Apples `ld` and LLVM".
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979096453)
Rather than adding text that repeats the code below, we could just say something like: "Using lld is required to due to issues with Apples `ld` and LLVM".
💬 Sjors commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#issuecomment-2696958623)
I did a mainnet sync using 4475d0babc070a19f3a0ac472304a8c9b87b87d7 and `-assumevalid=0` which worked. I verified the chain work for the new assume valid block, as well as the `getchaintxstats` result and `headerssync-params.py`.
(https://github.com/bitcoin/bitcoin/pull/31978#issuecomment-2696958623)
I did a mainnet sync using 4475d0babc070a19f3a0ac472304a8c9b87b87d7 and `-assumevalid=0` which worked. I verified the chain work for the new assume valid block, as well as the `getchaintxstats` result and `headerssync-params.py`.
✅ fanquake closed an issue: "Bitcoin Core MacOS - Possible Miner Infection - Bitcoin Core MacOS - Possível Infecção por Minerador"
(https://github.com/bitcoin/bitcoin/issues/31970)
(https://github.com/bitcoin/bitcoin/issues/31970)
💬 fanquake commented on issue "Bitcoin Core MacOS - Possible Miner Infection - Bitcoin Core MacOS - Possível Infecção por Minerador":
(https://github.com/bitcoin/bitcoin/issues/31970#issuecomment-2697012162)
Assuming you have downloaded our binaries from bitcoincore.org, this is a false-positive with your antivirus software.
(https://github.com/bitcoin/bitcoin/issues/31970#issuecomment-2697012162)
Assuming you have downloaded our binaries from bitcoincore.org, this is a false-positive with your antivirus software.