💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1632823304)
Actually, I just realized I was over complicating this: you don't need to refactor `IsDust`, you just need to wrap it with an `IsDust` function that turns a `CRecipient` into a `CTxOut`, same as `GetSerializeSizeForRecipient`. Will update.

👍 fanquake approved a pull request: "build: warn on self-assignment"
(https://github.com/bitcoin/bitcoin/pull/30235#pullrequestreview-2107065005)
ACK 15796d4b61342f75548b20a18c670ed21d102ba8 - not a huge fan of inline pragma usage, but this seems fine, given it's to work around an already-fixed compiler bug, and we'll only be carrying it for a shortish time in any case.

🚀 fanquake merged a pull request: "build: warn on self-assignment"
(https://github.com/bitcoin/bitcoin/pull/30235)

💬 fanquake commented on pull request "Enable clang-tidy checks for self-assignment":
(https://github.com/bitcoin/bitcoin/pull/30234#issuecomment-2157716676)
> @fanquake Would you be ok with carrying this in our subtree? I'm not sure of a better solution.

Yea I think that's fine.

💬 fanquake commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1632843604)
Wondering if it's just sporadic, given previous pushes passed, i.e https://cirrus-ci.com/task/5429980706897920, and it's doesn't look like much has changed upstream recently. Will push again soonish.

💬 glozow commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#issuecomment-2157725577)
ACK 429ec1aaaa

Thank you for rebasing!

💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1632851441)
Yeah, I guess it is similar to the intermittent wine errors on master that are already happening too frequent

🤔 glozow reviewed a pull request: "test: expand LimitOrphan and EraseForPeer coverage"
(https://github.com/bitcoin/bitcoin/pull/30082#pullrequestreview-2107113637)
ACK eaf4de028de8fa13227b6089785889f1c6c15b4d

💬 hebasto commented on pull request "build: warn on self-assignment":
(https://github.com/bitcoin/bitcoin/pull/30235#issuecomment-2157737186)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/227.

💬 fanquake commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2157739910)
> Any reason this is still in draft? I agree it seems useful do enable before the CMake switch.

Rebased, and fixed the conflict. Added another change for the Win64 CI. We'll probably need to fixup the failing ARM and previous releases job upstream:
```bash
In file included from minisketch/src/fields/generic_common_impl.h:12,
from minisketch/src/fields/generic_4bytes.cpp:12:
minisketch/src/fields/../int_utils.h:162:7: error: "HAVE_CLZ" is not defined, evaluates to 0 [-Werr
...

💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2157745629)
Re https://github.com/bitcoin/bitcoin/pull/30132#pullrequestreview-2106239011

> is this tested anywhere?

No, and I'm not sure it should be given we don't support this. Maybe we can add a comment and assert that just wiping the block index db is not supported for now?

💬 hebasto commented on pull request "build: add `-Wundef`":
(https://github.com/bitcoin/bitcoin/pull/29876#issuecomment-2157769746)
> This was the multiprocess job failure:

cc @ryanofsky

💬 cbergqvist commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1632112358)
(Would be nice to add something like
```
# Node3, -bind=... and -listenonion.
self.expected.append(
[
[f"-bind=127.0.0.1:{port}", "-listenonion"],
[(loopback_ipv4, port)]
],
)
port += 1
```
But would then also be nice to verify whether a port was being used for onion traffic or not, to distinguish Node3 from Node2, which I don't know how to do).

💬 cbergqvist commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1632107735)
Thanks for taking my suggestion in https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1607299718! To increase readability one can also use the named loop-variable here, sorry I didn't mention it before:
```suggestion
assert_equal(binds, set(expected_services))
```

💬 cbergqvist commented on pull request "Make it possible to disable Tor binds and abort startup on bind failure":
(https://github.com/bitcoin/bitcoin/pull/22729#discussion_r1632113314)
Feels confusing to have the test framework diverge in behavior this much from default `bitcoind` behavior.

Would be less surprising if the test framework avoided Tor port collisions through spreading out the ports among the nodes as is done for (P2P)Port and RPCPort. A suggested alternative to your current workaround is in 6b785873557696cc611d58fdcac5a3d54622082c.

🤔 ismaelsadeeq reviewed a pull request: "test: doc: fix units in tx-size standardness test (s/vbytes/weight units)"
(https://github.com/bitcoin/bitcoin/pull/30254#pullrequestreview-2107249409)
ACK d1581c6048478cf70c5fb9ec5ebc178f16b376b8

C.I Failure is unrelated https://github.com/bitcoin/bitcoin/actions/runs/9436375635/job/25991001694?pr=30254

💬 ismaelsadeeq commented on pull request "test: doc: fix units in tx-size standardness test (s/vbytes/weight units)":
(https://github.com/bitcoin/bitcoin/pull/30254#discussion_r1632929909)
Are these `bytes` values `vbytes`?

👍 willcl-ark approved a pull request: "build: Remove --enable-gprof"
(https://github.com/bitcoin/bitcoin/pull/30257#pullrequestreview-2107256462)
crACK fa780e1c25e8e98253e32d93db65f78a0092433f

Agree on the rationale given here for removal.

I've previously tried to use `gprof` in place of `perf` (when `addr2line` was broken/unusably slow on my Ubuntu kernel) and hit against many of the pain points ("requirements") @maflcko mentions in OP, which also had me wondering how often this is really used...

I did find a reference to `gprof` remaining in depends: https://github.com/bitcoin/bitcoin/blob/7fd4905c403ccb233f489e2f6a6aa3cd53b033
...

💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#issuecomment-2157846717)
Updated https://github.com/bitcoin/bitcoin/commit/b44f00251a77a3360888eeb10ec903375eb1e0f8 -> https://github.com/bitcoin/bitcoin/commit/f004b1625bcb9b833f36af47b53675e3dd77135b ([serialize-size-for-crecipient-00](https://github.com/josibake/bitcoin/tree/serialize-size-for-crecipient-00) -> [serialize-size-for-crecipient-01](https://github.com/josibake/bitcoin/serialize-size-for-crecipient-01), [compare](https://github.com/josibake/bitcoin/compare/serialize-size-for-crecipient-00..serialize-size-
...

💬 josibake commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1632944441)
Update, thanks @S3RK !