💬 glozow commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1879631591)
Hard to guess without logs (which would tell us why transactions failed). How much time elapsed between saving the mempool.dat and loading it? Mempool transactions that had confirmed during that time would be marked as "failed."

And to clarify, are you restarting with the same config? What are mempoolexpiry and maxmempool now? (Is it possible you have less space or expired old transactions?)

> Not sure if the problem was writing the txs into mempool.dat or reading them.

Must be loading
...

💬 dev7ba commented on issue "Failed loading mempool when restarting bitcoind":
(https://github.com/bitcoin/bitcoin/issues/29193#issuecomment-1879634958)
> How much time elapsed between saving the mempool.dat and loading it?
Only a few minutes.

> And to clarify, are you restarting with the same config? What are mempoolexpiry and maxmempool now?

I restarted with the same config. Checked I have enough disk space.

> Must be loading given the "X failed" log line.
Yes, I see.
> Maybe try upgrading to a newer version?
I can't test it anyway, I'm trying to contact people with a "full mempool", And asking them to restart the node and see wha
...

💬 maflcko commented on pull request "Weaken serfloat tests":
(https://github.com/bitcoin/bitcoin/pull/29192#discussion_r1443727513)
(Just a nit, no need to change anything, because `is_iec559` is already asserted globally, so the two other asserts can also happen globally. In any case, this will "fix itself" with C++23 at some far point in the future :sweat_smile: )

💬 hebasto commented on pull request "build: Fix LTO functionality":
(https://github.com/bitcoin/bitcoin/pull/28876#issuecomment-1879659386)
FWIW, the https://github.com/bitcoin/bitcoin/pull/29185 makes this PR outdated.

💬 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
...