💬 ajtowns commented on pull request "Use serialization parameters for CTransaction":
(https://github.com/bitcoin/bitcoin/pull/28438#discussion_r1393590541)
Little bit of a rabbit hole to do that -- need to change `BufferedFile` to accept an `AutoFile` first etc. Doesn't seem worth adding here to me. See https://github.com/ajtowns/bitcoin/commits/202311-autofile

💬 seedhammer commented on issue "MuSig2 support":
(https://github.com/bitcoin/bitcoin/issues/23326#issuecomment-1811758875)
> > Is it possible to support descriptors that include only the "aggregated xpub" and not every key as in the proposed `agg()` expression, leading to a O(1) size instead of O(number-of-keys)?
>
> You should be able to use Shamir secret sharing between the private key data so that a spending threshold can recover all pubkey data needed to reconstruct the agg() expression.

As I understand it, the `agg()` expression requires all `n` public keys. I don't see how a threshold, `m < n`, can recov
...

💬 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-1811766792)
> My contribution to this bikeshedding would be:
> * `OP_n` -> `n` (so `-1`, `0`, `1`, ..., `10`, ..., `16`).
> * Drop "OP_" prefix for other opcodes too; they're redundant.
> * Drop the sighash byte type decoding. I don't feel it matches what asm is trying to do.

:+1:

> * For non-minimal pushes, use `PUSHDATA1<...>` or `PUSHDATA2<...>` (with the contents of `...` using the same rules as for minimal pushes).

Also `PUSHDATA4<...>` for completeness, presumably. A direct pus
...

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1393623761)
`tx1_conflict` also displaces `tx3` because `tx3` spends `tx1`, so it is also considered mempool-conflicted. So instead `tx1_conflict` and `tx2` are the only transactions left in the mempool. I've added a comment indicating this.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1393624485)
I agree that it gets a bit confusing, so I've added a lot more comments.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#issuecomment-1811798648)
I have addressed the remaining questions, the only changes are the addition of comments to the functional test.

💬 kcalvinalvin commented on issue "`libbitcoinconsensus.a` is unusable":
(https://github.com/bitcoin/bitcoin/issues/28779#issuecomment-1811854517)
> 2. Disable building static libraries in the current build system.

I think this affects https://github.com/rust-bitcoin/rust-bitcoinconsensus (rust wrapper around libbitcoinconsensus).

cc @tcharding @apoelstra

💬 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()`?