💬 jamesob commented on pull request "rpc: Fix race in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/29262#issuecomment-1896156373)
Approach ACK, thanks.

💬 TheCharlatan commented on pull request "build: always set `-g -O2` in `CORE_CXXFLAGS`":
(https://github.com/bitcoin/bitcoin/pull/29205#issuecomment-1896171001)
Guix builds:
```
c65c40452ea0c2485c8a17a4d79f4befd2f3f5b66af5281a7833842990a236be guix-build-00c1e2aa4496/output/aarch64-linux-gnu/SHA256SUMS.part
c027a5f50f2b767a251953eded0cef07adf932f4905e5bd6cb1b19fd013a9f8d guix-build-00c1e2aa4496/output/aarch64-linux-gnu/bitcoin-00c1e2aa4496-aarch64-linux-gnu-debug.tar.gz
2baa2e82b8af6171b7861e6487fb3c1a07f0b0637e1518fb047d9f0773922781 guix-build-00c1e2aa4496/output/aarch64-linux-gnu/bitcoin-00c1e2aa4496-aarch64-linux-gnu.tar.gz
7452fa6211244d19607
...

💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1896179668)
5755454fb5cc4067fc94e2682116e0fc5c9dfc58 -> 487b12e18ce007379a997da48b5ee97f459f6342 ([noGlobalSignals_2](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_2) -> [noGlobalSignals_3](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_2..noGlobalSignals_3))

* Addressed @maflcko's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1455548750), adding `Assert` to guard against early
...

👍 vasild approved a pull request: "fuzz: make FuzzedDataProvider usage deterministic"
(https://github.com/bitcoin/bitcoin/pull/29043#pullrequestreview-1827705262)
ACK 01960c53c7d71c70792abe19413315768dc2275a

I think it is worth improving this even if currently we only fuzz with clang and even if clang has not changed orders before or due to compilation flags - this may happen in the future.

A clang-tidy plugin to enforce this would be nice but I do not see its absence as a blocker for this PR.

💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456045072)
Unrelated to this PR, out of scope, but passing `max_pct` greater than `100` is pointless. `AddrManImpl::GetAddr_()` will behave as if `100` is passed. In addition to being useless and confusing this skews the input towards `100`. That is, it will treat `> 97%` of the possibilities as `100` (`(4097-100)/4097*100 = 97.6%`).

💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456013304)
nit: all these variables can be `const`

💬 vasild commented on pull request "fuzz: make FuzzedDataProvider usage deterministic":
(https://github.com/bitcoin/bitcoin/pull/29043#discussion_r1456051965)
ditto and even worse because the range here is `0..2^64-1`

💬 joeyvee1986 commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896257073)
I'm all for this. The spam and constant duplicates have been slowing my
network down tremendously.

On Wed, Jan 17, 2024, 10:08 AM John Tromp ***@***.***> wrote:

> they put text (link) in many cases
>
> Yes, some do. But many others don't, and those are the ones these measures
> are targetting.
>
> —
> Reply to this email directly, view it on GitHub
> <https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896118061>,
> or unsubscribe
> <https://github.com/notifications/uns
...

👍 TheCharlatan approved a pull request: "test: Remove all-lint.py script"
(https://github.com/bitcoin/bitcoin/pull/29228#pullrequestreview-1827843026)
ACK fa2b95cf3f5148d27a8fd4fb3763ca1fc139bdd9

💬 pablomartin4btc commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#issuecomment-1896372688)
Updates:
- Switch the logic strategy to the previously presented alternative due to preference of reviewers @kevkevinpal & @furszy (thanks!);
- Added test coverage

💬 totient commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1896400042)
> No matter what new restrictions are imposed, it's safe to assume that the economic reality of people wanting to inscribe data on the blockchain will persist and use whatever means is available and cheapest. There will always be means available such as spreading the data over multiple outputs or multiple txs, encoding the data as arbitrary taproot scripts, or encoding the data as fake public keys or key hashes. Some of these encodings will be detectable while others will not. Raising the cost o
...

🤔 pablomartin4btc reviewed a pull request: "rpc: Fix race in loadtxoutset"
(https://github.com/bitcoin/bitcoin/pull/29262#pullrequestreview-1827979729)
ACK 5555d8db3313f893609eb0cf549bb597361d4466

✅ theuni closed a pull request: "Replace non-standard CLZ builtins with c++20's bit_width"
(https://github.com/bitcoin/bitcoin/pull/29057)

💬 theuni commented on pull request "Replace non-standard CLZ builtins with c++20's bit_width":
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1896546558)
@maflcko I'm thinking we should just take the c++20 code and assume it will be more performant going forward. That makes sense to me anyway.

I'm going to go ahead and close this and include it in another PR which gathers the serialization c++20 changes. We can do another round of benches there.

📝 theuni opened a pull request: "serialization: c++20 endian/byteswap/clz modernization"
(https://github.com/bitcoin/bitcoin/pull/29263)
This replaces #28674, #29036, and #29057. Now ready for testing and review.

Replaces platform-specific endian and byteswap functions. This is especially useful for kernel, as it means that our deep serialization code no longer requires bitcoin-config.h.

I apologize for the size of the last commit, but it's hard to avoid making those changes at once.

All platforms now use our internal functions rather than libc or platform-specific ones, with the exception of MSVC.

Sadly, benchmarking
...

✅ fanquake closed a pull request: "[POC] C++20 `std::endian`"
(https://github.com/bitcoin/bitcoin/pull/28674)

💬 theuni commented on pull request "Add missing byteswap functions for MSVC":
(https://github.com/bitcoin/bitcoin/pull/29036#issuecomment-1896567110)
Obsoleted by #29263.

✅ fanquake closed a pull request: "Add missing byteswap functions for MSVC"
(https://github.com/bitcoin/bitcoin/pull/29036)

💬 furszy commented on pull request "assumeutxo, rpc: Improve EOF error when reading snapshot metadata in loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28670#discussion_r1456421159)
I don't to see how this part of the message would be helpful to users. "file size being too small" does not tell me anything. Would be better to say that the file is corrupt and/or was crafted in an incompatible format. Also, no need to add the internal exception message to the user facing error.

📝 instagibbs opened a pull request: "Add max_tx_weight to transaction funding options"
(https://github.com/bitcoin/bitcoin/pull/29264)
This allows a transaction's weight to be bound under
a certain weight if possible an desired. This
can be beneficial for future RBF attempts, or
whenever a more restricted spend topology is
desired.

See https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418/11?u=instagibbs for more discussion.

Seeking concept ACK and maybe some help on approach for testing.