💬 romanz commented on pull request "index: store per-block transaction locations for efficient lookups":
(https://github.com/bitcoin/bitcoin/pull/32541#issuecomment-3255024309)
> Would be nice for the `LocationsIndex` if hashes were lazily computed (avoided) and transactions had a serialized size field that was cached upon deserialization so we don't need to call `GetSerializeSize`.

In https://github.com/romanz/bindex-rs, I am using https://github.com/RCasatta/bitcoin_slices which allows parsing Bitcoin blocks and transactions.
Maybe we can use a similar approach for building the indices (since each index may need a different part of the indexed transaction)?

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323097757)
Fixed.

💬 sipa commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2323098780)
Thanks! It worked for a tag, but not for a commit.

💬 willcl-ark commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#discussion_r2323116369)
I have extended https://github.com/bitcoin-core/libmultiprocess/pull/205 to include more helpful error messages when the capnproto package is not found by cmake.

👍 l0rinc approved a pull request: "test: Remove polling loop from test_runner (take 2)"
(https://github.com/bitcoin/bitcoin/pull/33141#pullrequestreview-3186496990)
tested ACK fa4885ef2fde2b9fd9eb7367564a2946a45f20ac

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323012972)
My understanding is that in Python a list `self.test_list.pop(0)` is O(n) so consuming all elements is quadratic this way.
A `collections.deque` with `popleft` might describe the desired behavior better (performance-wise it would likely be the same for a few hundred elements)

nit: `jobs` isn't a "list" anymore

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323018994)
nit: to avoid confusion with previous tuple, since job is a Future now, we might want to name it as `fut` as well to be consistent with above

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323014077)
Even better than what I suggested.

This is now sorted by name instead of insertion order if I understand it completely, but it's just a log so I'm fine with either - this seems more stable.

Alternatively, if we don't like future keys, we could also use the names as keys and Futures as values - but I'm fine with it as-is.

💬 l0rinc commented on issue "-loadblock doesn't work":
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3255070665)
It also seems to me like a xor mismatch.
Could you quote the start of the logs for both source and target instance what the obfuscation key values were?

```bash
cat debug.log | grep 'Using obfuscation key for'
```

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323132294)
> Alternatively, if we don't like future keys, we could also use the names as keys and Futures as values - but I'm fine with it as-is.

I don't think this works, because names could be duplicate and a dict can not have duplicate keys.

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323142111)
I don't think `pop` was ever used, it was `remove`. Though, this is just a sanity-check, so I don't think it matters. Will leave as-is for now.

💬 maflcko commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323144604)
will do if i have to re-touch

💬 0xB10C commented on pull request "contrib: update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/33283#issuecomment-3255107149)
> i2p and cjdns nodes

fwiw https://21.ninja/reachable-nodes/nodes-by-net-type/ shows about 3k reachable i2p and 13 cjdns nodes

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#discussion_r2323064850)
Are we listing coauthors here?

👍 l0rinc approved a pull request: "doc: archive v29.1 release notes"
(https://github.com/bitcoin/bitcoin/pull/33309#pullrequestreview-3186572490)
review ACK 61ec8866c63910c76c030ae828a707a71449399a

I left some suggestions, I'm fine with doing them in follow-ups or future releases or not at all. I haven't verified if the PR numbers match their descriptions (except for the RPC typo).

Could you please add some description to the PR to help others reproduce the changes here?

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#discussion_r2323070660)
nit: in other cases we've used the `#` notation instead: https://github.com/bitcoin/bitcoin/pull/33040#discussion_r2224549091

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#discussion_r2323145895)
Typo in original PR title, maybe we should fix it here at least
```suggestion
- #32708 rpc, doc: update listdescriptors RPC help
```

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#discussion_r2323082253)
Should we mention the value we had before, i.e.: "has been changed from 1 satoshi per vB to 1 satoshi per kvB"

💬 l0rinc commented on pull request "doc: archive v29.1 release notes":
(https://github.com/bitcoin/bitcoin/pull/33309#discussion_r2323093717)
nit: if this is alphabetic, this should like go before `ismaelsadeeq`

💬 willcl-ark commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#discussion_r2323167964)
OK I think this push got it: https://github.com/bitcoin/bitcoin/actions/runs/17473683984/job/49628131536?pr=33302#step:5:339