⚠️ dergoegge opened an issue: "fuzz: Left over tmp files when fuzzing with afl"
(https://github.com/bitcoin/bitcoin/issues/28811)
I frequently run out of disk space when fuzzing our targets with afl++ due to left over tmp files created by our `TestingSetup`s. Similar issues were previously reported (see #22572, #22472).
The tmp files are left over when test cases cause timeouts (or crashes) and the afl++ fork-server simply kills the process, leaving it no time to cleanup. Note: this is not a bug in afl++, they specifically advise against creating tmp files in fuzz tests. Increasing the timeout threshold sort of works bu
...
(https://github.com/bitcoin/bitcoin/issues/28811)
I frequently run out of disk space when fuzzing our targets with afl++ due to left over tmp files created by our `TestingSetup`s. Similar issues were previously reported (see #22572, #22472).
The tmp files are left over when test cases cause timeouts (or crashes) and the afl++ fork-server simply kills the process, leaving it no time to cleanup. Note: this is not a bug in afl++, they specifically advise against creating tmp files in fuzz tests. Increasing the timeout threshold sort of works bu
...
💬 maflcko commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798247265)
I am not sure how doable this is, as it requires every storage access to be mocked in all parts of the codebase?
I think it would be better to fix the underlying timeouts and crashes, and treat them as bugs.
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798247265)
I am not sure how doable this is, as it requires every storage access to be mocked in all parts of the codebase?
I think it would be better to fix the underlying timeouts and crashes, and treat them as bugs.
💬 virtu commented on issue "Increase # of block-relay-only connections ":
(https://github.com/bitcoin/bitcoin/issues/28462#issuecomment-1798257179)
> #### Estimating the number of reachable clearnet nodes (per sept 2023)
> 6155 ([bitnodes](https://bitnodes.io/nodes/))
> 8509 ([KIT](https://www.dsn.kastel.kit.edu/bitcoin/data/churn0-v.gpd))
> 3862 ([Luke Dashjr](https://luke.dashjr.org/programs/bitcoin/files/charts/historical.html))
> 7910 ([21 Ninja](https://21.ninja/reachable-nodes/nodes-by-net-type/))
I think the misnomer "number of reachable nodes" is finally coming back to haunt us, since these numbers actually represent the numb
...
(https://github.com/bitcoin/bitcoin/issues/28462#issuecomment-1798257179)
> #### Estimating the number of reachable clearnet nodes (per sept 2023)
> 6155 ([bitnodes](https://bitnodes.io/nodes/))
> 8509 ([KIT](https://www.dsn.kastel.kit.edu/bitcoin/data/churn0-v.gpd))
> 3862 ([Luke Dashjr](https://luke.dashjr.org/programs/bitcoin/files/charts/historical.html))
> 7910 ([21 Ninja](https://21.ninja/reachable-nodes/nodes-by-net-type/))
I think the misnomer "number of reachable nodes" is finally coming back to haunt us, since these numbers actually represent the numb
...
💬 Sjors commented on pull request "guix: switch to 6.1 kernel headers over 5.15":
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1798257771)
<details><summary>Some hashes</summary>
```
9aba5e0ea27cbf609baac52e65f0eb63bc42aa7e1865108422d30b0bad01a7da guix-build-380e3655631b/output/aarch64-linux-gnu/SHA256SUMS.part
67644dd783c781ba6ae6bffa91204d3add423fe856b3aac09cb18c5f6db18599 guix-build-380e3655631b/output/aarch64-linux-gnu/bitcoin-380e3655631b-aarch64-linux-gnu-debug.tar.gz
0130236c032fbdcb5b791510d2ecf61239b434a1996b4b35ad7e7996680d339f guix-build-380e3655631b/output/aarch64-linux-gnu/bitcoin-380e3655631b-aarch64-linux-gn
...
(https://github.com/bitcoin/bitcoin/pull/28786#issuecomment-1798257771)
<details><summary>Some hashes</summary>
```
9aba5e0ea27cbf609baac52e65f0eb63bc42aa7e1865108422d30b0bad01a7da guix-build-380e3655631b/output/aarch64-linux-gnu/SHA256SUMS.part
67644dd783c781ba6ae6bffa91204d3add423fe856b3aac09cb18c5f6db18599 guix-build-380e3655631b/output/aarch64-linux-gnu/bitcoin-380e3655631b-aarch64-linux-gnu-debug.tar.gz
0130236c032fbdcb5b791510d2ecf61239b434a1996b4b35ad7e7996680d339f guix-build-380e3655631b/output/aarch64-linux-gnu/bitcoin-380e3655631b-aarch64-linux-gn
...
💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798267555)
There's no way for a node to tell its peers that the mempool transactions its relaying rely on an assumed state. The same goes for block relay. Are you suggesting we should change the p2p protocol to add this information?
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798267555)
There's no way for a node to tell its peers that the mempool transactions its relaying rely on an assumed state. The same goes for block relay. Are you suggesting we should change the p2p protocol to add this information?
💬 Sjors commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1798278012)
> I think it's not a convenient way to call `txin.scriptSig.GetOp(pc, opcode, vch);` three times.
Indeed it's not. But I don't understand what you're trying to do here.
I would suggest first fixing the tests, so that it's clear if the code is correct. And then worry about making the code nicer.
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1798278012)
> I think it's not a convenient way to call `txin.scriptSig.GetOp(pc, opcode, vch);` three times.
Indeed it's not. But I don't understand what you're trying to do here.
I would suggest first fixing the tests, so that it's clear if the code is correct. And then worry about making the code nicer.
💬 dergoegge commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798293005)
> I think it would be better to fix the underlying timeouts and crashes, and treat them as bugs.
I mean for crashes sure but for timeouts I have the suspicion that this happens on almost all targets that use `TestingSetup`. There might even be another reason for why these files stick around besides timeouts.
There is just no nice way of preventing this besides "don't create tmp files" and I don't want to be running around "fixing" dozens of targets and trying to maintain this everytime we
...
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798293005)
> I think it would be better to fix the underlying timeouts and crashes, and treat them as bugs.
I mean for crashes sure but for timeouts I have the suspicion that this happens on almost all targets that use `TestingSetup`. There might even be another reason for why these files stick around besides timeouts.
There is just no nice way of preventing this besides "don't create tmp files" and I don't want to be running around "fixing" dozens of targets and trying to maintain this everytime we
...
💬 Sjors commented on pull request "index: blockfilter initial sync speedup, parallelize process":
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-1798294413)
> Could also give it a run rebased on top #27006.
That PR currently does not cleanly cherry-pick on top of this PR, not can I (trivially) rebase this PR on top top of it. Happy to try if you can make a branch.
(https://github.com/bitcoin/bitcoin/pull/26966#issuecomment-1798294413)
> Could also give it a run rebased on top #27006.
That PR currently does not cleanly cherry-pick on top of this PR, not can I (trivially) rebase this PR on top top of it. Happy to try if you can make a branch.
👍 brunoerg approved a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789#pullrequestreview-1717431352)
utACK fa7ba926300f44b350d04262e569cc607e4991d8
(https://github.com/bitcoin/bitcoin/pull/28789#pullrequestreview-1717431352)
utACK fa7ba926300f44b350d04262e569cc607e4991d8
🚀 fanquake merged a pull request: "fuzz: Avoid utxo_total_supply timeout (take 2)"
(https://github.com/bitcoin/bitcoin/pull/28789)
(https://github.com/bitcoin/bitcoin/pull/28789)
💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384772837)
taken
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384772837)
taken
💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384772909)
taken
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384772909)
taken
💬 maflcko commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798342200)
No, I am saying, as explained in my first comment, that this pull request isn't needed. If it was needed, a reason should be given in the pull request description. So far I haven't seen a valid reason, reading the whole thread.
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798342200)
No, I am saying, as explained in my first comment, that this pull request isn't needed. If it was needed, a reason should be given in the pull request description. So far I haven't seen a valid reason, reading the whole thread.
💬 maflcko commented on issue "fuzz: Left over tmp files when fuzzing with afl++":
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798350581)
> I don't want to be running around "fixing" dozens of targets and trying to maintain this everytime we add a new one.
In essence, this is identical to the rpc timeout issue in functional tests. I don't think a cheat code is available to sidestep maintenance here.
If you only care about not creating leftover tmp files, you can increase the timeout massively. If temp files are still created, it should be reported and treated as a bug and fixed.
However, I think we should also treat lower
...
(https://github.com/bitcoin/bitcoin/issues/28811#issuecomment-1798350581)
> I don't want to be running around "fixing" dozens of targets and trying to maintain this everytime we add a new one.
In essence, this is identical to the rpc timeout issue in functional tests. I don't think a cheat code is available to sidestep maintenance here.
If you only care about not creating leftover tmp files, you can increase the timeout massively. If temp files are still created, it should be reported and treated as a bug and fixed.
However, I think we should also treat lower
...
💬 glozow commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1798369856)
Addressed https://github.com/bitcoin/bitcoin/pull/28785#pullrequestreview-1716255880 and fixed the ci error
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1798369856)
Addressed https://github.com/bitcoin/bitcoin/pull/28785#pullrequestreview-1716255880 and fixed the ci error
💬 Sjors commented on pull request "Show transactions as not fully confirmed during background validation":
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798372461)
@maflcko I copy-pasted @luke-jr's comment in the PR description, if that helps?
(https://github.com/bitcoin/bitcoin/pull/28616#issuecomment-1798372461)
@maflcko I copy-pasted @luke-jr's comment in the PR description, if that helps?
💬 Sjors commented on issue "assumeutxo: Ensure transactions are not presented as confirmed until background sync is complete":
(https://github.com/bitcoin/bitcoin/issues/28598#issuecomment-1798373704)
@pablomartin4btc I also added that in #28616
(https://github.com/bitcoin/bitcoin/issues/28598#issuecomment-1798373704)
@pablomartin4btc I also added that in #28616
💬 theStack commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798376058)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798376058)
Concept ACK
💬 instagibbs commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1798385890)
reACK https://github.com/bitcoin/bitcoin/pull/28785/commits/1147e00e59e47f27024ec96629993c66a3ce4ef0
took reconsiderable variable renames, tests using CheckPackageMempoolAcceptResult more, unsure why ci was failing though :)
(https://github.com/bitcoin/bitcoin/pull/28785#issuecomment-1798385890)
reACK https://github.com/bitcoin/bitcoin/pull/28785/commits/1147e00e59e47f27024ec96629993c66a3ce4ef0
took reconsiderable variable renames, tests using CheckPackageMempoolAcceptResult more, unsure why ci was failing though :)
💬 maflcko commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798390693)
> Though after #24748 we might also want to consider running the entire suite with `--v2transport` in some CI.
I presume this will fix itself, once and if v2 is enabled by default?
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1798390693)
> Though after #24748 we might also want to consider running the entire suite with `--v2transport` in some CI.
I presume this will fix itself, once and if v2 is enabled by default?