💬 fjahr commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1719652387)
How up-to-date is that ASMap file? If it's not up updated regularly it might be better to use one of the ones available at https://github.com/fjahr/asmap-data. Maybe this is a change in process that requires more eyes on it and I wouldn't call this a blocker then but I would like to propose this as a goal for the next release to use something more recent.
cc: @sipa
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1719652387)
How up-to-date is that ASMap file? If it's not up updated regularly it might be better to use one of the ones available at https://github.com/fjahr/asmap-data. Maybe this is a change in process that requires more eyes on it and I wouldn't call this a blocker then but I would like to propose this as a goal for the next release to use something more recent.
cc: @sipa
💬 fjahr commented on pull request "seeds: Pull additional nodes from my seeder and update fixed seeds":
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1719633747)
nit: If we want this `MIN_BLOCKS` to default to mainnet then you could update it to `840_000` while you make edits here. But maybe it would be better to have a more concise behavior here in the future. I guess the file could have a header with the network, number of min blocks that makes sense etc. based on the numbers the seeder node has.
(https://github.com/bitcoin/bitcoin/pull/30008#discussion_r1719633747)
nit: If we want this `MIN_BLOCKS` to default to mainnet then you could update it to `840_000` while you make edits here. But maybe it would be better to have a more concise behavior here in the future. I guess the file could have a header with the network, number of min blocks that makes sense etc. based on the numbers the seeder node has.
💬 marcofleon commented on pull request "fuzz: Faster utxo_snapshot fuzz target":
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293291058)
The second one just shows the same plot zoomed in on the last 600k iterations or so. I'm going to follow up with a plot of executions vs time as well. And I'll do more than a million iterations, as it seems like `utxo_snapshot_invalid` might end up being faster if fuzzed for hours.
I compiled using only `
...
(https://github.com/bitcoin/bitcoin/pull/30644#issuecomment-2293291058)
The second one just shows the same plot zoomed in on the last 600k iterations or so. I'm going to follow up with a plot of executions vs time as well. And I'll do more than a million iterations, as it seems like `utxo_snapshot_invalid` might end up being faster if fuzzed for hours.
I compiled using only `
...
💬 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).