💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1393801774)
sounds much better! added a [new commit](https://github.com/bitcoin/bitcoin/pull/24748/commits/7f0e797dc9524c03f650f60a028cb46852877284).

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1811980868)
> I've noticed that the test p2p_v2_encrypted.py fails if the node under test sends a small amount of garbage (in the range of 0-19 bytes).

thanks for catching that @theStack! before the drop garbage authentication packet change in #28525, `recvbuf` needed 16+20+20 bytes to authenticate the handshake. now it only needs 16+20 bytes. i've fixed it now.

💬 maflcko commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1393811001)
This will also go away before the next release, see https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1393533653

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1812003580)
> https://api.cirrus-ci.com/v1/task/6171895557521408/logs/ci.log

i wasn't able to reproduce this error - tried it on the docker container, setting randomseed and running it in loop. also confused how it could have happened.

this is what happens inside `p2p_v2_earlykeyresponse.py` test
- we start an inbound connection to the TestNode (`TestNode` <---- `P2PInterface`).
- v2 connection starts by sending 64 bytes ellswift (`TestNode` <--64 bytes-- `P2PInterface`). this is sent in 2 parts:
...

💬 ajtowns commented on issue "new RPC: sendrawtransactiontopeer":
(https://github.com/bitcoin/bitcoin/issues/28636#issuecomment-1812032062)
In the meantime, you could presumably do `bitcoin-cli sendmsgtopeer 0 "tx" "$txhex"`

💬 hebasto commented on pull request "build: Fix LTO functionality":
(https://github.com/bitcoin/bitcoin/pull/28876#issuecomment-1812038153)
The other point of this PR is to make the current build system comparable to a new CMake-based one, as there is no reason to pessimize the latter, for example, by not applying LTO for libsecp code, for the sake of compatibility with the master branch.

See: https://github.com/hebasto/bitcoin/pull/52.

💬 MatthewLM commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1812093815)
> Provided you've already dealt with non-minimally encoded -1 and 1..16, using decimals seems nicer here? You can cope with negatives cleanly (`-2` vs `<82>`), and it's one less encoding variation?

I was thinking it would be nice to retain decimal but, due to ambiguity that could arise with CLTV and CSV, I'm not sure.

Having embedded ASM for redeem scripts and Tapscripts would be very useful.

⚠️ janus opened an issue: "Clean up some code inside interpreter.cpp"
(https://github.com/bitcoin/bitcoin/issues/28879)
### Please describe the feature you'd like to see added.

The below code would run a bit faster.

```
// Drop the signature in pre-segwit scripts but not segwit scripts
if (sigversion == SigVersion::BASE) {
for (int k = 0; k < nSigsCount; k++)
{
valtype& vchSig = stacktop(-isig-k);
int found = FindAndDelete(scriptCode, CScript() << vchSig);

...

💬 fanquake commented on pull request "build: Fix LTO functionality":
(https://github.com/bitcoin/bitcoin/pull/28876#issuecomment-1812120275)
> See: https://github.com/hebasto/bitcoin/pull/52.

I think the example usage there:
> cmake -B build -DLTO=ON -DLTO_FLAGS="-flto"

is somewhat helping make my point. Seems odd to have a `LTO=ON` option, but then still have to explicitly pass `-flto` into a non-standard `LTO_FLAGS` variable. Shouldn't that be what `LTO=ON` would do for you? I'm not sure how that is better/clearer than just `cmake -B build CXXFLAGS="-flto"`?

It's also not clear if `LTO_FLAGS` is a catch-all for compile an
...

✅ maflcko closed an issue: "Clean up some code inside interpreter.cpp"
(https://github.com/bitcoin/bitcoin/issues/28879)

💬 maflcko commented on issue "Clean up some code inside interpreter.cpp":
(https://github.com/bitcoin/bitcoin/issues/28879#issuecomment-1812132295)
You'll have to provide benchmarks, if you claim that this speeds up something. Keep in mind that compilers can produce optimized executables, when passed optimization flags, without having to change any code at all.

🚀 fanquake merged a pull request: "test: migrate to some per-symbol ubsan suppressions"
(https://github.com/bitcoin/bitcoin/pull/28865)

💬 vasild commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#discussion_r1393965320)
Dunno, maybe `ConnectNodeAndInitialize()`?

💬 fanquake commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-1812161592)
> Any idea where in6addr_any is coming from? Seems fine, just curious.

I can't remember exactly, but it looks like this is actually no-longer required. Did a new round of builds with this branch, and it dropped from the script.

🤔 BrandonOdiwuor reviewed a pull request: "wallet: propagete `checkChainLimits` error message to wallet"
(https://github.com/bitcoin/bitcoin/pull/28863#pullrequestreview-1731695630)
Concept ACK using the error message from `checkChainLimits(...)`

💬 hebasto commented on pull request "build: Windows SSP roundup":
(https://github.com/bitcoin/bitcoin/pull/28461#issuecomment-1812181560)
My Guix builds:
```
x86_64
c446041204b8b099c0b3e91f34bb783d11f57077ec685a141a766a9c44b917e0 guix-build-f95af98128f1/output/dist-archive/bitcoin-f95af98128f1.tar.gz
f929fd24af143247ca32a747ec29a7982b808c87d2221a47d257760efa1a216c guix-build-f95af98128f1/output/x86_64-w64-mingw32/SHA256SUMS.part
4287bb5bdc1ef7253fec176cd18ad1459dcb49705cc6b925aea41694fd5e390a guix-build-f95af98128f1/output/x86_64-w64-mingw32/bitcoin-f95af98128f1-win64-debug.zip
6109618130b5e240739da023a94d1b93ed05e619cee0
...

💬 ajtowns commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#issuecomment-1812212359)
> I was thinking it would be nice to retain decimal but, due to ambiguity that could arise with CLTV and CSV, I'm not sure.

What ambiguity?

📝 fanquake opened a pull request: "build: switch to using LLVM 17.x for macOS builds"
(https://github.com/bitcoin/bitcoin/pull/28880)
This is the last step before #21778. We need LLVM 17.x so that lld has `-fixup_chains`. I did want to just get everything merged in that PR, without a bump, but I've split this out because that PR currently has an issue with Qt and lld, and, as a side-effect, this PR also unblocks #28622 (C++20), so this is similar to #28732 (cherry-picked the commit), but with the required Guix change, now that LLVM 17 is available there (#28580). If this is merged it would also close #28851, which is the Qt w
...

💬 naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay)":
(https://github.com/bitcoin/bitcoin/pull/28765#discussion_r1394020665)
This happens before the addition. Your suggestion would mean that the size could be 3001, which is not desired.

💬 fanquake commented on pull request "build: use macOS 14 SDK (Xcode 15.0)":
(https://github.com/bitcoin/bitcoin/pull/28622#issuecomment-1812255751)
> The PR description looks outdated after the recent push :)

Updated the description, and rebased on #28880 instead, now that we can do that.