Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 willcl-ark commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#issuecomment-3546745541)
Concept ACK.

I like the idea of us providing (useful) optional indexes.

I haven't done an in-depth code review yet, but at a high level I feel that this feature would be worthy of a release note? I have reindexed with this enabled and see that the index currently takes about 75GB of space, which could be worth mentioning in such a note so users know what they need before enabling.

```
❯ du -h .
74G ./txospenderindex/db
74G ./txospenderindex
```
💬 fanquake commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2537372396)
I can't remember where we landed on making this is a build option. Has anyone asked for the ability to turn this off at build time, as opposed to just a runtime (if it becomes the default). I can imagine that could be asked for, for someone wanting to do a from-source build without any arbitrary blobs. In that case, what is the process for that person to recreate, (after the fact), the `ip_asn.dat` file that will exist in this repo?
💬 l0rinc commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537409027)
> InsertNewlineAtEOF is already set to true in this pull

Yes, please scroll to the bottom of the patch above, I'm also setting `KeepEmptyLinesAtEOF` there.

My understanding from https://github.com/llvm/llvm-project/issues/63150 is that `InsertNewlineAtEOF` alone doesn't work in some versions
> The clang format option InsertNewlineAtEOF : true is not being applied, instead the newline is being removed

, so they have added `KeepEmptyLinesAtEOF` in `17` https://reviews.llvm.org/D152305:

...
💬 hebasto commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546810258)
> Which kernel are you running? `5.10.113-scw1` here. (Apparently released `Wed, 27 Apr 2022`. That's ancient especially in RISC-V land!)

```
# uname -srv
Linux 5.10.113-scw1 #1 SMP PREEMPT Fri Jul 12 15:31:22 UTC 2024
```
💬 hodlinator commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537452864)
As I understand it some people wanted to keep *additional* empty lines at the end of the file (\n\n). https://github.com/llvm/llvm-project/issues/63150#issuecomment-1579150950

I don't think this is a concern for us?
⚠️ fanquake opened an issue: "depends: fallback server missing Qt downloads"
(https://github.com/bitcoin/bitcoin/issues/33898)
Looks like `https://code.qt.io` is down / having issues. None of the files needed from that site are available from the fallback server:
```bash
Checksum missing or mismatched for qt source. Forcing re-download.
Fetching qtbase-everywhere-src-6.7.3.tar.xz from https://download.qt.io/archive/qt/6.7/6.7.3/submodules
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
100 281 100 281 0
...
💬 l0rinc commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537483356)
The issue claims:
> For C and C++ an empty line at the end of each file (header and implementation) is mandatory.

Hmmm, so maybe the problem was that multiple empty lines were changes to none? I can try it out a bit later, but I'm working on something else locally...
👍 willcl-ark approved a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33611#pullrequestreview-3477117974)
ACK 4917d0c0de50da204b002bd4ae0c53cafd268f0c
💬 vasild commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3547036442)
Indeed the GUI signs the transaction and the RPC does not. Here is the GUI code path:

```
SendCoinsDialog::sendButtonClicked()
SendCoinsDialog::PrepareSendText()
WalletModel::prepareTransaction() // result saved in SendCoinsDialog::m_current_transaction
WalletImpl::createTransaction(/*sign=*/!wallet().privateKeysDisabled()) // sign=true
CreateTransaction(sign=true)
```

`walletcreatefundedpsbt` RPC code path:
```
FundTransaction()
CreateTransaction(sign=false)
```

Why is
...
💬 maflcko commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537644226)
> I don't think this is a concern for us?

Agree, I don't see the issue for us, but happy to test, if there are steps to test.

> Hmmm, so maybe the problem was that multiple empty lines were changes to none?

I can't reproduce this locally:

```
echo '' >> src/init.cpp
echo '' >> src/init.cpp
git diff

# Restore newline:
git diff -U0 | ./contrib/devtools/clang-format-diff.py -p1 -i -v
```

I'll leave this as-is for now, but I am happy to review a separate pull changing the valu
...
💬 maflcko commented on issue "depends: fallback server missing Qt downloads":
(https://github.com/bitcoin/bitcoin/issues/33898#issuecomment-3547055070)
They are cgit requests, so likely the server is just dos'd by llm bots
💬 stickies-v commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2537688942)
It doesn't return a header, though. I'm not saying we shouldn't add it, but it is orthogonal to the PR, and it is bundled in an unrelated commit without motivation. I think doing this separately makes more sense, I'd like to think more about how we want to expose this functionality, whereas the actual block header changes are quite straightforward and mechanistic.
💬 waketraindev commented on pull request "test, refactor: Embedded ASMap [1/3]: Selected minor preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2537716651)
> The call should be fully optimized out anyway, no?

I sure hope so!
It's a test-only code, so not a big deal.
💬 hebasto commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3547137380)
> > Which kernel are you running? `5.10.113-scw1` here. (Apparently released `Wed, 27 Apr 2022`. That's ancient especially in RISC-V land!)
>
> ```
> # uname -srv
> Linux 5.10.113-scw1 #1 SMP PREEMPT Fri Jul 12 15:31:22 UTC 2024
> ```

I've just checked out Fedora 40:
```
# uname -srv
Linux 5.10.113-scw1 #1 SMP PREEMPT Mon Jul 15 10:10:04 UTC 2024
```
👍 stickies-v approved a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896#pullrequestreview-3477255388)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d
🚀 fanquake merged a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33611)
💬 TheCharlatan commented on pull request "kernel: Add block header support and validation":
(https://github.com/bitcoin/bitcoin/pull/33822#discussion_r2537757275)
I think it should be part of this PR. We should not make changes to the API if we don't know how completing simple tasks will look like in the future. If there are doubts about how this should be exposed, this seems like the correct place to voice them and come up with something better. Would you be more on favour if it returned the header directly instead of having to retrieve the header from the entry?
👍 hebasto approved a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896#pullrequestreview-3477293559)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d.
💬 willcl-ark commented on pull request "guix: produce a `-static-pie` bitcoind":
(https://github.com/bitcoin/bitcoin/pull/25573#issuecomment-3547222727)
> > ;; --enable-static-nss isn't used yet, because it has been broken
> > ;; since 2.33: [sourceware.org/bugzilla/show_bug.cgi?id=27959](https://sourceware.org/bugzilla/show_bug.cgi?id=27959).
>
> What are the exact implications of this? I guix-built this branch and loaded it into a scratch docker container and the dns seeds were connected to and loaded fine. Is this coming from my host system perhaps, even inside a scratch container?
>
> I also tested the binary on alpine and it appeared
...