💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293319752)
@marcofleon Thanks for checking. I presume you checked the `utxo_snapshot` target, not the `utxo_snapshot_invalid` target and you found that the new target is slower in your run? I guess it is a bit hard to compare a random search path done by the fuzz engine in the input search space, because different paths may take a different time. As mentioned earlier, improving the "valid" target isn't a goal of this pull (https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2291393187). However, I'd
...
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293319752)
@marcofleon Thanks for checking. I presume you checked the `utxo_snapshot` target, not the `utxo_snapshot_invalid` target and you found that the new target is slower in your run? I guess it is a bit hard to compare a random search path done by the fuzz engine in the input search space, because different paths may take a different time. As mentioned earlier, improving the "valid" target isn't a goal of this pull (https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2291393187). However, I'd
...
💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293339170)
Thank you for the suggestions, I'll do more trials for sure. That actually is the `utxo_snapshot_invalid` target compared to the old `utxo_snapshot` (sorry, file names should've been clearer). It's much faster at the beginning. I ran it from scratch, not on any corpus. I'm generating a corpus now for the invalid target and then I'll do a comparison for both when fuzzing from a corpus.
Also, for the bug from https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2285605841. What's the r
...
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293339170)
Thank you for the suggestions, I'll do more trials for sure. That actually is the `utxo_snapshot_invalid` target compared to the old `utxo_snapshot` (sorry, file names should've been clearer). It's much faster at the beginning. I ran it from scratch, not on any corpus. I'm generating a corpus now for the invalid target and then I'll do a comparison for both when fuzzing from a corpus.
Also, for the bug from https://github.com/bitcoin/bitcoin/issues/30514#issuecomment-2285605841. What's the r
...
💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293345228)
To clarify, I reproduced the bug fairly quickly with the line you provided in https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2290858777. I would like to see what happens with `max_len` not specified.
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293345228)
To clarify, I reproduced the bug fairly quickly with the line you provided in https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2290858777. I would like to see what happens with `max_len` not specified.
💬 maflcko commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293350198)
> Thank you for the suggestions, I'll do more trials for sure. That actually is the `utxo_snapshot_invalid` target compared to the old `utxo_snapshot` (sorry, file names should've been clearer).
Are you sure you are on the right commit binaries? Locally I am seeing a speed difference of night and day. 1e6 iterations with the new target should happen within a few minutes, whereas with the old it would take hours/days.
> Also, for the bug from https://github.com/bitcoin/bitcoin/issues/30514#
...
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293350198)
> Thank you for the suggestions, I'll do more trials for sure. That actually is the `utxo_snapshot_invalid` target compared to the old `utxo_snapshot` (sorry, file names should've been clearer).
Are you sure you are on the right commit binaries? Locally I am seeing a speed difference of night and day. 1e6 iterations with the new target should happen within a few minutes, whereas with the old it would take hours/days.
> Also, for the bug from https://github.com/bitcoin/bitcoin/issues/30514#
...
💬 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.