🤔 tdb3 reviewed a pull request: "http: set TCP_NODELAY when creating HTTP server"
(https://github.com/bitcoin/bitcoin/pull/30675#pullrequestreview-2246864976)
Concept ACK
Good find! Left just a nit for now, but will return to review in more detail (and possibly run some tests for different size transfers/blocks)

💬 tdb3 commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1722614782)
nit: could use `n_one` to better align with style guidelines.

https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#coding-style-c
> Variable (including function arguments) and namespace names are all lowercase and may use _ to separate words (snake_case).

💬 sipa commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1722617576)
Following the style guide it'd just be called `one`. The `n` prefix is a Hungarian notation prefix for "number", which we no longer use.

💬 romanz commented on pull request "http: set TCP_NODELAY when creating HTTP server":
(https://github.com/bitcoin/bitcoin/pull/30675#discussion_r1722690617)
Thanks! Fixed to `one` in c1bec82697.

💬 paplorinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1722716550)
I don't have write access to yiur PR:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/about-pull-request-reviews#resolving-conversations

💬 paplorinc commented on pull request "coins: remove logic for spent-and-FRESH cache entries and writing non-DIRTY entries":
(https://github.com/bitcoin/bitcoin/pull/30673#discussion_r1722725094)
If we need to document the behavior of a method, it suggests that we’ve acknowledged it may not be intuitive for everyone, indicating that we’ve conceded on finding a better design.
I think there's a way we can avoid the comment: by separating the two concerns of getting a value from cache and checking if it's spent or not.
Or if you insist, we can call it "GetUnspent" in which case we can also get rid of the comment and call-site asserts and keep the current behavior.
What do you think?

💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#discussion_r1722725975)
Thanks, added back

💬 maflcko commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-2298040795)
rfm with 5 acks on the first commit (2 stale and 3 current), and 3 acks on the second commit?

💬 hodlinator commented on pull request "node: reduce unsafe uint256S usage":
(https://github.com/bitcoin/bitcoin/pull/30569#issuecomment-2298062851)
Would be great to get the final issues resolved. Possible way forward, preserving backwards compatibility:

<details>
<summary>
diff
</summary>

```diff
diff --git a/src/node/chainstatemanager_args.cpp b/src/node/chainstatemanager_args.cpp
index ff5a7ebd00..e83b5f38b6 100644
--- a/src/node/chainstatemanager_args.cpp
+++ b/src/node/chainstatemanager_args.cpp
@@ -33,7 +33,7 @@ util::Result<void> ApplyArgsManOptions(const ArgsManager& args, ChainstateManage

if (auto value{args.
...

⚠️ MummaStacks opened an issue: "Running Bitcoin Bitcore on new Apple M3"
(https://github.com/bitcoin/bitcoin/issues/30680)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

“Bitcoin Core” can’t be opened because Apple cannot check it for malicious software. This software needs to be updated. Contact the developer for more information."

### Expected behaviour

upload lates version of the application

### Steps to reproduce

wont allow me to install application

### Relevant log output

_No response_

### How did you obtain Bitcoin Core

Compiled from source


...

💬 maflcko commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#issuecomment-2298084825)
Edited pull description and title to remove the mention of the global, now that it is removed in the last commit

💬 maflcko commented on pull request "refactor: Remove Span operator==, Use std::ranges::equal":
(https://github.com/bitcoin/bitcoin/pull/29071#issuecomment-2298086275)
Is this rfm with three acks, or does it need more, or is it waiting for after the branch off?

💬 maflcko commented on pull request "MiniWallet funcitonality for more readable code in functional test `rpc_signtransactionwithkey.py`":
(https://github.com/bitcoin/bitcoin/pull/30667#issuecomment-2298095817)
You'll have to run the test locally, after you modify it and before you push the code to a pull request. There is a contributing guide for developers and how to run the tests.

👍 TheCharlatan approved a pull request: "fuzz: Faster utxo_snapshot fuzz target"
(https://github.com/bitcoin/bitcoin/pull/30644#pullrequestreview-2247125663)
Re-ACK fa899fb7aa8a14acecadd8936ad5824fa0f697ff

💬 maflcko commented on issue "Unable to build Bitcoin using './contrib/guix/guix-build'":
(https://github.com/bitcoin/bitcoin/issues/30676#issuecomment-2298116470)
There is a packaging error fixed in https://salsa.debian.org/debian/guix/-/commit/b27fcc3fa6d3633eb2beceb014507045f534f2d0

You can ask for it to be backported upstream, or work around it manually for now via `apt install netbase`

💬 maflcko commented on issue "Running Bitcoin Bitcore on new Apple M3":
(https://github.com/bitcoin/bitcoin/issues/30680#issuecomment-2298119594)
This may be a duplicate of #15774 ?

💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#issuecomment-2298177522)
Apologies for another push.

Noticed that some of the commit messages were incorrect/imprecise:
"Only touching functions that will be modified in upcoming 2 commits." -> "Only touching functions that will be modified in next commit."
(A prior local version had code fixups happening after the de-Hungarianization and vector->span, which wasn't as smooth).
"Lines will be touched in upcoming commits within this PR." -> "Lines will be touched in next 2 commits."

Content change: The de-Hungari
...

💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722846451)
style nit in 8ad548f389f6abd16db39dadafe3fbeefaedec3a: (up to you) If you want, you can add the missing `{}` after all `if`, according to the dev notes, in this commit. Feel free to ignore.

💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722843198)
> Content change: The de-Hungarianization commit was adding a space after `if`, which now happens in the commit before.

style nit in 08a880de6c94bc84c1f43a3845e1645d3eb67607: Not sure about adding missing space to lines in one commit and then modifying the lines again in a rename commit. I think adding "missing" space can be done in the rename commit. (Same for the other clang-format changes in "unrelated lines" in commit 08a880de6c94bc84c1f43a3845e1645d3eb67607) (But just a style nit)

💬 maflcko commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1722852311)
style nit in 2c68649f0932dac146b941fb6b4e2fbc52fbd8a0: Not sure about switching string/string_views to a raw C-array in C++ code when there is no reason for it. Modern compilers can optimize away sting/string_view sizes on literals, so my preference would be to not change the type here.

Example:

```cpp
int main() {
std::string sv{"04678afdb0"};
return sv.size();
}
```

Optimized into:

```cpp
int main() {
return 10;
}
```

on modern compilers.