🚀 fanquake merged a pull request: "guix: Remove librt usage from release binaries"
(https://github.com/bitcoin/bitcoin/pull/28069)

💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1265311577)
Needs rebase

👍 0xB10C approved a pull request: "test: Disable known broken USDT test"
(https://github.com/bitcoin/bitcoin/pull/28088#pullrequestreview-1532737766)
ACK on disabling the test for now - I'll revisit #27380 when I have more bandwith it.

🤔 MarcoFalke reviewed a pull request: "Make poly1305 support incremental computation + modernize"
(https://github.com/bitcoin/bitcoin/pull/27993#pullrequestreview-1527628367)
Sure, happy to leave comments, but I think it may be better to fix them in a follow-up, unless you really want to here.

💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1262023316)
```suggestion
BOOST_CHECK_EQUAL(HexStr(total_tag), "64afe2e8d6ad7bbdd287f97c44623d39");
```

style nit, if you retouch.

💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1265337111)
nit: If you want to avoid the cast here, you could make `ConsumeRandomLengthByteVector` a template on `B` and then directly consume into a `std::vector<std::byte>` and use that.

💬 MarcoFalke commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#discussion_r1265338188)
```suggestion
std::vector<std::byte> msg(i, std::byte{uint8_t(i)});
```

(nit, just personal preference)

💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1265351007)
Guess that you are referring to the last two args?

I think that it isn't much of a problem because it is an internal static helper function.

They are needed for Knapsack min change target calculation. What maybe could do is to pass them as one: `mean_amount = std::floor(recipients_sum / recipients_size)` and document the function properly.

And also, in a follow-up, we could go further and move the else branch of the `scriptChange` block of code inside `ComputeChangeParams` too. That par
...

📝 MarcoFalke opened a pull request: "test: Bump walletpassphrase timeout to avoid intermittent issue"
(https://github.com/bitcoin/bitcoin/pull/28089)
There is no risk increasing the timeout, but it avoids intermittent issues (when running in valgrind) of the form:

```
node1 2023-07-15T06:17:37.827609Z [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:45950
node1 2023-07-15T06:17:37.833168Z [httpworker.3] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=signrawtransactionwithwallet user=__cookie__
test 2023-07-15T06:17:38.037000Z TestFramework (ERROR): JSONRPC error

...

📝 fanquake opened a pull request: "validation: use noexcept instead of deprecated throw()"
(https://github.com/bitcoin/bitcoin/pull/28090)
We fixed this once before in https://github.com/bitcoin/bitcoin/pull/10965.
Turn on https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-noexcept.html#modernize-use-noexcept.

💬 fanquake commented on pull request "test: Disable known broken USDT test":
(https://github.com/bitcoin/bitcoin/pull/28088#issuecomment-1638210191)
Seems fine to disable this for now.
Also cc @virtu.

🚀 fanquake merged a pull request: "test: Disable known broken USDT test"
(https://github.com/bitcoin/bitcoin/pull/28088)

💬 furszy commented on pull request "test: Bump walletpassphrase timeout to avoid intermittent issue":
(https://github.com/bitcoin/bitcoin/pull/28089#discussion_r1265432942)
This looks fishy. It seems that we unlock the wallet inside a specific test case, and use the unlocked period for other test cases.

A better approach would be to relock the wallet at the end of this test case ( `self.nodes[1].walletlock()`). And unlock-lock the wallet in the other cases when it is needed.

💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#discussion_r1265436928)
Ok I've used a `GlobalMutex` for this now. I never Recursive behaviour but was getting clang static analysis errors without. Looking at _sync.h_ it seems that `GlobalMutex` should be the correct type for this I think.

💬 MarcoFalke commented on pull request "test: Bump walletpassphrase timeout to avoid intermittent issue":
(https://github.com/bitcoin/bitcoin/pull/28089#discussion_r1265441187)
Sounds good. An alternative might be to not use a passphrase after this test. Do you want to submit either as a pull request? Happy to close mine then. Otherwise, I'll leave the feedback for a follow-up, as the goal here is to not change any logic, only the constant.

💬 willcl-ark commented on pull request "net: disconnect inside AttemptToEvictConnection":
(https://github.com/bitcoin/bitcoin/pull/27912#discussion_r1265441313)
`DeleteNode()` now gated behind `if (pnode->nRefCount.compare_exchange_strong(expectedRefCount, -1))` so it can only be called once (by a single thread).

💬 MarcoFalke commented on pull request "validation: use noexcept instead of deprecated throw()":
(https://github.com/bitcoin/bitcoin/pull/28090#issuecomment-1638254580)
lgtm ACK 047daad4f59942488163c6be8516a69291646294

📝 brunoerg opened a pull request: "fuzz: use `ConnmanTestMsg` in `connman`"
(https://github.com/bitcoin/bitcoin/pull/28091)
Fixes #27980

Using `ConnmanTestMsg` we can add nodes and be
more effective fuzzing functions like `DisconnectNode`,
`FindNode`, `GetNodeStats` and other ones.

📝 fanquake opened a pull request: "build: document that -Wreturn-type has been fixed upstream (mingw-w64)"
(https://github.com/bitcoin/bitcoin/pull/28092)
`noreturn` attributes have been added to the mingw-w64 headers, https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe, meaning that [from 11.0.0 onwards](https://www.mingw-w64.org/changelog/), you'll no-longer see `-Wreturn-type` warnings when using `assert(false)`.

At the same time, move this out of the `-Werror` check, it's not clear why it was in there, rather than being applied/or-not along with all other cxxflags. A somewhat similar change was proposed in
...

💬 fanquake commented on pull request "Add `UNREACHABLE` macro":
(https://github.com/bitcoin/bitcoin/pull/26504#issuecomment-1638262812)
> but I think having a macro for those that find it helpful, can't hurt, can it?

I agree that having a macro could be useful.

Separately, note that upstream, the missing `noreturn` attributes have been added to `assert` in the mingw-w64 headers, https://github.com/mingw-w64/mingw-w64/commit/1690994f515910a31b9fb7c7bd3a52d4ba987abe, so beginning with [11.0.0](https://www.mingw-w64.org/changelog/), there are no-longer `-Wreturn-type` warnings with `assert(false)` usage. See #28092 where I've
...