Bitcoin Core Github
42 subscribers
126K links
Download Telegram
πŸ’¬ mzumsande commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2134916035)
This doesn't return the timestamp of the best header, but the time of the tip (which is still at genesis, after all headers up to minchainwork have been downloaded but no blocks connected). As far as I know (didn't check) we don't expose `m_chainman.m_best_header->Time()` via RPC, so it may be hard to test the dynamic part in a functional test (which the test doesn't do anyway because no headers are exchanged, but it currently pretends that it could).
πŸ€” mzumsande reviewed a pull request: "test: headers sync timeout"
(https://github.com/bitcoin/bitcoin/pull/32677#pullrequestreview-2908751605)
Concept ACK.
I wonder if it might be better to add the additional tests to `p2p_initial_headers_sync.py` - after all, this is not a particularly long test.
πŸ’¬ mzumsande commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#discussion_r2134912428)
this seems unnecessary, `add_outbound_p2p_connection` already has a built-in wait for the `verack`.
πŸ’¬ yuvicc commented on issue "avoid using invalidateblock to directly test reorg behavior":
(https://github.com/bitcoin/bitcoin/issues/32531#issuecomment-2954644351)
I've opened a PR [#32587](https://github.com/bitcoin/bitcoin/pull/32587) to update the reorg pattern to the real one mentioned above β€” it's ready for review! If anyone’s interested in taking a look, your feedback would be most welcome and appreciated!

@instagibbs @mzumsande
πŸ’¬ hebasto commented on pull request "test: Turn util/test_runner into functional test":
(https://github.com/bitcoin/bitcoin/pull/32697#issuecomment-2955009919)
Concept ACK.
πŸ’¬ yuvicc commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2955049712)
Concept ACK
πŸ€” shahsb reviewed a pull request: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685#pullrequestreview-2909362789)
Could we also guard higher-level wallet operations (e.g., in LoadWallet, TopUpKeyPool, UpgradeWallet, etc.) against unintended writes in read-only mode?.

This helps future-proof against accidental changes, even if developers mistakenly invoke write operations upstream.
πŸ€” shahsb reviewed a pull request: "wallet: Allow read-only database access for info and dump commands"
(https://github.com/bitcoin/bitcoin/pull/32685#pullrequestreview-2909366676)
Concept ACK
πŸ€” mabu44 reviewed a pull request: "docs: adds correct updated documentation links"
(https://github.com/bitcoin/bitcoin/pull/32699#pullrequestreview-2909378292)
This makes it necessary to manually update the links with every release, or at least with every new version that contains changes to those documentation files.
πŸ’¬ yuvicc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#issuecomment-2955103592)
Concept ACK

Will benchmark this soon!
⚠️ fanquake opened an issue: "depends: Windows libevent build broken against mingw-w64 13.x"
(https://github.com/bitcoin/bitcoin/issues/32707)
#### make -C depends HOST=x86_64-w64-mingw32
```bash
/opt/homebrew/Cellar/mingw-w64/13.0.0/toolchain-x86_64/x86_64-w64-mingw32/include/iphlpapi.h:51:44: error: unknown type name 'PMIB_TCP6ROW'; did you mean 'PMIB_TCPROW'?
51 | ULONG WINAPI SetPerTcp6ConnectionEStats (PMIB_TCP6ROW Row, TCP_ESTATS_TYPE EstatsType, PUCHAR Rw, ULONG RwVersion, ULONG RwSize, ULONG Offset);
| ^~~~~~~~~~~~
| PMIB_TCPR
...
πŸ“ rkrux opened a pull request: "rpc, doc: update `listdescriptors` RCP help"
(https://github.com/bitcoin/bitcoin/pull/32708)
This RPC lists all the descriptors present in the wallet, not only the ones that were imported, but also the ones generated when a new wallet is created.

It can be verified by creating a new wallet and calling the `listdescriptors` RPC, which will contain 8 ranged descriptors that are created for every new wallet.

Also, update the description to get rid of "descriptor-enabled" because this is the only wallet type available now after removal of legacy wallets.

<!--
*** Please remove the
...
πŸ’¬ Sjors commented on issue "RFC: Multiprocess binaries and packaging options":
(https://github.com/bitcoin/bitcoin/issues/30983#issuecomment-2955317432)
The wrapper binary has been merged with #31375. Actually shipping the binaries would happen if #31802 lands.
πŸ’¬ PeterWrighten commented on pull request "wallet: Allow read-only database access for info and dump commands":
(https://github.com/bitcoin/bitcoin/pull/32685#issuecomment-2955355673)
> Could we also guard higher-level wallet operations (e.g., in LoadWallet, TopUpKeyPool, UpgradeWallet, etc.) against unintended writes in read-only mode?.
>
> This helps future-proof against accidental changes, even if developers mistakenly invoke write operations upstream.

Good idea. I could check it and work on it.
πŸ’¬ hebasto commented on pull request "depends: Bump boost to 1.88.0 and use new CMake buildsystem":
(https://github.com/bitcoin/bitcoin/pull/32665#issuecomment-2955357844)
cc @theuni
πŸ’¬ saikiran57 commented on pull request "Added rescan option for import descriptors":
(https://github.com/bitcoin/bitcoin/pull/31668#discussion_r2135490336)
Hi @achow101 could you please share your thoughts with the latest changes.
πŸ’¬ stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#discussion_r2135515163)
> but I'm actually not seeing where this was happening

There's a `InvalidChainFound(to_mark_failed)` call near the end of `InvalidateBlock()`, which in turn calls `RecalculateBestHeader()`.
πŸ’¬ stickies-v commented on pull request "validation: stricter internal handling of invalid blocks":
(https://github.com/bitcoin/bitcoin/pull/31405#issuecomment-2955469784)
re-ACK f6b782f3aad4a6bcf823a9a0fabb4418bca1eea1

No changes except minor ones addressing review feedback wrt documentation updates and inlining.
πŸ’¬ stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2135540635)
Oh no I agree, debug logs should be exempt from ratelimiting. My point is just that I think it's bad to have different logging behaviour depending on whether `LogInfo()` or `LogPrintLevel(..., BCLog::Info)` is used, so I think something like the below would work well?

<details>
<summary>git diff on 911ee520c8</summary>

```diff
diff --git a/src/logging.h b/src/logging.h
index d588ef86dc..5f298405e7 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -349,17 +349,13 @@ inline void LogPri
...
πŸ“ hebasto opened a pull request: "cmake: Fix `FindQt` module"
(https://github.com/bitcoin/bitcoin/pull/32709)
This PR improves the `FindQt` module in two ways:

1. Ensures that the `find_package(Qt .. MODULE REQUIRED COMPONENTS ...)` call treats any missing component as a fatal error. In the master branch, only a warning is currently issued.

2. Cleans up the user-facing part of the CMake cache following the migration to Qt 6. An exception (`Qt6_DIR`) is documented.