Bitcoin Core Github
44 subscribers
122K links
Download Telegram
💬 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-3546510385)
> > It builds fine for me on an instance from the same provider:
>
> Okay, now i'm confused. What operating system? Also debian forky/sid?

Yes, debian forky/sid.
👍 hodlinator approved a pull request: "clang-format: Set InsertNewlineAtEOF: true"
(https://github.com/bitcoin/bitcoin/pull/33896#pullrequestreview-3476584168)
ACK fa1bf6818f0910f997e5235a197ff51f2e18780d

Makes sense for this project to conform to the UNIXy style of ending all lines with `\n`.

Previous setting of having it `false` comes from 13f36c020f0329b5e975282b45292fdf2a495e31 which just added a bunch of newer settings to src/.clang-format with default clang-format values, rather than being a conscious decision by this project.

Have recently been discussing how GitHub and Git also seem to agree on this: https://github.com/bitcoin/bitcoin/pull/2
...
💬 Sjors commented on issue "IPC via TCP Sockets":
(https://github.com/bitcoin/bitcoin/issues/32802#issuecomment-3546616865)
RPC connections are not encrypted either and I'm not aware of any plans for changing that.

Maybe virtio socket support would be the easiest approach for getting Docker setups to work? That might be useful for RPC too.
💬 l0rinc commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537251411)
llvm@17 has a `KeepEmptyLinesAtEOF: false` as well - added exactly to avoid the problem we're trying to avoid here: https://github.com/llvm/llvm-project/issues/56054
Should we make it explicit here?
```patch
diff --git a/src/.clang-format b/src/.clang-format
--- a/src/.clang-format (revision 9d5bad11b6cbf50c776f9e9a583ac1fb79271fe8)
+++ b/src/.clang-format (date 1763460129408)
@@ -123,7 +123,7 @@
IndentWidth: 4
IndentWrappedFunctionNames: false
InsertBraces: false
-InsertNewlineAtEOF:
...
💬 fanquake commented on pull request "build: Embedded ASMap [3/3]: Build binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#discussion_r2537289781)
> a new creation process

What is the "new creation process":
* Who is involved (and has the permissions needed to merge into the required repos).
* How many total people are required?
* Can it happen at anytime, or does it require coordination days ahead of time?
* What should happen if the process creates data that isn't usable?
💬 rustaceanrob commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3546665825)
> moreover, I'm not sure what to expect in case getCoinbase doesn't exist? I guess capnp-rpc would bubble up some kind of error? does @rustaceanrob know?

I would assume a new client requesting `getCoinbase` of an old server would result in the [`ErrorKind::Unimplemented`](https://docs.rs/capnp/0.23.0/capnp/enum.ErrorKind.html). In the case of this response then a `getCoinbaseTx` could be used as a fallback.

As far as updating the `capnp` files in the Rust types, I can do so on a developmen
...
⚠️ Sjors opened an issue: "rfc: virtio-vsock for RPC and IPC"
(https://github.com/bitcoin/bitcoin/issues/33897)
> virtio-vsock provides a way for applications running on a guest VM and the host system to communicate with each other using the standard socket interface (`socket`, `connect`, `bind`, `listen`, `accept`). It defines a new socket address family (`AF_VSOCK`) and uses a (context id, port) pair of integers for identifying processes. The host system always has 2 as its context id while each guest VM is assigned a unique context id on startup.

https://chromium.googlesource.com/chromiumos/platform2/
...
💬 laanwj commented on issue "guix build fails on RISC-V due to python-py-cpuinfo test failure":
(https://github.com/bitcoin/bitcoin/issues/33873#issuecomment-3546710178)
Could also be a kernel bug:
```
[155604.743750] BUG: using smp_processor_id() in preemptible [00000000] code: guile/662593
[155604.751799] caller is debug_smp_processor_id+0x20/0x2a
[155604.757101] CPU: 3 PID: 662593 Comm: guile Not tainted 5.10.113-scw1 #1
[155604.763806] Call Trace:
[155604.766349] [<ffffffe0002040f0>] walk_stackframe+0x0/0x112
[155604.771928] [<ffffffe000fcd022>] show_stack+0x46/0x62
[155604.777075] [<ffffffe000fd6be2>] dump_stack_lvl+0x7a/0x9a
[155604.782567] [<ffffffe000fd6
...
💬 maflcko commented on pull request "clang-format: Set InsertNewlineAtEOF: true":
(https://github.com/bitcoin/bitcoin/pull/33896#discussion_r2537358027)
> llvm@17 has a `KeepEmptyLinesAtEOF: false` as well - added exactly to avoid the problem we're trying to avoid here: [llvm/llvm-project#56054](https://github.com/llvm/llvm-project/issues/56054)
> Should we make it explicit here?

Not sure I understand the problem you are referring to. `InsertNewlineAtEOF` is already set to `true` in this pull request. Can you add exact steps to reproduce the problem?
💬 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