💬 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
...

💬 Christewart commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#discussion_r2528317136)
Ok i think i understand what you are saying conceptually - but perhaps I'm being a bit dense - but toggling the `reject_reason` comment that I have in https://github.com/bitcoin/bitcoin/pull/31823/commits/94a1bbb0d7f70513b68b28d4111cac56efce864a doesn't seem to affect whether the `p2p_invalid_tx.py` test suite passes or fails. I.e. the `reject_reason` seems irrelevant?

I'm a bit short on time at the moment and will take a look further in the next day or 2, but figured I would throw that out
...

💬 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-3533900033)
i patched all versions of `python-xyz.scm` on the system (not sure how to determine which exactly i need to) with this:
```patch
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index cba99cce62..7d91059365 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -31650,6 +31650,7 @@ (define-public python-py-cpuinfo
(base32
"0h5wi1bfcqqr1x3j1pa7dmkx7siprsyksbsy80fl2sdrrgpji0b0"))))
(build-system python-build-system)
+
...

💬 yancyribbens commented on pull request "kernel: handle null or empty directories in implementation":
(https://github.com/bitcoin/bitcoin/pull/33867#discussion_r2528455954)
However, there really isn't a way to truly guard against UB here since a len could be inaccurate (greater than string length). Nothing that can really be done in that case though I guess.

💬 theStack commented on pull request "tests: Add witness commitment if we have a witness transaction in `FullBlockTest.update_block()`":
(https://github.com/bitcoin/bitcoin/pull/31823#discussion_r2528459159)
No worries, I think it's indeed a bit confusing. The testing of a reject reason is done by asserting that the corresponding string appears in the bitcoind log file. If no explicit `reject_reason` is specified in an invalid tx test case, the default value of `""` (empty string) is used, i.e. a submission error is still expected, but the exact reason for why it matters doesn't matter.

You would see a difference in behaviour if you specified `reject_reason = None`, meaning that the expected outc
...

💬 andrewtoth commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#discussion_r2528472456)
Also, just thinking about 32-bit systems and possibility of overflows here. Although unlikely unless spammed with many `sendrawtransaction` for a long time, would it be best to maybe make `m_num_to_open` a `atomic_uint64_t` to remove all doubt?

📝 naiyoma opened a pull request: "wallet: refactor ProcessDescriptorImport , validate descriptors before rescan"
(https://github.com/bitcoin/bitcoin/pull/33874)


Fixes: https://github.com/bitcoin/bitcoin/issues/33655

Currently, we validate and import descriptors in the same loop. If any descriptor fails validation, an error is added to the response array, but the complete response (including that error) is only returned to after a rescan completes—potentially wasting significant time when validation error could have been caught immediately.

This PR separates validation from processing: validate all descriptors first, and process in the second
...

🤔 TheCharlatan reviewed a pull request: "guix: build `bitcoin-qt` with static libxcb & utils"
(https://github.com/bitcoin/bitcoin/pull/33537#pullrequestreview-3466200695)
Tested the binaries on x86 ubuntu 24.04

Guix build
```
fe9e088e3481013bf59dfc500803b415bf29a28cb8721974040e1ea7d0a75770 guix-build-96963b888e5a/output/aarch64-linux-gnu/SHA256SUMS.part
7e070397b6bace67e0960c2e9838717c77117571eaff88aaad9e6c03d24a9400 guix-build-96963b888e5a/output/aarch64-linux-gnu/bitcoin-96963b888e5a-aarch64-linux-gnu-debug.tar.gz
9979dbb9896915bf99c9d23accccb5cab8f6b81de17c51d03c328763270f14f3 guix-build-96963b888e5a/output/aarch64-linux-gnu/bitcoin-96963b888e5a-aarc
...

💬 Sjors commented on pull request "mining: add getCoinbase()":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3534061801)
> The second PR removing the deprecated methods could also swap the coinbase TX with an empty one as suggested to avoid getBlock returning it.

It makes sense to split this idea over two PR's. I have a local branch already, but still need to split out the commits and get the commit order right.