Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 hebasto commented on issue "qt: translation related warnings":
(https://github.com/bitcoin/bitcoin/issues/32710#issuecomment-3058055918)
> [@hebasto](https://github.com/hebasto) If this is an actual issue can it be fixed up?

Fixed in https://github.com/bitcoin/bitcoin/pull/32940.
💬 Crypt-iQ commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3058062207)
Still looking into the above source of non-determinism, it points to `RecalculateBestHeader`.

A separate source of non-determinism is because `m_cached_finished_ibd` is not reset (if set) at the end of the run.
🤔 BrandonOdiwuor reviewed a pull request: "cmake: Move internal binaries from bin/ to libexec/"
(https://github.com/bitcoin/bitcoin/pull/31679#pullrequestreview-3006391049)
Tested ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f

Tested on macOS 15.5 that `bitcoin-node` and `test_bitcoin` have been correctly moved from `bin/` to `libexec/`

<img width="635" height="311" alt="Screenshot 2025-07-10 at 18 56 31" src="https://github.com/user-attachments/assets/7f0e6ae5-f06f-488f-b913-1494bd20c05a" />
📝 glozow opened a pull request: "TxOrphanage revamp cleanups"
(https://github.com/bitcoin/bitcoin/pull/32941)
Followup to #31829:
- Release notes
- Have the orphanage auto-trim itself whenever necessary (and test changes) https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2169508690
- Reduce duplicate reconsiderations by keeping track of which txns are already reconsiderable so we only mark it for reconsideration for 1 peer at a time
- Rename `OrphanTxBase` to `OrphanInfo`
- Get rid of `Size()` method by replacing all calls with `CountUniqueOrphans`
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198155263)
I added this to a followup PR: #32941

I think the tests added here are easier to reason about when `LimitOrphans` is an explicit call, so it feels better to do it in a separate step.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#issuecomment-3058077194)
I've created a followup PR with some of the cleanups and optimizations: #32941
💬 sipa commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198159044)
This `std::floor` has no effect. `m_tx->vin.size() / 10` is an integer division, not floating point, and it already rounds towards 0.
💬 maflcko commented on pull request "fuzz: Add fuzz target for block index tree and related validation events":
(https://github.com/bitcoin/bitcoin/pull/31533#issuecomment-3058096115)
> A separate source of non-determinism is because `m_cached_finished_ibd` is not reset (if set) at the end of the run.

This could be fixed by calling `ResetIbd()`, if there is need to.
🤔 maflcko reviewed a pull request: "qa: Fix `wallet_multiwallet.py`"
(https://github.com/bitcoin/bitcoin/pull/31410#pullrequestreview-3006435253)
the rebase looks correct. (ci obviously still fails, as expected)

re-ACK 605593547aa196204b1e5b8f7857abacd36906b2 🏣

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZ
...
💬 maflcko commented on pull request "qa: Fix `wallet_multiwallet.py`":
(https://github.com/bitcoin/bitcoin/pull/31410#discussion_r2198179128)
nit in 8f22dd5e4c4b116ad873e464ea7f75caaef3476d: Would be nice to use the same logic flow like above in this test:

```py
self.log.info("Verify warning is emitted when failing to scan the wallets directory")
if platform.system() == 'Windows':
self.log.warning('Skipping test involving chmod as Windows does not support it.')
elif os.geteuid() == 0:
self.log.warning('Skipping test involving chmod as it requires a non-root user.')
else:
...
💬 fanquake commented on issue "cmake: searching across directories for config files":
(https://github.com/bitcoin/bitcoin/issues/32938#issuecomment-3058124124)
That seems to fix it. I haven't tested extensively though.
💬 glozow commented on pull request "p2p: improve TxOrphanage denial of service bounds":
(https://github.com/bitcoin/bitcoin/pull/31829#discussion_r2198190875)
Removed
💬 1BitcoinBoWP1FZ4xwTNkq6XksKidmgYYw commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3058141540)
@hebasto The logo sizes remain unchanged; only the margins have been removed. When compiling Bitcoin Core, everything appears exactly as it did before.

The shadows were removed to match the original Bitcoin logo from bitboy, which also did not include shadows. Since users may download the Bitcoin logo (example the SVG version) directly from Bitcoin Core's source code, this ensures they receive properly optimized logo files.
💬 hebasto commented on pull request "New SVG, Icons, PNGs and X PixMaps":
(https://github.com/bitcoin-core/gui/pull/879#issuecomment-3058183303)
GitHub renders chnages as follows:

<img width="541" height="330" alt="image" src="https://github.com/user-attachments/assets/9100a63c-334d-47bd-8b65-73433092dc2d" />


> ... only the margins have been removed.

Why?

> The shadows were removed to match the original Bitcoin logo from bitboy, which also did not include shadows.

Bitcoin Core has its own logo for years. I don't see your reference as a justification for this change.

> Since users may download the Bitcoin logo (example
...
💬 tnndbtc commented on issue "intermittent timeout in wallet_signer.py : 'createwallet' RPC took longer than 1200.000000 seconds":
(https://github.com/bitcoin/bitcoin/issues/32855#issuecomment-3058210889)
Thanks for @Sjors ' quick clue on the potential issue. I searched the CI output and agree that it's likely due to the calling of

`return RunCommandParseJSON(m_command + " --fingerprint " + m_fingerprint + NetworkArg() + " getdescriptors --account " + strprintf("%d", account));
`
which requires a read from stdin in `test/functional/mocks/signer.py`
```
if not sys.stdin.isatty(): # on my local mac test, this is not from terminal so it will try to run read()
buffer = sys.stdin.read() # this
...
💬 HowHsu commented on pull request "index: fix wrong assert of current_tip == m_best_block_index":
(https://github.com/bitcoin/bitcoin/pull/32878#issuecomment-3058232808)
> @HowHsu, check this test [furszy@f2b8a06](https://github.com/furszy/bitcoin-core/commit/f2b8a06060e2a313f35101454c7669d46e0edc38). It triggers the reorg assertion failure without needing to deduplicate the index synchronization code. The test fails without your fix commit and passes with it. Feel free to cherry-pick it.
>
> > The Sync in the test is same as in bitcoind, expect the thread synchronization code which is to trigger the reorg. The only cons is we have to keep it updated when the
...
📝 stickies-v opened a pull request: "rest/rpc: use more util::Join"
(https://github.com/bitcoin/bitcoin/pull/32942)
There are a few places in the RPC and REST code where we manually join multiple items into a string. This PR removes a bunch of that logic and replaces it with `util::Join`. To minimize unnecessary container allocations and simplify implementations, the first commit slightly refactors `util::Join` so it can subsequently be used with range adaptors.

Tangentially, since we're touching the code anyway, refactor and modernize `rf_names` into a `std::map`.
💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#discussion_r2198282402)
mempool is already clean?
🤔 stickies-v reviewed a pull request: "rest: replace `rf_names[0].rf` by `RESTResponseFormat::UNDEF`"
(https://github.com/bitcoin/bitcoin/pull/32884#pullrequestreview-3006657167)
post-merge ACK 6d19815cd44031ff2b45fc9532f579cd81c62749

Looking at this code inspired me to make some related modernizations to the `rf_names` code: https://github.com/bitcoin/bitcoin/pull/32942