💬 fanquake commented on pull request "Don't fix Python patch version":
(https://github.com/bitcoin/bitcoin/pull/33051#issuecomment-3113924602)
> but then not just using pyenv install

`pyenv install` is a wrapper around `python-build`. Looks like it was used to use slightly less resources.

👍 ryanofsky approved a pull request: "util: Abort on failing CHECK_NONFATAL in debug builds"
(https://github.com/bitcoin/bitcoin/pull/32588#pullrequestreview-3052141897)
Code review ACK fac50ee1f9e577527650397891d356bca6316321, just rebased, added back functional test, and tweaked fuzz test since last review.

Overall this looks good and conceptually I like this change because it makes all the checking macros do the exact same thing in debug builds and abort, only having varying behavior in release builds.

💬 ryanofsky commented on pull request "util: Abort on failing CHECK_NONFATAL in debug builds":
(https://github.com/bitcoin/bitcoin/pull/32588#discussion_r2228833280)
In commit "test: Allow testing of check failures" (fa8251bb883300bb84cad5486468d07b5b1b7322)

Not important, but I think it makes more sense to drop this test in the next commit instead dropping it here, because the next commit is what actually breaks this test and the claim that this fuzz test is "redundant to the new unit test" is not 100% accurate, since the fuzz test and unit test execute with different `CheckFailuresAreExceptions` values.

✅ maflcko closed a pull request: "test: handle potential None value for change address in setlabel"
(https://github.com/bitcoin/bitcoin/pull/33055)

💬 maflcko commented on pull request "test: handle potential None value for change address in setlabel":
(https://github.com/bitcoin/bitcoin/pull/33055#issuecomment-3113970375)
Thanks, but there is no need to open an LLM generated pull request without any description and motivation. Please do not submit LLM generated stuff that you do not understand yourself.

🤔 marcofleon reviewed a pull request: "[29.x] backport #32069"
(https://github.com/bitcoin/bitcoin/pull/33052#pullrequestreview-3052237027)
ACK c6fe6971bfa52d8fe2901a051f18da9f3bcb26a9

💬 raul-anton-2005 commented on pull request "test: handle potential None value for change address in setlabel":
(https://github.com/bitcoin/bitcoin/pull/33055#issuecomment-3113984652)
Mmmm, it was not LLM generated. I understood what I did since it was not a significant but good practice change. I can open it with an explenation if you want.

💬 maflcko commented on pull request "util: Abort on failing CHECK_NONFATAL in debug builds":
(https://github.com/bitcoin/bitcoin/pull/32588#discussion_r2228928982)
> the claim that this fuzz test is "redundant to the new unit test" is not 100% accurate, since the fuzz test and unit test execute with different `CheckFailuresAreExceptions` values.

I think it is 100% accurate, because `test_only_CheckFailuresAreExceptionsNotAborts` does not affect `CHECK_NONFATAL` in commit https://github.com/bitcoin/bitcoin/commit/fa8251bb883300bb84cad5486468d07b5b1b7322, so the fuzz test and unit test are 100% identical and redundant in this commit.

💬 mzumsande commented on pull request "net: Fix Discover() not running when using -bind=0.0.0.0:port":
(https://github.com/bitcoin/bitcoin/pull/32757#issuecomment-3114010941)
> ifconfig en0 alias 1.1.1.1

ok, you don't add it to the loopback interface.
But the instructions for Linux
`ifconfig lo:0 1.1.1.1/32 up && ifconfig lo:1 2.2.2.2/32 up # to set up`
do that, while #29984 resulted in `IFF_LOOPBACK` addresses being filtered out. So I don't think the problem from https://github.com/bitcoin/bitcoin/issues/31336 is fixed by this as the OP says.

🚀 fanquake merged a pull request: "[29.x] backport #32069"
(https://github.com/bitcoin/bitcoin/pull/33052)

💬 sstone commented on pull request "Add a "tx output spender" index":
(https://github.com/bitcoin/bitcoin/pull/24539#discussion_r2228937333)
I had forgotten this one , fixed in https://github.com/bitcoin/bitcoin/pull/24539/commits/f2a496c827442c7650e5742fdfd49147f0da0143

💬 maflcko commented on pull request "test: handle potential None value for change address in setlabel":
(https://github.com/bitcoin/bitcoin/pull/33055#issuecomment-3114023892)
If it wasn't LLM generated, it is still wrong, because the value is never `None`. And if it was, it would have already been handled properly.

💬 fanquake commented on pull request "guix: Always canonicalize HOST using `./depends/config.sub`":
(https://github.com/bitcoin/bitcoin/pull/21671#issuecomment-3114038666)
I'm going to remove "Up for grabs" here. Not entirely convinvced that we should end up doing this.

💬 ryanofsky commented on pull request "util: Abort on failing CHECK_NONFATAL in debug builds":
(https://github.com/bitcoin/bitcoin/pull/32588#discussion_r2228952920)
> I think it is 100% accurate, because `test_only_CheckFailuresAreExceptionsNotAborts` does not affect `CHECK_NONFATAL` in commit [fa8251b](https://github.com/bitcoin/bitcoin/commit/fa8251bb883300bb84cad5486468d07b5b1b7322), so the fuzz test and unit test are 100% identical and redundant in this commit.

I wouldn't say it's 100% accurate to say that two tests have identical coverage just because the they have the same results in one commit, especially when the results immediately diverge in th
...

💬 ryanofsky commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#issuecomment-3114080535)
Rebased f92422e308ec33e2b211b866e218efacc77a4f7f -> c0d9515a3aef468bf4c5949c419ab1c9bab0dfa3 ([`pr/ipc-chain.14`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.14) -> [`pr/ipc-chain.15`](https://github.com/ryanofsky/bitcoin/commits/pr/ipc-chain.15), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipc-chain.14-rebase..pr/ipc-chain.15)) to fix silent merge conflict with #32862 and spelling error. (Thanks zaidmstrr and maflcko for pointing these out!)

💬 darosior commented on pull request "[29.x] Backport #32521":
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3114084771)
Should i rebase now that #33052 is merged?

🤔 glozow reviewed a pull request: "[29.x] Backport #32521"
(https://github.com/bitcoin/bitcoin/pull/33013#pullrequestreview-3052376345)
ACK f25dc84b2892e6bdbbd0471add9fcb2757700981

No you don't need to rebase

🚀 glozow merged a pull request: "[29.x] Backport #32521"
(https://github.com/bitcoin/bitcoin/pull/33013)

💬 fanquake commented on pull request "net, validation: don't punish peers for consensus-invalid txs":
(https://github.com/bitcoin/bitcoin/pull/33050#issuecomment-3114131937)
CI failure here is (#33015).

💬 fanquake commented on issue "intermittent timeout in wallet_signer.py : sendall timed out":
(https://github.com/bitcoin/bitcoin/issues/33015#issuecomment-3114133331)
https://cirrus-ci.com/task/6439525662064640?logs=ci#L1711