💬 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
...
(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: )
(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.
(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
(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
(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.
(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
(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).
(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!
(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?
(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?
(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.
(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?
(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
...
(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
...
(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
(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.
(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
(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
...
(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.
(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.