💬 hebasto commented on pull request "guix: Use LTO to build releases":
(https://github.com/bitcoin/bitcoin/pull/25391#issuecomment-1879669019)
Related:
- https://github.com/bitcoin/bitcoin/pull/29185

💬 furszy commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1443784908)
fixed

💬 furszy commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1443785461)
> In [ff21425](https://github.com/bitcoin/bitcoin/commit/ff214259de28216a5190c358b786306985f30227) "wallet: migration, remove watch-only transactions atomically"
>
> It seems unnecessary to be making this change here as `ZapSelectTx` is now atomic.

True. This PR does not requires it. Will re-introduced it within #28574.

💬 furszy commented on pull request "wallet: simplify and batch zap wallet txes process":
(https://github.com/bitcoin/bitcoin/pull/28987#discussion_r1443784954)
fixed

🤔 furszy reviewed a pull request: "wallet: simplify and batch zap wallet txes process"
(https://github.com/bitcoin/bitcoin/pull/28987#pullrequestreview-1807495476)
Updated per feedback. Thanks achow. [Diff](https://github.com/bitcoin/bitcoin/compare/ff214259de28216a5190c358b786306985f30227..d26a28508d5322fe7c29b3642dcf3a17b0ea31f1).

💬 vasild commented on pull request "Avoid returning references to mutex guarded members":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1879738697)
`d4ef700405...7a8e5310e7`: rebase as pointed about above by @jonatack. Thanks!

💬 vasild commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1879740549)
> I have applied the suggestions below in [`vasild/getnetmsgstats`](https://github.com/vasild/bitcoin/commits/getnetmsgstats) plus some other simplifications. Feel free to pick them.

Should I PR the above branch?

💬 kevkevinpal commented on pull request "test: randomize perturbed files excluding ldb":
(https://github.com/bitcoin/bitcoin/pull/29182#issuecomment-1879742334)
I tested this by creating a loop similar to how @maflcko found an issue in https://github.com/bitcoin/bitcoin/pull/28831

I also noticed that in this test we only perturbed one `blk?????.dat` file which is `blk00000.dat`
Should we add another .dat file to test this on?

💬 apoelstra commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1879746573)
Thanks for the ping. (a) agreed that rust-bitcoinconsensus can just do a "final" release and this wouldn't be a problem for us, and (b) if there were a blackbox script validator somewhere else, we'd happily use that. Especially if it were a more maintained/supported part of Core rather than something hanging off the side.

💬 kevkevinpal commented on pull request "tests: add functional test for miniscript decaying multisig":
(https://github.com/bitcoin/bitcoin/pull/29156#issuecomment-1879749727)
PR description link is routes to 404 can you update to use this https://github.com/bitcoin/bitcoin/blob/master/test/functional/wallet_multisig_descriptor_psbt.py instead?

💬 wizkid057 commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1879750836)
> Since CVE ID is used to validate this as a vulnerability

It's not, really. In fact, I didn't even mention "CVE" once in [my detailed comment above](https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1879268385).

This is clearly a bug, clearly a vulnerability in the software, clearly unintended behavior, and clearly being actively exploited in the wild. That _more_ than qualifies this particular issue as a valid CVE, but absolutely no one credible is doing the reverse of using
...

💬 hebasto commented on pull request "build: remove `--enable-lto`":
(https://github.com/bitcoin/bitcoin/pull/29185#issuecomment-1879764210)
Tested 2d1b1c7daeeada3f737e62ceb2db7484cde5ff4e on Ubuntu 22.04:

```
$ ./configure CXXFLAGS="-g -O2 -flto=auto" SECP_CFLAGS="-flto=auto"
$ make -C src bitcoind
```

```
$ make -C depends CXXFLAGS="-O2 -flto=auto" LTO=1
$ ./configure CONFIG_SITE=$PWD/depends/x86_64-pc-linux-gnu/share/config.site SECP_CFLAGS="-flto=auto"
$ make
```

> Ah, I guess this would disable O2, see https://github.com/bitcoin/bitcoin/pull/28071/files. So I guess it would have to be appended to `CXX` and possib
...

💬 kristapsk commented on pull request "rpc: add 'getnetmsgstats' RPC":
(https://github.com/bitcoin/bitcoin/pull/28926#issuecomment-1879765838)
Concept ACK

💬 josibake commented on pull request "wallet, rpc: `FundTransaction` refactor":
(https://github.com/bitcoin/bitcoin/pull/28560#issuecomment-1879780205)
`git range-diff 5892014...909c6bb`

Addresses review comments from @aureleoules and @achow101 , most notable being now clearing `tx.vout` and asserting that it is empty inside both `FundTransaction` functions. This ensures this is no longer used to pass outputs to `FundTransaction`, now and in future code.

💬 L0laL33tz commented on pull request "test: randomize perturbed files excluding ldb":
(https://github.com/bitcoin/bitcoin/pull/29182#discussion_r1443842409)
I tested this the same way @fjahr tested the size of the ldb files in #28831 with

```
self.log.info(f"Perturbing file to ensure failure {target_file}, size: {os.path.getsize(target_file)}")
```
The tested blockfile should always have 16777216 bytes

💬 dooglus commented on issue "new crash in v26.0":
(https://github.com/bitcoin/bitcoin/issues/29153#issuecomment-1879806191)
It happened again.

This node only connects to one other node. I run two nodes on the same machine. One connects to lots of peers, and this one that keeps crashing hosts my wallets, and only connects to the other node.

```
Thread 1 "bitcoin-qt-v26." received signal SIGSEGV, Segmentation fault.
0x00007ffff7a7c52d in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5
(gdb) where
#0 0x00007ffff7a7c52d in ?? () from /lib/x86_64-linux-gnu/libQt5Core.so.5
#1 0x00007ffff7a82b57 in ?? () from
...

💬 jonatack commented on pull request "net, cli: use v2transport for manual/addrfetch connections, add to -netinfo":
(https://github.com/bitcoin/bitcoin/pull/29058#discussion_r1443852237)
> @jonatack do you have an opinion?

Am catching up with merged pulls and discussions; this one is high on my list when I've caught up.

💬 GregTonoski commented on issue "[Bug]: Blockspace price shouldn't be higher for a simple transaction (price discrimination against simple txs)":
(https://github.com/bitcoin/bitcoin/issues/29146#issuecomment-1879825003)
> Yes it takes more bytes to spend a transaction than to split a transaction because input consumption includes signatures (witness data) and outputs typically contain only P2SH which is compact. So the discount balances those input consumption bytes and output creation bytes so there is no longer a financial incentive to create dust. To be sure if you run out of coins your wallet will use change, but until then it will keep splitting coins. Say you have 100 1BTC lumps in your wallet for privacy
...

💬 jonatack commented on pull request "sync: introduce a thread-safe generic container and use it to remove a bunch of "GlobalMutex"es":
(https://github.com/bitcoin/bitcoin/pull/25390#issuecomment-1879827022)
Will review the updates since my initial ACK https://github.com/bitcoin/bitcoin/pull/25390#pullrequestreview-1390434676.

💬 GregTonoski commented on issue "[Bug]: Blockspace price shouldn't be higher for a simple transaction (price discrimination against simple txs)":
(https://github.com/bitcoin/bitcoin/issues/29146#issuecomment-1879828846)
https://delvingbitcoin.org/t/bug-spammers-get-bitcoin-blockspace-at-discounted-price-lets-fix-it/327/4?u=gregtonoski