📝 MarcoFalke opened a pull request: "ci: Remove unused EXPECTED_TESTS_DURATION_IN_SECONDS env var"
(https://github.com/bitcoin/bitcoin/pull/27209)
Remove long unused travis leftover

👍 fanquake approved a pull request: "ci: Remove unused EXPECTED_TESTS_DURATION_IN_SECONDS env var"
(https://github.com/bitcoin/bitcoin/pull/27209)
ACK 3fffff50f625ed5e3c45139b3ae874f63f121a1e

💬 willcl-ark commented on pull request "util: Use steady clock in SeedStrengthen, FindBestImplementation, FlushStateToDisk":
(https://github.com/bitcoin/bitcoin/pull/27189#issuecomment-1456083159)
Concept ACK for moving to monotonic clocks.

Am I correct in my understanding that there are no current tests/benchmarks which are calling these while adjusting the system clock, but rather this is _additional_ hardening for a user whose system clock happens to be changed _while_ one of these is executing?

I couldn't easily find any callsites which might be directly affected by this, only indirectly. i.e. The system clock happens to change during a call to `GetRandHash()` or `GetRandBytes()
...

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1126384700)
well, this method is just a push function. It doesn't have to tell anything about the context where it is used. The struct is basically a map container with a cache, it doesn't need to explain how APS works.

If you are strong over the bool args usage, that would be the argument to make the changes for me.

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1126384935)
sounds good

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#discussion_r1126387031)
pushed, thanks

💬 furszy commented on pull request "wallet: group outputs only once, decouple it from Coin Selection":
(https://github.com/bitcoin/bitcoin/pull/25806#issuecomment-1456099290)
Updated per feedback, thanks S3RK 👌🏼.

[Small diff](https://github.com/bitcoin/bitcoin/compare/1cfed6ce6cd8c9f9f1e93662ff8f92cd5a5b91c4..6a302d40dfcc33b15bf27a1d3fcc04bfd3cbda44), changes are in the last test scenario and the test commit message.

💬 MarcoFalke commented on pull request "util: Use steady clock in SeedStrengthen, FindBestImplementation, FlushStateToDisk":
(https://github.com/bitcoin/bitcoin/pull/27189#issuecomment-1456125811)
> Am I correct in my understanding that there are no current tests/benchmarks which are calling these while adjusting the system clock, but rather this is additional hardening for a user whose system clock happens to be changed while one of these is executing?

yes, I don't think the system clock is ever adjusted in tests. Usually, we'd use `NodeClock` for a mockable clock. `faketime` may be a way to mock the system clock in tests, but faketime may not be available on Windows.

About the use
...

📝 vasild opened a pull request: "gui: use the stored CSubNet entry when unbanning"
(https://github.com/bitcoin-core/gui/pull/717)
The previous code visualized the `CSubNet` object as string, then parsed that string back to `CSubNet`. This is sub-optimal given that the original `CSubNet` object can be used directly instead.

This avoids calling `LookupSubNet()` from the GUI.

💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1456152963)
@mzumsande, it is possible to avoid calling `LookupSubNet()` from the GUI, see https://github.com/bitcoin-core/gui/pull/717. If that gets merged then this PR will be simplified - no need to touch the GUI code from here.

💬 vasild commented on pull request "gui: use the stored CSubNet entry when unbanning":
(https://github.com/bitcoin-core/gui/pull/717#issuecomment-1456154849)
This would simplify https://github.com/bitcoin/bitcoin/pull/27071.

💬 petertodd commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1456161328)
So the next step here for people who want to actually see this implemented is code, and perhaps more importantly, tests.

The code change is fairly simple: change the `datacarrierlimit` to default to max-int, and make the rule that checks if there is more than one OP_Return only take action if the limit isn't the default. I don't think there is any reason to implement a separate rule to check if there is more than one OP_Return if you aren't limiting the size.

Second, tests. You need to che
...

👍 dergoegge approved a pull request: "ci: Remove unused EXPECTED_TESTS_DURATION_IN_SECONDS env var"
(https://github.com/bitcoin/bitcoin/pull/27209)
ACK 3fffff50f625ed5e3c45139b3ae874f63f121a1e

🚀 glozow merged a pull request: "ci: Remove unused EXPECTED_TESTS_DURATION_IN_SECONDS env var"
(https://github.com/bitcoin/bitcoin/pull/27209)

💬 MarcoFalke commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1456213291)
Rebased. For review I suggest `--color-moved=dimmed-zebra`.

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#issuecomment-1456215491)
Yeah, the same happens on Lunar (gcc 12.2). Steps to reproduce on a fresh install:


```
export HOST=powerpc64le-linux-gnu && export MAKEJOBS="$(nproc)" && apt update && apt install git vim htop -y && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./bitcoin-core && cd bitcoin-core && apt install -y bzip2 make automake cmake curl g++-multilib libtool binutils bsdmainutils pkg-config python3 patch bison && apt install -y g++-arm-linux-gnueabihf binutils-arm-linux-gnueabihf
...

💬 Ayms commented on issue "Allow several OP_RETURN in one tx and no limited size":
(https://github.com/bitcoin/bitcoin/issues/27043#issuecomment-1456245569)
@petertodd then could someone please do it? (you?)

I am good with js/nodejs/browsers but can't pretend to be a C/C++ expert, even if the change is simple it would be a waste of time/not serious I believe that I issue a PR for this, and approximation can't work with Bitcoin code

@ChristopherA did open the debate, which did intersect some discussions that I started, that's why I opened this issue myself, you proposed a solution but at the end it just must be done

💬 MarcoFalke commented on pull request "Handle invalid hex encoding in ParseHex":
(https://github.com/bitcoin/bitcoin/pull/25227#issuecomment-1456248391)
It works on gcc-10 (debian buster), same steps to reproduce as above.

👍 instagibbs approved a pull request: "test: psbt: check non-witness UTXO removal for segwit v1 input"
(https://github.com/bitcoin/bitcoin/pull/27200)
ACK 3dd2f6461b4bb28b2b212c691a3df28ac793ad91

Documenting the expected behavior is good. I wonder if some signers could have trouble if they haven't implemented taproot support, and thus will expect non witness utxos?

Apparently this hasn't caused a problem yet, so maybe not.

💬 instagibbs commented on pull request "test: psbt: check non-witness UTXO removal for segwit v1 input":
(https://github.com/bitcoin/bitcoin/pull/27200#discussion_r1126520060)
feel like there should be a subroutine to do this for any given test