Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ glozow commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3245366459)
> Not everyone knows what "33106" is. To avoid it being interpreted as trying to hide something, could you please detail the changes in the PR title and description to help the reviewers and provide full transparency?
> Can we please make this not common practice anymore? Memorising PR numbers is no fun. "Followups for min fee changes" would have been fine.

Agree with this practice, changed now. The intention was not to "hide something" - that's pretty hurtful.
πŸ“ fanquake opened a pull request: "contrib: update fixed seeds"
(https://github.com/bitcoin/bitcoin/pull/33283)
Update the fixed seeds pre 30 branch off.
πŸ’¬ hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245511525)
> Sure, see https://github.com/maflcko/b-c-gui-translations-review/tree/99bf41c4e5ab74eee8c248b240e8e940e47e09ec/reviews

Many thanks, @maflcko! This is indeed very useful.

I discarded only a few suggestions for the Ukrainian translation:

1.
```
<source>Copy &amp;raw transaction</source>
<translation>ΠšΠΎΠΏΡ–ΡŽΠ²Π°Ρ‚ΠΈ &amp;всю Ρ‚Ρ€Π°Π½Π·Π°ΠΊΡ†Ρ–ΡŽ</translation>

ERR
The term "raw transaction" in the Bitcoin context refers specifically to the transaction in its raw (serialized) he
...
πŸ’¬ glozow commented on pull request "p2p: never check tx rejections by txid":
(https://github.com/bitcoin/bitcoin/pull/33066#issuecomment-3245518101)
Closing for now. We can revisit this cleanup later.
βœ… glozow closed a pull request: "p2p: never check tx rejections by txid"
(https://github.com/bitcoin/bitcoin/pull/33066)
βœ… glozow closed a pull request: "[NO MERGE] BIP331 Ancestor Package Relay"
(https://github.com/bitcoin/bitcoin/pull/27742)
πŸ’¬ glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#issuecomment-3245588079)
Ok Drahty I'll take the hint :joy:.

Most of the code here is in master now - the Orphan Resolution Module is part of `TxDownloadManager` (#30110, #31397, etc.) and the `TxOrphanage` eviction changes (#31829, etc.) replace the token bucket protection stuff here. One difference is that we didn’t add the `TxRequestTracker orphan_request_tracker` for vanilla orphan resolution, but that is probably needed for ancestor package relay.

Parts 2 and 3 are relatively smaller, but still need the mempo
...
πŸ’¬ ellenkampguus commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3245607749)
I want to reopen the discussion as I am starting to doubt if Bitcoin Core is still catering to the Bitcoin user's needs.
πŸ’¬ sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3245620914)
Rebased, and cherry-picked @Sjors' commit.
πŸ’¬ pinheadmz commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3245622643)
What "user need" do you feel is not being addressed? Legacy wallets can be converted to the new improved format. This has been at least a [five-year project](https://github.com/bitcoin/bitcoin/issues/20160)
πŸ€” danielabrozzoni reviewed a pull request: "rpc, logging: add backgroundvalidation to getblockchaininfo"
(https://github.com/bitcoin/bitcoin/pull/33259#pullrequestreview-3176862036)
Concept ACK

I did a very rough first pass and the code looks good to me. As others have said, I would like to see a functional test testing the newly introduced fields when the background validation is in progress
πŸ’¬ hebasto commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3245626058)
cc @purpleKarrot @theuni
πŸ’¬ hebasto commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245657952)
Can it be localized into the `src/ipc/capnp/.clang-tidy` file?
πŸ’¬ hebasto commented on pull request "cmake: make missing Python interpreter behaviour more explicit":
(https://github.com/bitcoin/bitcoin/pull/33278#issuecomment-3245677855)
cc @purpleKarrot
πŸ’¬ m3dwards commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2316358241)
Lint job looks at GPG keys on commits and AFAIK rarely fails. It's not like a "normal" lint that checks code style and therefore often fails.
πŸ’¬ marcofleon commented on issue "build: clang-16 build broken on Debian Bookworm":
(https://github.com/bitcoin/bitcoin/issues/33279#issuecomment-3245714093)
Just got the same error with Clang 19. Installing the default Capn Proto packages (0.9.2) won't work it seems. I upgraded to 1.1.0 by building capnp from source and bitcoin builds now. It even builds an extra 1%...

<img width="1459" height="324" alt="Image" src="https://github.com/user-attachments/assets/19cca262-77e1-4984-ad19-ceb84c0c573d" />
πŸ’¬ m3dwards commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3245714292)
ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
πŸ’¬ ryanofsky commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245742642)
> Can it be localized into the `src/ipc/capnp/.clang-tidy` file?

That seems like a good idea. But even the current version of this PR (c157a1bc8388fecec85f61286ae928721aa8d78a) doesn't seem to be working right now. I wonder if it is because the files are generated files not inside the source directory? I'm not sure what change might make sense to fix this.

Current tidy job output is:

- https://cirrus-ci.com/task/5720235140972544
- https://api.cirrus-ci.com/v1/task/5720235140972544/logs
...
πŸ“ ryanofsky converted_to_draft a pull request: "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning"
(https://github.com/bitcoin/bitcoin/pull/33281)
Disable `clang-analyzer-core.UndefinedBinaryOperatorResult` warning because it produces a false positive in a Cap'n Proto header:

```c++
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
609 | return *(void**)(*(char**)obj + voff);
```

This was reported by fanquake in https://github.com/bitcoin/bitcoin/issues/33256 and was previously addressed in https://github.com/bitcoin-core/libmultiproc
...
πŸ’¬ ryanofsky commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245745115)
Marking as draft because the change does not seem to have any effect right now.