💬 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

👍 l0rinc approved a pull request: "build: suggest -DENABLE_IPC=OFF when missing capnp"
(https://github.com/bitcoin/bitcoin/pull/33290#pullrequestreview-3186721892)
Concept ACK

💬 l0rinc commented on pull request "build: suggest -DENABLE_IPC=OFF when missing capnp":
(https://github.com/bitcoin/bitcoin/pull/33290#discussion_r2323169139)
We might also want to mention that `doc/build-*.md` contains instructions for installing this new dependency, should they choose to not add `-DENABLE_IPC=OFF`

💬 davidgumberg commented on pull request "gui: Avoid pathological QT text/markdown behavior...":
(https://github.com/bitcoin-core/gui/pull/886#issuecomment-3255145030)
> You implying that an upstream bug must be searched in Qt?

"Bug" might be too strong, having one HTML element be so large is probably pretty unsually behavior for users of `QTextEdit`, but I can't say for sure what is going wrong in [`QTextMarkdownWriter::writeFrame`](https://github.com/qt/qtbase/blob/b617d1176593963a2a9ed21dd5d9a63e84a09400/src/gui/text/qtextmarkdownwriter.cpp#L95).


> possible nit:
>
> ```
> PlainCopyQTextEdit
> ^
> ```

I don't really agree, I figure
...

💬 l0rinc commented on pull request "test: Remove polling loop from test_runner (take 2)":
(https://github.com/bitcoin/bitcoin/pull/33141#discussion_r2323179848)
We have `self.test_list.pop(0)` at the beginning: https://github.com/bitcoin/bitcoin/blob/master/test/functional/test_runner.py#L724

👍 l0rinc approved a pull request: "wallet, refactor: Remove Legacy check and error"
(https://github.com/bitcoin/bitcoin/pull/33082#pullrequestreview-3186748868)
code review ACK d3c5e47391e2f158001e3e199d625852c7f18998

It also removes an objectionable translation 👍