Bitcoin Core Github
42 subscribers
126K links
Download Telegram
🤔 polespinasa reviewed a pull request: "test: Use rpc_deprecated only for testing deprecation"
(https://github.com/bitcoin/bitcoin/pull/31977#pullrequestreview-2656938692)
Concept ACK.

No need to double test a functionality.
🤔 pablomartin4btc reviewed a pull request: "wallet: migration, don't create spendable wallet from a watch-only legacy wallet"
(https://github.com/bitcoin/bitcoin/pull/31423#pullrequestreview-2656958431)
Concept ACK

nit (top PR's description & last commit body - perhaps it's obvious but still):
- If the legacy wallet contains only watch-only scripts, the migration
process should only generate a watch-only **_descriptor_** wallet instead.

(I'll test and review soon)
📝 Sjors opened a pull request: "Add checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31981)
This PR adds the IPC equivalent of the `getblocktemplate` in `proposal` mode.

In order to do so it moves `TestBlockValidity` to `ChainstateManager` and has is return error reasons as a string instead of `BlockValidationState`. This avoids complexity in IPC code for handling the latter struct.

The new Mining interface method is used in `miner_tests` and the `getblocktemplate` and `generateblock` RPC calls, so it has test coverage.

The `inconclusive-not-best-prevblk` check is moved from R
...
Sjors closed a pull request: "Add checkblock RPC and checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31564)
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2697113675)
I opened a fresh PR #31981 that takes only the first three commits from this one, dropping the new `checkblock` RPC as well as the reduced target verification.

Assuming most IPC / RPC consumers will use some sort of Bitcoin library to process the contents of blocks and templates, it should be easy enough for them to check the PoW.

And if I drop the `target` argument from `checkblock` it's no different from the `getblocktemplate` RPC in `proposal` mode. Which itself is easy enough to use,
...
💬 hebasto commented on pull request "ci: Test cross-built Windows executables on Windows natively":
(https://github.com/bitcoin/bitcoin/pull/31176#issuecomment-2697188523)
@davidgumberg @hodlinator @maflcko

As the CI is now green, mind taking another look?
📝 fanquake opened a pull request: "scripted-diff: rename libmultiprocess repository"
(https://github.com/bitcoin/bitcoin/pull/31982)
For when we shift `libmultiprocess` into the `bitcoin-core` organisation.
💬 willcl-ark commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979246362)
> On my existing node du -h returns 681G for the blocks and 12G for the chainstate.

I also have numbers matching these on my nodes too.

<details>
<summary>Details</summary>

```
bitcoin in ~/.bitcoin ➜ du -h | rg chainstate
77M ./signet/chainstate
12G ./chainstate
bitcoin in ~/.bitcoin ➜ cd /mnt/data/bitcoin/.bitcoin/
bitcoin in data/bitcoin/.bitcoin ➜ du -h | rg blocks
469M ./signet/blocks
681G ./blocks
```

</details>
💬 Sjors commented on pull request "Add checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31981#issuecomment-2697242642)
I also salvaged the functional tests from 15d841e41bb61f435dd3e60e99fa4db47d34a3a6.
📝 fanquake opened a pull request: "build: don't show ccache summary with MSVC"
(https://github.com/bitcoin/bitcoin/pull/31983)
Given we skip MSVC when performing any ccache setup, it doesn't really make sense to show this output when using MSVC. Even less so if it's going to (possibly incorrectly) show it as Enabled.

Fixes #31771.
💬 fanquake commented on issue "cmake: incorrectly reporting MSVC as using ccache":
(https://github.com/bitcoin/bitcoin/issues/31771#issuecomment-2697266556)
I've opened #31983 to deal with this for now.
💬 dergoegge commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2697276918)
The docs still say that the default is 4: https://github.com/bitcoin/bitcoin/blob/15717f0ef3960969ee550a4a41741987b86684dc/doc/reduce-memory.md#thread-configuration
💬 hebasto commented on pull request "build: don't show ccache summary with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31983#issuecomment-2697283225)
Ccache could be supported for MSVC in the future, which might lead to these changes being reverted. Why not use https://github.com/bitcoin/bitcoin/pull/30861/commits/c19a187c42fe867d61ca5dbd48ae18f15201839f instead?
💬 fjahr commented on pull request "wallet, rpc: deprecate settxfee and paytxfee":
(https://github.com/bitcoin/bitcoin/pull/31278#issuecomment-2697295701)
utACK f4344220d7195324f921dcf001c1a117008477fd
💬 fanquake commented on pull request "build: don't show ccache summary with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31983#issuecomment-2697305165)
> Why not use https://github.com/bitcoin/bitcoin/commit/c19a187c42fe867d61ca5dbd48ae18f15201839f instead?

From my read of #30861 nobody seems to agree that we should make this change? https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1937227612
💬 Sjors commented on pull request "scripted-diff: rename libmultiprocess repository":
(https://github.com/bitcoin/bitcoin/pull/31982#issuecomment-2697315226)
Concept ACK
💬 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-2697317938)
CI has passed, so I think this PR is ready for review.
💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2697341062)
Tested ACK e181bda061ca63021511be6e286fdf6a5818df49

I only tested on macOS (Intel and Apple Silicon), and only lightly reviewed the code.
💬 hebasto commented on pull request "build: don't show ccache summary with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31983#discussion_r1979315337)
The [`MSVC`](https://cmake.org/cmake/help/latest/variable/MSVC.html) variable is defined based on the compiler. However, the current ccache settings rely on the [`<LANG>_COMPILER_LAUNCHER`](https://cmake.org/cmake/help/latest/prop_tgt/LANG_COMPILER_LAUNCHER.html) properties, which are considered depending on the generator rather than the compiler.
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979319485)
Changed to this
💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979319968)
Thanks, dropped.