💬 instagibbs commented on pull request "p2p: ensure mapBlockSource is removed from in ProcessBlock":
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1927184068)
@Crypt-iQ could you just open a parallel issue for this for tracking?
(https://github.com/bitcoin/bitcoin/pull/28235#issuecomment-1927184068)
@Crypt-iQ could you just open a parallel issue for this for tracking?
💬 maflcko commented on pull request "assumeutxo: Get rid of faked nTx and nChainTx values":
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1927200215)
Also, it would be good to include the test to ensure the crash no longer happens.
https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1918947945
(https://github.com/bitcoin/bitcoin/pull/29370#issuecomment-1927200215)
Also, it would be good to include the test to ensure the crash no longer happens.
https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1918947945
💬 theuni commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1927224463)
I've pushed a commit which temporarily puts the includes back in the low-level headers for the sake of addressing the benchmarks first, setting aside the possibility of missing defines.
@aureleoules's [benchmarks show 2 small regressions](https://corecheck.dev/bitcoin/bitcoin/pulls/29263), though I am unable to reproduce those locally.
My tests show: `./bench/bench_bitcoin -min-time=5000 -filter="AddrManGetAddr|PrevectorDeserializeNontrivial"`
master (3d52cedb497e0ec029ba3cef95b00169c157d
...
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1927224463)
I've pushed a commit which temporarily puts the includes back in the low-level headers for the sake of addressing the benchmarks first, setting aside the possibility of missing defines.
@aureleoules's [benchmarks show 2 small regressions](https://corecheck.dev/bitcoin/bitcoin/pulls/29263), though I am unable to reproduce those locally.
My tests show: `./bench/bench_bitcoin -min-time=5000 -filter="AddrManGetAddr|PrevectorDeserializeNontrivial"`
master (3d52cedb497e0ec029ba3cef95b00169c157d
...
💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-1927234971)
Then I misunderstood this:
> This class could Assume that the file was flushed/closed before the destructor is called
How?
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-1927234971)
Then I misunderstood this:
> This class could Assume that the file was flushed/closed before the destructor is called
How?
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927243781)
Removed the max_len and mutate_depth randomization, because it seemed too controversial? Removed this from OP:
----
Also, randomize `-max_len=` to possibly get some runs with faster iterations, or to produce smaller reduced fuzz inputs over time.
Also, randomize `-mutate_depth`, as lower values seem to be beneficial as well. [2]
[2] https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1655477388
This picks up the work started in commit https://www.github.com/bitcoin/bitcoin/
...
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927243781)
Removed the max_len and mutate_depth randomization, because it seemed too controversial? Removed this from OP:
----
Also, randomize `-max_len=` to possibly get some runs with faster iterations, or to produce smaller reduced fuzz inputs over time.
Also, randomize `-mutate_depth`, as lower values seem to be beneficial as well. [2]
[2] https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1655477388
This picks up the work started in commit https://www.github.com/bitcoin/bitcoin/
...
🤔 furszy reviewed a pull request: "index: blockfilter initial sync speedup, parallelize process"
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-1863088709)
Focus is on #28955, which contains a good number of commits decoupled from this PR.
Will come back here after it.
(https://github.com/bitcoin/bitcoin/pull/26966#pullrequestreview-1863088709)
Focus is on #28955, which contains a good number of commits decoupled from this PR.
Will come back here after it.
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927247191)
Also, added missing rss limit to avoid OOM
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927247191)
Also, added missing rss limit to avoid OOM
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478433023)
Sure, most have done it, but have you done it while *this* script is running? :)
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478433023)
Sure, most have done it, but have you done it while *this* script is running? :)
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478434665)
I am happy to review a pull request to change from running all targets to run `N` randomly drawn targets. However, I don't need the feature, so I won't be working on this personally.
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478434665)
I am happy to review a pull request to change from running all targets to run `N` randomly drawn targets. However, I don't need the feature, so I won't be working on this personally.
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478435806)
In any case, the change to max_total_time seems fine, given that https://github.com/bitcoin/bitcoin/pull/28178/files#r1346327126 also does it.
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478435806)
In any case, the change to max_total_time seems fine, given that https://github.com/bitcoin/bitcoin/pull/28178/files#r1346327126 also does it.
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478436472)
(removed max_len randomization for now)
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478436472)
(removed max_len randomization for now)
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478438572)
mutate_depth was dropped from this pull, for now
(https://github.com/bitcoin/bitcoin/pull/28178#discussion_r1478438572)
mutate_depth was dropped from this pull, for now
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927264262)
cc @dergoegge are you still unconvinced about this? Is there more references or data I can provide?
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1927264262)
cc @dergoegge are you still unconvinced about this? Is there more references or data I can provide?
💬 maflcko commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-1927271889)
AutoFile holds a file, which will be nullptr when it is closed. So if the file is nullptr, then it can be assumed to be flushed.
(https://github.com/bitcoin/bitcoin/pull/29307#issuecomment-1927271889)
AutoFile holds a file, which will be nullptr when it is closed. So if the file is nullptr, then it can be assumed to be flushed.
💬 edilmedeiros commented on pull request "test: Add makefile target for running unit tests":
(https://github.com/bitcoin/bitcoin/pull/29377#issuecomment-1927295459)
Tested ACK
`make -C src check-unit` does execute the same unit tests as `make check`.
I tried to include as many things as possible:
```
Options used to compile and link:
external signer = yes
multiprocess = no
with libs = yes
with wallet = yes
with sqlite = yes
with bdb = yes
with gui / qt = yes
with qr = yes
with zmq = yes
with test = yes
with fuzz binary = yes
with bench = yes
with upnp
...
(https://github.com/bitcoin/bitcoin/pull/29377#issuecomment-1927295459)
Tested ACK
`make -C src check-unit` does execute the same unit tests as `make check`.
I tried to include as many things as possible:
```
Options used to compile and link:
external signer = yes
multiprocess = no
with libs = yes
with wallet = yes
with sqlite = yes
with bdb = yes
with gui / qt = yes
with qr = yes
with zmq = yes
with test = yes
with fuzz binary = yes
with bench = yes
with upnp
...
💬 josibake commented on pull request "CKey: add Serialize and Unserialize":
(https://github.com/bitcoin/bitcoin/pull/29295#issuecomment-1927304095)
> > It also seems strange to me that all of this care is being taken to keep the secret data secure in memory when you are reading writing it unencrypted to and from disk.
>
> That's literally what the wallet does by default. `CKey` provides extra security for free, so it's better to use it. The optional wallet encryption feature makes use of this security feature, by ensuring that once a wallet is (re)locked the key data is erased from memory.
https://github.com/bitcoin/bitcoin/blob/maste
...
(https://github.com/bitcoin/bitcoin/pull/29295#issuecomment-1927304095)
> > It also seems strange to me that all of this care is being taken to keep the secret data secure in memory when you are reading writing it unencrypted to and from disk.
>
> That's literally what the wallet does by default. `CKey` provides extra security for free, so it's better to use it. The optional wallet encryption feature makes use of this security feature, by ensuring that once a wallet is (re)locked the key data is erased from memory.
https://github.com/bitcoin/bitcoin/blob/maste
...
💬 aureleoules commented on pull request "serialization: c++20 endian/byteswap/clz modernization":
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1927311589)
@theuni yeah the `AddrManGetAddr` and `PrevectorDeserializeNontrivial` are known to be flaky. I'll filter them out from the UI while I find a better solution.
Some benchmarks depend on I/O or randomness so it's hard a have consistent results between runs.
You can see ignored benchmarks here in the meantime: https://github.com/corecheck/frontend/blob/master/src/routes/%5Bowner%5D/%5Brepo%5D/pulls/%5Bnumber%5D/Benchmarks.svelte#L334
(https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1927311589)
@theuni yeah the `AddrManGetAddr` and `PrevectorDeserializeNontrivial` are known to be flaky. I'll filter them out from the UI while I find a better solution.
Some benchmarks depend on I/O or randomness so it's hard a have consistent results between runs.
You can see ignored benchmarks here in the meantime: https://github.com/corecheck/frontend/blob/master/src/routes/%5Bowner%5D/%5Brepo%5D/pulls/%5Bnumber%5D/Benchmarks.svelte#L334
👍 dergoegge approved a pull request: "fuzz: Generate with random libFuzzer settings"
(https://github.com/bitcoin/bitcoin/pull/28178#pullrequestreview-1863171718)
utACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde
(https://github.com/bitcoin/bitcoin/pull/28178#pullrequestreview-1863171718)
utACK fa3a4102ef0ae06d8930d7a7b567759e2a5b5fde
💬 Sjors commented on pull request "Stratum v2 Template Provider (take 2)":
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1927349134)
Added to `2024/02/sv2-poll-ellswift`: we now hold on to templates for 10 seconds after a new block arrives, in a case a miner was still working on it. The resulting block still wouldn't get broadcast.
(https://github.com/bitcoin/bitcoin/pull/28983#issuecomment-1927349134)
Added to `2024/02/sv2-poll-ellswift`: we now hold on to templates for 10 seconds after a new block arrives, in a case a miner was still working on it. The resulting block still wouldn't get broadcast.
💬 aureleoules commented on pull request "rpc: Optimize serialization disk space of dumptxoutset":
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1927351585)
Rebased. I had to change the offset again in `feature_assumeutxo.py`.
(https://github.com/bitcoin/bitcoin/pull/26045#issuecomment-1927351585)
Rebased. I had to change the offset again in `feature_assumeutxo.py`.