💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686726401)
> it would help if you could say more specifically what you find confusing about this code

I'll try to rephrase.
Firstly, as you've mentioned as well, the name of the method hints at mutation, but it actually returns a copy of the view.
Second, it's inline + pass by value mutation, which implies it's not a simple search&replace type inline, since the parameters aren't consts, but it also can't just mutate the original values passed as parameters, so the copy of the parameter is implicit. Ma
...

✅ fanquake closed an issue: "Spurious markdown linter failure"
(https://github.com/bitcoin/bitcoin/issues/30496)

🚀 fanquake merged a pull request: "lint: Use consistent out-of-tree build for python and test_runner"
(https://github.com/bitcoin/bitcoin/pull/30499)

💬 edilmedeiros commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2243216680)
Isn't possible/desirable to try to get it from the Github repo instead?

https://github.com/miniupnp/miniupnp

💬 paplorinc commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1686729618)
> This pull request has more than a 100 comments for less lines of code changed

I did all the changes I requested during review (even provided diffs to speed it up), they're not complicated.

💬 hebasto commented on pull request "depends: Fetch miniupnpc sources from an alternative website":
(https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2243222945)
> Isn't possible/desirable to try to get it from the Github repo instead?

See https://github.com/bitcoin/bitcoin/pull/30151#issuecomment-2124512367:
> If that does keep the sha256 hash the same then maybe. Mind that github-generated tar.gzs aren't guaranteed to have a stable hash.

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#issuecomment-2243227225)
utACK 9afa2c9e50370b2918377f3f3eac0950a4296ec0

👍 ryanofsky approved a pull request: "Have createNewBlock() return a BlockTemplate interface"
(https://github.com/bitcoin/bitcoin/pull/30440#pullrequestreview-2191872106)
Code review ACK bde6c7f7b126beea1359990e012c76c5cafc34a6. Left one suggestion, but this change is much simpler now and look good.

💬 ryanofsky commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1686727569)
In commit "Have createNewBlock return BlockTemplate interface" (f434c8562281dfec1bab37e7232ca35fd759e25f)

While changing this it would make sense to add std::move() on line 148 below so the block data does not need to be copied there.

👍 hebasto approved a pull request: "contrib: simplify `test-security-check`"
(https://github.com/bitcoin/bitcoin/pull/30423#pullrequestreview-2191894000)
ACK 1bc9f64bee919bc46eb061ef8c66f936eb6a8918 (assuming my Guix hashes match; I'll provide them shortly).

My [concern](https://github.com/bitcoin/bitcoin/pull/30423#discussion_r1674030590) about `-pie` support for Windows is not a blocker.

📝 maflcko opened a pull request: " lint: Add missing docker.io prefix to ci/lint_imagefile "
(https://github.com/bitcoin/bitcoin/pull/30501)
Currently, the `ci/lint_imagefile` may pick the wrong (non-native) architecture due to the missing prefix.

For example, assuming the user has previously pulled an s390x image:

```
$ podman run --rm 'docker.io/s390x/debian:bookworm' dpkg --print-architecture
exec /usr/bin/dpkg: exec format error
```

Now, `debian:bookworm` will refer to the same image:

```
$ podman run --rm 'debian:bookworm' dpkg --print-architecture
exec /usr/bin/dpkg: exec format error
```

However, `docker.i
...

💬 maflcko commented on pull request "lint: Use consistent out-of-tree build for python and test_runner":
(https://github.com/bitcoin/bitcoin/pull/30499#discussion_r1686749450)
Done in https://github.com/bitcoin/bitcoin/pull/30501

👍 paplorinc approved a pull request: "lint: Add missing docker.io prefix to ci/lint_imagefile"
(https://github.com/bitcoin/bitcoin/pull/30501#pullrequestreview-2191914039)
utACK fa7bee13bf745d8d244fa8d3579a21016a0cb66d

Thanks!

👍 hebasto approved a pull request: "lint: Add missing docker.io prefix to ci/lint_imagefile"
(https://github.com/bitcoin/bitcoin/pull/30501#pullrequestreview-2191930364)
ACK fa7bee13bf745d8d244fa8d3579a21016a0cb66d.

✅ maflcko closed an issue: "Fuzz coverage for wallet RPCs is zero"
(https://github.com/bitcoin/bitcoin/issues/30458)

💬 maflcko commented on issue "Fuzz coverage for wallet RPCs is zero":
(https://github.com/bitcoin/bitcoin/issues/30458#issuecomment-2243304292)
> Added this in #29901.

Thanks!

I'll close my issue then. Happy to review a pull request if someone creates one, but probably no need to keep this issue open in the meantime.

💬 Sjors commented on pull request "Have createNewBlock() return a BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/30440#discussion_r1686795623)
Changed

💬 vasild commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2243385353)
Which are those flags?

🚀 fanquake merged a pull request: "depends: build zeromq with CMake"
(https://github.com/bitcoin/bitcoin/pull/29723)

💬 sipa commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1686762705)
In commit "tests: add key tweak smoke test"

The `merkle_root.IsNull()` condition will never hold here. Perhaps it makes sense to run this test in a loop, and only set `merkle_root` to `InsecureRand256()` in some of the iterations?