Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 vasild commented on issue "Load PSBT error: Unable to decode PSBT":
(https://github.com/bitcoin/bitcoin/issues/30070#issuecomment-3533027320)
I can reproduce this. A cursory debug shows that `txin.scriptWitness` IsNull but `txin.scriptSig` is not empty:

https://github.com/bitcoin/bitcoin/blob/e221b2524659d22cc5a2b0d0023254115a7a5622/src/psbt.h#L1253-L1254

A transaction created via the RPC does not exhibit this problem. Only when created from the GUI.
📝 ryanofsky converted_to_draft a pull request: "kernel: Improve logging API"
(https://github.com/bitcoin/bitcoin/pull/33847)
Simplify kernel logging API and try to connect it to the rest of the kernel API by letting log options be set on `Logger` objects directly and `Logger` objects to be associated with `Context` objects directly. Also drop `logging_disable()` function and avoid having library accumulate log messages in a 1MB buffer by default.

Changes are intended to make the API a little less surprising and more future proof. Rationale is described in some more detail the commit message.

This change is not b
...
💬 ryanofsky commented on pull request "kernel: Improve logging API":
(https://github.com/bitcoin/bitcoin/pull/33847#issuecomment-3533073827)
> prefer to convert this to draft

Sure happy to do that while there is conceptual discussion.

> having the interface behave in unexpected ways is just not desirable

I think I've pointed out practical ways the current interface behaves in unexpected ways and is counterintuitive, and practical ways this PR is an improvement. The concerns you and stringintech raised do seem plausible, but also vague so if there are any concrete scenarios you can think of where this PR might create a proble
...
💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2527749355)
Ahh right, and we don't have to recheck `current_value` each time which will have 1 less atomic operation if there is a lost race. If you push again maybe touch up to:
```C++
size_t current_value{m_num_to_open};
size_t new_value;
do {
new_value = current_value > n ? current_value - n : 0;
} while (!m_num_to_open.compare_exchange_weak(current_value, new_value));
```
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527752096)
Seems reasonable, done.
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527763616)
No, but upon looking into it I think this is still better. The `size()` function uses the current file handle while `file_size()` would create a new one which could create race conditions etc.
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527764328)
Done
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527765721)
Added a more extensive test for this in a new commit.
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527766160)
Done
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527777820)
That's a bug, nice find! The line where I had `seek(pos, SEEK_END)` in the `size()` implementation should have been `seek(0, SEEK_END)`, this lead to these wrong numbers. In these tests the position was at the end (7) and `seek(pos, SEEK_END)` moved the pos to EOF plus `pos`, which was 14. Fixed!
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#discussion_r2527778252)
Done
💬 fjahr commented on pull request "test, refactor: Embedded ASmap selected preparatory work":
(https://github.com/bitcoin/bitcoin/pull/33026#issuecomment-3533149878)
Addressed all feedback from @hodlinator , thanks for the review!
💬 fjahr commented on pull request "init: Require explicit -asmap filename":
(https://github.com/bitcoin/bitcoin/pull/33770#issuecomment-3533199372)
ACK deac31dd779d28a8c01f1c3153650f5827cfe75d

Reviewed code and tested locally. I had grepped for other relevant mentions of the default location previously.
👍 stickies-v approved a pull request: "refactor: remove incorrect lifetimebounds"
(https://github.com/bitcoin/bitcoin/pull/33870#pullrequestreview-3465409234)
ACK 99d012ec80a4415e1a37218fb4933550276b9a0a

Fixes lifetime attributes, improves clarity of interface by excluding the possibility of null result.
💬 fanquake commented on pull request "guix: build `bitcoin-qt` with static libxcb & utils":
(https://github.com/bitcoin/bitcoin/pull/33537#issuecomment-3533276038)
Tested a Guix build from this branch on a Ubuntu 24.04 (aarch64 VM)
<img width="1130" height="747" alt="ldd" src="https://github.com/user-attachments/assets/dc0919be-7ee1-45c5-8bee-e2c58d0e9687" />

<img width="1028" height="723" alt="gui" src="https://github.com/user-attachments/assets/800aec26-f812-4bec-bac7-e527f7ac7a82" />
⚠️ laanwj opened an issue: "guix build fails on RISC-V due to python-py-cpuinfo test failure"
(https://github.com/bitcoin/bitcoin/issues/33873)
i'm trying to do a guix build (of the master branch) on RISC-V hardware. The device is a [Scaleway EM-RV1-C4M16S128-A](https://labs.scaleway.com/en/em-rv1/) server from Scaleway.

```
Architecture: riscv64
Byte Order: Little Endian
CPU(s): 4
On-line CPU(s) list: 0-3
```

```
processor : 0
hart : 0
isa : rv64imafdcsu
mmu : sv39
cpu-freq : 1.848Ghz
cpu-icache : 64KB
cpu-dcache : 64KB
cpu-l2cache : 1M
...
💬 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-3533560301)
Which OS?

I’ve found Debian to be more suitable for RISC-V.
💬 yancyribbens commented on pull request "kernel: handle null or empty directories in implementation":
(https://github.com/bitcoin/bitcoin/pull/33867#discussion_r2528229709)
> Not all users will use language bindings, and regardless, I don't think the distinction between "direct usage" and "wrapped usage" is meaningful here. Our implementation introduces UB when a non-zero length is used with a nullptr, so our implementation is responsible for preventing the UB.

Makes sense

> How? In the API, a string is defined as {ptr, ptr+len}. Being empty means a size of 0, and size depends on len. We could introduce the requirement for strings to be null-terminated and re
...
💬 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-3533735681)
It's debian (Debian GNU/Linux forky/sid). Intentially chose that because i assumed it would be least hassle 😄
👍 andrewtoth approved a pull request: "net_processing: reorder the code that handles the VERSION message"
(https://github.com/bitcoin/bitcoin/pull/33792#pullrequestreview-3465930909)
ACK 52149b0e2b7897cc2ea059a9b50c4cb3b9fcd722

Moving the `WTXIDRELAY` and `SENDADDRV2` `MakeAndPushMessage`s doesn't change the order of messages pushed. Nothing between below where they are moved from and above where they are moved to has an effect on any of the members of `pfrom` accessed in `MakeAndPushMessage`.

Moving `mapped_as` and the subsequent log above does not change anything with `mapped_as` either, since the `pfrom.addr` is not modified in any of the lines between where it is m
...