💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293358037)
I just re-tried `FUZZ=utxo_snapshot_invalid ./fuzz_exe_fa0b66400c12d11579b38b1ac76f677b23e8c82a -seed=1337 -runs=1000000` and it took less time than to write the second-last comment, and as much time as `FUZZ=utxo_snapshot ./fuzz_exe_389cf32aca0be6c4b01151c92cc3be79c120f197 -seed=1337 -runs=50000`.
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293358037)
I just re-tried `FUZZ=utxo_snapshot_invalid ./fuzz_exe_fa0b66400c12d11579b38b1ac76f677b23e8c82a -seed=1337 -runs=1000000` and it took less time than to write the second-last comment, and as much time as `FUZZ=utxo_snapshot ./fuzz_exe_389cf32aca0be6c4b01151c92cc3be79c120f197 -seed=1337 -runs=50000`.
💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293359132)
I'll recheck my setup and get back to you.
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293359132)
I'll recheck my setup and get back to you.
💬 fjahr commented on pull request "kernel: pre-28.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1719736632)
I raise you 17G! (height 2873788)
```
$ du -sh /Volumes/PortableSSD/datadir/testnet3/chainstate/ /Volumes/PortableSSD/datadir/testnet3/blocks/
17G /Volumes/PortableSSD/datadir/testnet3/chainstate/
82G /Volumes/PortableSSD/datadir/testnet3/blocks/
```
I know LevelDB can cause some differences in size on disk but this seems extreme...
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1719736632)
I raise you 17G! (height 2873788)
```
$ du -sh /Volumes/PortableSSD/datadir/testnet3/chainstate/ /Volumes/PortableSSD/datadir/testnet3/blocks/
17G /Volumes/PortableSSD/datadir/testnet3/chainstate/
82G /Volumes/PortableSSD/datadir/testnet3/blocks/
```
I know LevelDB can cause some differences in size on disk but this seems extreme...
💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719742146)
Yes, #29369 looks like a possible fix. Applied @ryanofsky's workaround for now but will have a look at that PR.
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719742146)
Yes, #29369 looks like a possible fix. Applied @ryanofsky's workaround for now but will have a look at that PR.
💬 fjahr commented on pull request "kernel: pre-28.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1719750035)
Either way, `m_assumed_blockchain_size` can be updated here too.
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1719750035)
Either way, `m_assumed_blockchain_size` can be updated here too.
💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719753108)
Ah, hadn't seen your #25227 before. It seems easier to default to `std::byte` when introducing `TryParseHex` without any prior callers. But thanks to your elaboration on the effort already under way to use `std::byte`, I did some more grepping and decided it might be worth attempting. Changed defaults in latest push 01e18d94d9577f415748869376988e3f0f59ced0 and worked through the fallout. The scripted-diff ended up with a lot less work in that version.
(The push right before it in 3a96b9f4b56c
...
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719753108)
Ah, hadn't seen your #25227 before. It seems easier to default to `std::byte` when introducing `TryParseHex` without any prior callers. But thanks to your elaboration on the effort already under way to use `std::byte`, I did some more grepping and decided it might be worth attempting. Changed defaults in latest push 01e18d94d9577f415748869376988e3f0f59ced0 and worked through the fallout. The scripted-diff ended up with a lot less work in that version.
(The push right before it in 3a96b9f4b56c
...
💬 RandyMcMillan commented on pull request "Hide peers details":
(https://github.com/bitcoin-core/gui/pull/505#issuecomment-2293390861)
thanks!
(https://github.com/bitcoin-core/gui/pull/505#issuecomment-2293390861)
thanks!
💬 hodlinator commented on pull request "refactor: Replace ParseHex with consteval HexLiteral":
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719761131)
Continued in https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1718988155
Resolving this one for now.
(https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719761131)
Continued in https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1718988155
Resolving this one for now.
💬 fjahr commented on pull request "kernel: pre-28.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/30658#issuecomment-2293446211)
ACK cd913de6488d25057e3bf7dcad1508e9d689f87f
Aside from the size-on-disk weirdness discussed above, I have compared all the values to my local nodes in the different networks and got matches each time. I also executed `contrib/devtools/headerssync-params.py` locally and got the same results.
(https://github.com/bitcoin/bitcoin/pull/30658#issuecomment-2293446211)
ACK cd913de6488d25057e3bf7dcad1508e9d689f87f
Aside from the size-on-disk weirdness discussed above, I have compared all the values to my local nodes in the different networks and got matches each time. I also executed `contrib/devtools/headerssync-params.py` locally and got the same results.
👍 TheCharlatan approved a pull request: "fuzz: Faster utxo_snapshot fuzz target"
(https://github.com/bitcoin/bitcoin/pull/30644#pullrequestreview-2242642075)
Re-ACK fa0b66400c12d11579b38b1ac76f677b23e8c82a
I saw the old "libFuzzer disabled leak detection" info message in the invalid test, but not sure what might be causing it.
(https://github.com/bitcoin/bitcoin/pull/30644#pullrequestreview-2242642075)
Re-ACK fa0b66400c12d11579b38b1ac76f677b23e8c82a
I saw the old "libFuzzer disabled leak detection" info message in the invalid test, but not sure what might be causing it.
💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293464039)
> I saw the old "libFuzzer disabled leak detection" info message in the invalid test, but not sure what might be causing it.
I haven't seen that one. Are you saying it only happens for this test? What are the steps to reproduce?
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293464039)
> I saw the old "libFuzzer disabled leak detection" info message in the invalid test, but not sure what might be causing it.
I haven't seen that one. Are you saying it only happens for this test? What are the steps to reproduce?
💬 sipa commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1719813387)
The file is from May 2022. No reason to keep using it; there is no inherent reason why it should be considered more reliable than yours.
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1719813387)
The file is from May 2022. No reason to keep using it; there is no inherent reason why it should be considered more reliable than yours.
💬 TheCharlatan commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293495649)
> I haven't seen that one. Are you saying it only happens for this test? What are the steps to reproduce?
Only saw it for the invalid test after about 2 million iterations just running `FUZZ=utxo_snapshot_invalid ./src/test/fuzz/fuzz`, configured with `address,fuzzer` (though I guess that should not matter).
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293495649)
> I haven't seen that one. Are you saying it only happens for this test? What are the steps to reproduce?
Only saw it for the invalid test after about 2 million iterations just running `FUZZ=utxo_snapshot_invalid ./src/test/fuzz/fuzz`, configured with `address,fuzzer` (though I guess that should not matter).
💬 glozow commented on pull request "cluster mempool: optimized candidate search":
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2293497129)
fwiw, my bench results (I commented out `assert(iters_performed == max_iters)` since it fails)
`git rebase -i --exec="make -j12 > /dev/null && src/bench/bench_bitcoin -filter=Linearize.*" HEAD~11`
Output: [bench_30286_all.txt](https://github.com/user-attachments/files/16637507/bench_30286_all.txt)
Reformatted as a csv: [output_benchmarks_ns_op.csv](https://github.com/user-attachments/files/16637477/output_benchmarks_ns_op.csv)
Displayed as a line graph (x-axis is commit, y-axis is ns
...
(https://github.com/bitcoin/bitcoin/pull/30286#issuecomment-2293497129)
fwiw, my bench results (I commented out `assert(iters_performed == max_iters)` since it fails)
`git rebase -i --exec="make -j12 > /dev/null && src/bench/bench_bitcoin -filter=Linearize.*" HEAD~11`
Output: [bench_30286_all.txt](https://github.com/user-attachments/files/16637507/bench_30286_all.txt)
Reformatted as a csv: [output_benchmarks_ns_op.csv](https://github.com/user-attachments/files/16637477/output_benchmarks_ns_op.csv)
Displayed as a line graph (x-axis is commit, y-axis is ns
...
👍 hodlinator approved a pull request: "refactor: Allow CScript construction from any std::input_iterator"
(https://github.com/bitcoin/bitcoin/pull/29369#pullrequestreview-2242723589)
ACK fa7b9b99a2ed466d1765d748f4a59c2f581b974f
Verified that it resolved the issue I was having on other PR (https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719107583) through cherry-picking the commits in this PR, undoing the local workaround, and pushing to a personal GitHub branch 239709e194203e06bd1f0610059e00998f645d4c to see if Windows CI compiled it successfully, which it did (https://github.com/hodlinator/bitcoin/actions/runs/10419824929/job/28858657327).
nit: I'd prefer t
...
(https://github.com/bitcoin/bitcoin/pull/29369#pullrequestreview-2242723589)
ACK fa7b9b99a2ed466d1765d748f4a59c2f581b974f
Verified that it resolved the issue I was having on other PR (https://github.com/bitcoin/bitcoin/pull/30377#discussion_r1719107583) through cherry-picking the commits in this PR, undoing the local workaround, and pushing to a personal GitHub branch 239709e194203e06bd1f0610059e00998f645d4c to see if Windows CI compiled it successfully, which it did (https://github.com/hodlinator/bitcoin/actions/runs/10419824929/job/28858657327).
nit: I'd prefer t
...
📝 maflcko opened a pull request: "test: Enable detect_leaks=1 in ASAN_OPTIONS explicitly"
(https://github.com/bitcoin/bitcoin/pull/30665)
It should be enabled by default, but being explicit can't hurt.
(https://github.com/bitcoin/bitcoin/pull/30665)
It should be enabled by default, but being explicit can't hurt.
💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293522814)
Lsan should be enabled by default, so if there was a problem, it should still be found. I've enabled it explicitly in https://github.com/bitcoin/bitcoin/pull/30665, but that should be unrelated to the changes here.
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293522814)
Lsan should be enabled by default, so if there was a problem, it should still be found. I've enabled it explicitly in https://github.com/bitcoin/bitcoin/pull/30665, but that should be unrelated to the changes here.
💬 0xB10C commented on pull request "kernel: pre-28.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1719854952)
```
$ du -sh blocks/ chainstate/
84G blocks/
13G chainstate/
```
(https://github.com/bitcoin/bitcoin/pull/30658#discussion_r1719854952)
```
$ du -sh blocks/ chainstate/
84G blocks/
13G chainstate/
```
💬 maflcko commented on pull request "refactor: Allow CScript construction from any std::input_iterator":
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-2293532880)
> changes were squashed into a second commit as they appear to belong closer together.
Yeah, they are related, but the prevector one adds requirements on the code, and the cscript one removes requirements on the code, which is why they are in two commits.
(https://github.com/bitcoin/bitcoin/pull/29369#issuecomment-2293532880)
> changes were squashed into a second commit as they appear to belong closer together.
Yeah, they are related, but the prevector one adds requirements on the code, and the cscript one removes requirements on the code, which is why they are in two commits.
💬 glozow commented on pull request "test: add functional test for XORed block/undo files (`-blocksxor` option)":
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1719919217)
I tried this on my apple machine and am able to make this test fail if I add a sleep right before the line checking `fs::is_empty`. No sign of DS_Store. It kind of seems more like blk0000.dat, rev0000.dat, and index/ are showing up early... I don't think I know init well enough to know exactly what order everything should happen in.
(https://github.com/bitcoin/bitcoin/pull/30657#discussion_r1719919217)
I tried this on my apple machine and am able to make this test fail if I add a sleep right before the line checking `fs::is_empty`. No sign of DS_Store. It kind of seems more like blk0000.dat, rev0000.dat, and index/ are showing up early... I don't think I know init well enough to know exactly what order everything should happen in.