💬 kevkevinpal commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r1609172911)
I've updated this to instead only take three arguments and the last one being an optional message since I did not see anywhere we were already asserting `!=` in a chain.

This should clear up the message and allow the one location we do use a message to be used

updated in [4488120](https://github.com/bitcoin/bitcoin/pull/29500/commits/4488120ab7db775ce0ae8a73f910dddca1dee123)

💬 kevkevinpal commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r1609173422)
I've updated the function to not accept the `args` param since it did not seem to be needed and instead allowed an optional 3rd param as the message

updated in [4488120](https://github.com/bitcoin/bitcoin/pull/29500/commits/4488120ab7db775ce0ae8a73f910dddca1dee123)

💬 kevkevinpal commented on pull request "test: create assert_not_equal util":
(https://github.com/bitcoin/bitcoin/pull/29500#discussion_r1609177324)
fixed in [4358c6e](https://github.com/bitcoin/bitcoin/pull/29500/commits/4358c6e7221eed21d9cf78ead5cabd6982fb9a03)

thanks for the review!

📝 achow101 opened a pull request: "contrib: Renew Windows code signing certificate"
(https://github.com/bitcoin/bitcoin/pull/30149)
Renewed the Windows code signing certificate for another 3 years.

🤔 vasild reviewed a pull request: "build: Remove `--enable-threadlocal`"
(https://github.com/bitcoin/bitcoin/pull/30137#pullrequestreview-2070106741)
ACK 17fe948cce2eb75f0f3f4b0db9d0d90648c7d4af

💬 vasild commented on pull request "build: Remove `--enable-threadlocal`":
(https://github.com/bitcoin/bitcoin/pull/30137#discussion_r1609265987)
With this it will be possible to remove `export NOWARN_CXXFLAGS="-Wno-error=unreachable-code"` from dev environment on FreeBSD, thanks!

💬 vasild commented on pull request "init: fixes file descriptor accounting":
(https://github.com/bitcoin/bitcoin/pull/30065#issuecomment-2123884329)
> The current logic for file descriptor accounting is pretty convoluted and hard to follow.

Concept ACK, I stumbled on this recently in https://github.com/bitcoin/bitcoin/pull/29415

💬 BenWestgate commented on pull request "script: Fix errors in verify-binaries/verify.py OS platform parsing, update test.py & docs":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2123904192)
@DrahtBot: I am fairly sure this PR is NOT a consensus change. It's a bugfix to a download verification tool script. Replace that label with a more accurate one.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2123952687)
> Concept ACK, kinda works.
>
> Minor nit noticed - could avoid adding ":0" as a port for I2P addresses.

thanks for the suggestion, did this.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2123989036)
> Another thing - list is currently comma separated, without wrapping. Wouldn't it be too long if somebody has IPv4, IPv6, CJDNS, Tor and I2P addresses at the same time?

Thanks for this point, it is fixed and the widget worswraps and extends when needed.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1609378065)
Thanks. Did the flow as you said. Now we are querying layer by layer. Please comment if it still needs improvement. I decided to pass the `std::map<CNetAddr, LocalServiceInfo>` to keep it flexible enough for different use cases; for example in this specific usecase (showing IPs in GUI), i was able to do check for "IsI2P" on CNetAddrs.

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#discussion_r1609379939)
changed to a normal loop and checking for .begin().

💬 fjahr commented on pull request "rpc: Optimize serialization and enhance metadata of dumptxoutset output":
(https://github.com/bitcoin/bitcoin/pull/29612#discussion_r1609402573)
Sorry, I must have overlooked that if you already mentioned it before when I changed the type. I will address it if I have to retouch.

🚀 fanquake merged a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/30120)

💬 stratospher commented on pull request "test: improve robustness of connect_nodes()":
(https://github.com/bitcoin/bitcoin/pull/30118#issuecomment-2124125832)
reACK 6629d1d.

💬 naiyoma commented on pull request "cli: restrict multiple exclusive argument usage in bitcoin-cli":
(https://github.com/bitcoin/bitcoin/pull/30148#issuecomment-2124129963)

> This is very close to @Brotcrunsher's original commit. At the very least they should be a co-author.

I've added the co-author.

📝 maflcko opened a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150)
`script` is confusing, because in the context of Bitcoin, it usually means Bitcoin script (c.f. `CScript` in `script.h`, or pull requests such as https://github.com/bitcoin/bitcoin/pull/27122 using the prefix).

This could be fixed by renaming it to `scripts` (with a plural `s` at the end), however, looking at the current usage `contrib` and `cli` seem more common (https://github.com/bitcoin/bitcoin/pull/29687, https://github.com/bitcoin/bitcoin/pull/26953, https://github.com/bitcoin/bitcoin/p
...

💬 maflcko commented on pull request "script: Fix errors in verify-binaries/verify.py OS platform parsing, update test.py & docs":
(https://github.com/bitcoin/bitcoin/pull/30147#issuecomment-2124140010)
@BenWestgate I believe this is a typo in the documentation, fixed in https://github.com/bitcoin/bitcoin/pull/30150. (You can change the title yourself to `contrib:` and the bot will pick it up at some point.)

💬 jadijadi commented on pull request "Showing Local Addresses in Node Window":
(https://github.com/bitcoin-core/gui/pull/626#issuecomment-2124149228)
> This PR still have a couple of unaddressed comments: [#626 (comment)](https://github.com/bitcoin-core/gui/pull/626#discussion_r915334998) and [#626 (comment)](https://github.com/bitcoin-core/gui/pull/626#discussion_r1274077350).

Dear @hebasto , addressed all issues, updated the code & replied to queries. Thanks for the follow up.

👍 fanquake approved a pull request: "doc: Correct pull request prefix for scripts and tools"
(https://github.com/bitcoin/bitcoin/pull/30150#pullrequestreview-2070467640)
ACK fa3e1151a28345edff8f371283745bdd647f9a74