Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 davidgumberg commented on pull request "[29.x] bump to v29.0rc1":
(https://github.com/bitcoin/bitcoin/pull/32046#issuecomment-2719475327)
ACK https://github.com/bitcoin/bitcoin/commit/47e2fa86dc5433852fd9e5050a23de2accfdca8d

I'm ~0 on whether or not the release candidate should be held up by the example `bitcoin.conf`, but everything else looks good to me.
💬 Prabhat1308 commented on pull request "doc: Update documentation to include Clang/llvm based coverage report generation":
(https://github.com/bitcoin/bitcoin/pull/31933#issuecomment-2719581088)
Addressing the review from [comment](https://github.com/bitcoin/bitcoin/pull/31933#pullrequestreview-2656773286)
- The coverage now includes functional tests
- Removed coverage from external deps (crc32,leveldb,minisketch,secp256k1) and `src/test`.
- Now uses `-DCMAKE_BUILD_TYPE=Debug`
- Notes are now in Blockquotes
- Incuding the functional tests has increased the mismatch from 2 to 61 .
📝 jonatack opened a pull request: "p2p: protect addnode peers during IBD"
(https://github.com/bitcoin/bitcoin/pull/32051)
When an addnode peer is disconnected by our IBD headers/blocks download timeout or stalling logic, ThreadOpenAddedConnections attempts to immediately reconnect it.

During IBD in a low-bandwidth environment, I see these disconnections/reconnections frequently with high-quality addnode peers.

This pull attempts to avoid unnecessary disconnections where it may make sense to do so. For now, it is an initial quick draft.
💬 fanquake commented on pull request "p2p: protect addnode peers during IBD":
(https://github.com/bitcoin/bitcoin/pull/32051#issuecomment-2719746790)
```bash
[23:32:25.656] /ci_container_base/src/rpc/net.cpp:310:125: error: invalid operands to binary expression ('const char[198]' and 'const char[196]')
[23:32:25.656] "Nodes added using addnode (or -connect) are protected from disconnection due to DoS or IBD header/block\n" +
[23:32:25.656] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^
[23:32:25.656] 1 error generated.
```
💬 fanquake commented on pull request "[29.x] bump to v29.0rc1":
(https://github.com/bitcoin/bitcoin/pull/32046#issuecomment-2719771413)
There's likely going to be more rcs than usual, so just getting this out for testing with the incomplete example .conf is fine.
🚀 fanquake merged a pull request: "[29.x] bump to v29.0rc1"
(https://github.com/bitcoin/bitcoin/pull/32046)
💬 Prabhat1308 commented on pull request "qa: Fix TxIndex race conditions":
(https://github.com/bitcoin/bitcoin/pull/32010#issuecomment-2719819873)
Are the files mentioned in `files_to_delete` and `files_to_perturb` just for informational purposes that they don't trigger an error or you want to indicate that these files should trigger an error but currently they don't.
🤔 glozow reviewed a pull request: "contrib: Fix `gen-bitcoin-conf.sh`"
(https://github.com/bitcoin/bitcoin/pull/32049#pullrequestreview-2680493978)
ACK a24419f8bed5e1145ce171dbbdad957750585471
🚀 fanquake merged a pull request: "contrib: Add deterministic-unittest-coverage"
(https://github.com/bitcoin/bitcoin/pull/31901)
🤔 furszy reviewed a pull request: "removed duplicate call to GetDescriptorScriptPubKeyMan"
(https://github.com/bitcoin/bitcoin/pull/32023#pullrequestreview-2680558849)
Please try to avoid introducing and reverting changes within the same PR. The first two commits should be squashed. Aside from that, I left you a comment suggesting a possible alternative implementation.
💬 furszy commented on pull request "removed duplicate call to GetDescriptorScriptPubKeyMan":
(https://github.com/bitcoin/bitcoin/pull/32023#discussion_r1992789597)
What about returning `util::Result<void>` instead and return the error message here? (see how we do it in other places).

Also, if you're going down this route, you should check any other usages of this function, as it would no longer be throwing an exception.

And you would also have to return an `util::Result<ScriptPubKeyMan*>` in `AddWalletDescriptor` too. Which I think that is cleaner than catching the specific exception.
💬 davidrobinsonau commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1992796261)
Is there a use case for someone to pass a timestamp prior to January 1, 1970? Which would be a negative number, causing bitcoind not to rescan when it was expected to.
Alternatively, the code could be updated to only look for "-1", meaning anything below that could still be a full rescan.
⚠️ fanquake opened an issue: "v29.0 Testing"
(https://github.com/bitcoin/bitcoin/issues/32052)
Umbrella issue for 29.0 testing. Please help testing on a wide variety of supported platforms, as well as interaction with different software.

Let us know which version you tested on which operating system.

If you find an issue, please search Github for known issues first and then open a new Github issue.

This meta issue should not be used to report bugs, as a single thread makes it impossible to track more than one topic.

See [29.0 Release Notes Draft](https://github.com/bitcoin-core/bitcoi
...
⚠️ fanquake pinned an issue: "v29.0 Testing"
(https://github.com/bitcoin/bitcoin/issues/32052)
Umbrella issue for 29.0 testing. Please help testing on a wide variety of supported platforms, as well as interaction with different software.

Let us know which version you tested on which operating system.

If you find an issue, please search Github for known issues first and then open a new Github issue.

This meta issue should not be used to report bugs, as a single thread makes it impossible to track more than one topic.

See [29.0 Release Notes Draft](https://github.com/bitcoin-core/bitcoi
...
💬 ajtowns commented on pull request "[IBD] Tracking PR for speeding up Initial Block Download":
(https://github.com/bitcoin/bitcoin/pull/32043#issuecomment-2720097064)
> Plotting the performance of the blocks from the produced `debug.log` files shows (from the last run, can differ slightly from the normalized average shown below) the effect of each commit:

Wouldn't these plots be easier to read with block height on the x-axis and time on the y-axis, giving a consistent domain (each case goes from height 0 to 880k or so) with a simple "lower is better" comparison? (rather than "further left is better")

Plotting the difference between the various proposed
...
💬 maflcko commented on pull request "contrib: Add deterministic-unittest-coverage":
(https://github.com/bitcoin/bitcoin/pull/31901#issuecomment-2720322464)
@brunoerg @janb84 @Prabhat1308 I expect all of you were running into the issue on macOS? I probably can't fix it myself, but I'd investigate whether the last commit is working:

* Is your result different with or without the last commit?
* Does `ResetCoverageCounters` work?
* What is the coverage result for `src/test/util/setup_common.cpp`, especially `g_rng_temp_path_init`? (You can find it in `fuzz_det_cov.show.{run_id}.txt`)
⚠️ Sjors opened an issue: "Add test coverage for getblocktemplate fees"
(https://github.com/bitcoin/bitcoin/issues/32053)
Suggested in https://github.com/bitcoin/bitcoin/pull/31897#discussion_r1992245751

There's probably other aspects of `getblocktemplate` and the underlying template construction methods that coverage.

Related:
- #31981 adds coverage for the `proposal` mode
💬 Sjors commented on pull request "mining: drop unused -nFees and sigops from CBlockTemplate":
(https://github.com/bitcoin/bitcoin/pull/31897#discussion_r1992988592)
Tracking in #32053.
💬 maflcko commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r1992994451)
It has been permitted previously and this commit is silently breaking it. If you want to use -1, it seems better to properly sanitize the user input first, or use a different approach.