💬 stratospher commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1792239111)
d08a2ebf: sv2 is OFF when fuzzing - so we need to turn it ON here to fuzz locally. Also the sv2 fuzz tests aren't run on the CI.
(https://github.com/bitcoin/bitcoin/pull/29346#discussion_r1792239111)
d08a2ebf: sv2 is OFF when fuzzing - so we need to turn it ON here to fuzz locally. Also the sv2 fuzz tests aren't run on the CI.
💬 0xB10C commented on pull request "Halt processing of unrequested transactions v2":
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2401651398)
> Is anyone running this currently with the code set to always enforce or just log? it would be useful to know at what rate nodes are currently seeing unsolicited.
I switched one of my monitoring nodes to run https://github.com/0xB10C/bitcoin/commit/ba39837d999407a55c3784059f7cf07bdbdfce76 to collect some data on this. Having a glance at the logs since yesterday, I've mostly seen the same few peers sending me unsolicited transactions - at a rate of a few per minute.
> I'm particularly inte
...
(https://github.com/bitcoin/bitcoin/pull/30572#issuecomment-2401651398)
> Is anyone running this currently with the code set to always enforce or just log? it would be useful to know at what rate nodes are currently seeing unsolicited.
I switched one of my monitoring nodes to run https://github.com/0xB10C/bitcoin/commit/ba39837d999407a55c3784059f7cf07bdbdfce76 to collect some data on this. Having a glance at the logs since yesterday, I've mostly seen the same few peers sending me unsolicited transactions - at a rate of a few per minute.
> I'm particularly inte
...
🚀 fanquake merged a pull request: "refactor: include the proper header rather than forward-declaring RemovalReasonToString"
(https://github.com/bitcoin/bitcoin/pull/31058)
(https://github.com/bitcoin/bitcoin/pull/31058)
💬 hebasto commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2401784317)
> Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
Why are ninja builds not affected?
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2401784317)
> Combining with #30911 produces even more of a speedup (with Make, ninja is about the same).
Why are ninja builds not affected?
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2401821128)
`a51c2cdda5...09a7394759`: silence the bogus GCC warning about uninitialized `std::optional`
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2401821128)
`a51c2cdda5...09a7394759`: silence the bogus GCC warning about uninitialized `std::optional`
💬 vasild commented on pull request "Broadcast own transactions only via short-lived Tor or I2P connections":
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2401828752)
`09a7394759...6b10008441`: rebase due to conflicts
(https://github.com/bitcoin/bitcoin/pull/29415#issuecomment-2401828752)
`09a7394759...6b10008441`: rebase due to conflicts
💬 dergoegge commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2401942272)
> I build with these options to be able to be able to know if changes not related to fuzzing will break the build.
I assumed (incorrectly) that almost no one would be doing this anymore since building the fuzz binary is no longer the default behavior. What I'm proposing would require you to do a separate build with `-DBUILD_FOR_FUZZING=ON`, which is of course annoying if you just want to check that the fuzz binary compiles.
Another assumption I have (perhaps also incorrect) is that no one
...
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2401942272)
> I build with these options to be able to be able to know if changes not related to fuzzing will break the build.
I assumed (incorrectly) that almost no one would be doing this anymore since building the fuzz binary is no longer the default behavior. What I'm proposing would require you to do a separate build with `-DBUILD_FOR_FUZZING=ON`, which is of course annoying if you just want to check that the fuzz binary compiles.
Another assumption I have (perhaps also incorrect) is that no one
...
💬 VivaRado commented on issue "support BIP39 mnemonic in descriptors":
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-2402017418)
So we delete the comment about BIP39 UI implementation because @junderw down voted, without probably even looking at the code. @junderw your behavior is not appreciated. If you do not have something constructive to add, avoid negative displays of grandeur.
(https://github.com/bitcoin/bitcoin/issues/19151#issuecomment-2402017418)
So we delete the comment about BIP39 UI implementation because @junderw down voted, without probably even looking at the code. @junderw your behavior is not appreciated. If you do not have something constructive to add, avoid negative displays of grandeur.
💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402048557)
Thanks for the heads up. I'm now doing IBD with 28.0 on the same Windows installation. On earlier versions, the issue came up after a few days. I will check in again after a week if the issue is solved with blocksxor.
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402048557)
Thanks for the heads up. I'm now doing IBD with 28.0 on the same Windows installation. On earlier versions, the issue came up after a few days. I will check in again after a week if the issue is solved with blocksxor.
💬 naumenkogs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793328846)
Similarly, the `AddKnownTx` won't be ever called upon this return.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793328846)
Similarly, the `AddKnownTx` won't be ever called upon this return.
💬 naumenkogs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793322248)
673d6ee17dd48281fa42983f8b95553d5494e914
In this particular commit the updated value of `add_extra_compact_tx` is dropped on the floor. I think this is fixed in the final version on this PR :)
(and `AddToCompactExtraTransactions` is not called anymore inside ` if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {` )
I'm not much worried about the quality of intermediate commits, but it makes following the changes a bit difficult.
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793322248)
673d6ee17dd48281fa42983f8b95553d5494e914
In this particular commit the updated value of `add_extra_compact_tx` is dropped on the floor. I think this is fixed in the final version on this PR :)
(and `AddToCompactExtraTransactions` is not called anymore inside ` if (state.GetResult() == TxValidationResult::TX_MISSING_INPUTS) {` )
I'm not much worried about the quality of intermediate commits, but it makes following the changes a bit difficult.
💬 naumenkogs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793355952)
Not sure why it says the file is outdated. Probably because i commented on the un-touched line?
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1793355952)
Not sure why it says the file is outdated. Probably because i commented on the un-touched line?
📝 maflcko opened a pull request: " refactor: Check original (translatable) format string at compile-time "
(https://github.com/bitcoin/bitcoin/pull/31061)
All original format strings are fixed. This change surfaces errors in them at compile-time.
The implementation achieves this by allowing to delay the translation (or `std::string` construction) that previously happened in `Untranslated()` or `_()` by returning a new type from those functions. The new type can be converted to `bilingual_str` where needed.
This can be tested by adding a format string error in an original string literal and observing a new compile-time failure.
(https://github.com/bitcoin/bitcoin/pull/31061)
All original format strings are fixed. This change surfaces errors in them at compile-time.
The implementation achieves this by allowing to delay the translation (or `std::string` construction) that previously happened in `Untranslated()` or `_()` by returning a new type from those functions. The new type can be converted to `bilingual_str` where needed.
This can be tested by adding a format string error in an original string literal and observing a new compile-time failure.
💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402110673)
No luck I'm afraid. Version 28.0 also hangs after a few hours.
```
2024-10-09T11:48:11Z UpdateTip: new best=0000000000000000fee087a31f2c9c00f8a26935bf133a63ae4659bcc671460c height=287693 version=0x00000002 log2_work=76.854021 tx=33605130 date='2014-02-25T05:33:06Z' progress=0.030781 cache=323.7MiB(2627196txo)
2024-10-09T11:48:11Z UpdateTip: new best=0000000000000000db9ac4e9bbd0335f94305fc1f6772f5b39bb7c188f610f3e height=287694 version=0x00000002 log2_work=76.854163 tx=33606248 date='2014
...
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402110673)
No luck I'm afraid. Version 28.0 also hangs after a few hours.
```
2024-10-09T11:48:11Z UpdateTip: new best=0000000000000000fee087a31f2c9c00f8a26935bf133a63ae4659bcc671460c height=287693 version=0x00000002 log2_work=76.854021 tx=33605130 date='2014-02-25T05:33:06Z' progress=0.030781 cache=323.7MiB(2627196txo)
2024-10-09T11:48:11Z UpdateTip: new best=0000000000000000db9ac4e9bbd0335f94305fc1f6772f5b39bb7c188f610f3e height=287694 version=0x00000002 log2_work=76.854163 tx=33606248 date='2014
...
💬 maflcko commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402132872)
I presume this is semi-related to the "Anti"-virus software. It is possible that the software adds load on the system, which makes data corruption more likely, due to increased heat production. You could try to limit the power consumption for the machine, or limit the compute that Bitcoin Core consumes and see if it helps. However, I am not sure how to do that on Windows.
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-2402132872)
I presume this is semi-related to the "Anti"-virus software. It is possible that the software adds load on the system, which makes data corruption more likely, due to increased heat production. You could try to limit the power consumption for the machine, or limit the compute that Bitcoin Core consumes and see if it helps. However, I am not sure how to do that on Windows.
💬 maflcko commented on pull request "tinyformat: refactor: increase compile-time checks and don't throw for tfm::format_error":
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2402146900)
> EDIT: I also think the new uses of .c_str() are gross, though I didn't look into why those are necessary and that is not my main objection to this PR.
I removed `tfm::format_raw(fmt.original.c_str(), ...` in https://github.com/bitcoin/bitcoin/pull/31061, where it remains just `tfm::format(fmt.original, ...`. Obviously the two pulls conflict a bit, but the conflicts are trivial to solve either way.
(https://github.com/bitcoin/bitcoin/pull/30928#issuecomment-2402146900)
> EDIT: I also think the new uses of .c_str() are gross, though I didn't look into why those are necessary and that is not my main objection to this PR.
I removed `tfm::format_raw(fmt.original.c_str(), ...` in https://github.com/bitcoin/bitcoin/pull/31061, where it remains just `tfm::format(fmt.original, ...`. Obviously the two pulls conflict a bit, but the conflicts are trivial to solve either way.
📝 fanquake locked a pull request: "build: Disallow building fuzz binary without -DBUILD_FOR_FUZZING"
(https://github.com/bitcoin/bitcoin/pull/31059)
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057).
Build System:
- Modified `CMakeLists.txt` to prevent building the fuzz binary unless `BUILD_FOR_FUZZING` is enabled.
- Added a fatal error message if an attempt is made to build the fuzz binary without `BUILD_FOR_FUZZING`.
Building the fuzz binary without `-DBUILD_FOR_FUZZING` results in a binary that is less effective for testing because it won't crash on `Assume` statements and won't bypass
...
(https://github.com/bitcoin/bitcoin/pull/31059)
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057).
Build System:
- Modified `CMakeLists.txt` to prevent building the fuzz binary unless `BUILD_FOR_FUZZING` is enabled.
- Added a fatal error message if an attempt is made to build the fuzz binary without `BUILD_FOR_FUZZING`.
Building the fuzz binary without `-DBUILD_FOR_FUZZING` results in a binary that is less effective for testing because it won't crash on `Assume` statements and won't bypass
...
📝 fanquake locked a pull request: "ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING"
(https://github.com/bitcoin/bitcoin/pull/31060)
Certainly! Here is a title and description for your pull request that you can copy and paste:
---
**Title:**
ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
---
**Pull Request Description:**
### ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057) by updating the Continuous Integration (CI) workflow to disallow building the fuzz binary without `-DBUILD_FOR_FUZZING`
...
(https://github.com/bitcoin/bitcoin/pull/31060)
Certainly! Here is a title and description for your pull request that you can copy and paste:
---
**Title:**
ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
---
**Pull Request Description:**
### ci: Add separate fuzz-only CI jobs with -DBUILD_FOR_FUZZING
This pull request addresses [issue #31057](https://github.com/bitcoin/bitcoin/issues/31057) by updating the Continuous Integration (CI) workflow to disallow building the fuzz binary without `-DBUILD_FOR_FUZZING`
...
⚠️ virtu opened an issue: "Distribute darknet node addresses via DNS seeds using AAAA records"
(https://github.com/bitcoin/bitcoin/issues/31062)
### Please describe the feature you'd like to see added.
Right now, Bitcoin Core can only receive IPv4 and IPv6 node addresses from DNS seeds. Adding support for darknet addresses would bring the advantages of using DNS seeds for node discovery to darknet nodes.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
Before TorV2 addresses were deprecated, [Bitcoin Core](https://github.com/bitcoin/bitcoin/blob/5fe6878b5f7c1c97
...
(https://github.com/bitcoin/bitcoin/issues/31062)
### Please describe the feature you'd like to see added.
Right now, Bitcoin Core can only receive IPv4 and IPv6 node addresses from DNS seeds. Adding support for darknet addresses would bring the advantages of using DNS seeds for node discovery to darknet nodes.
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
Before TorV2 addresses were deprecated, [Bitcoin Core](https://github.com/bitcoin/bitcoin/blob/5fe6878b5f7c1c97
...
👍 ryanofsky approved a pull request: "refactor: Check translatable format strings at compile-time"
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2356947303)
Code review ACK fabb28f6c8f772d38917a23dfbda706292070ba1. This looks great, and implementation is very clean. However, I think it could be simplified significantly if there were a scripted diff commit to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))`, which would be a good change on its own for clarity and consistency. More details in comments below.
(https://github.com/bitcoin/bitcoin/pull/31061#pullrequestreview-2356947303)
Code review ACK fabb28f6c8f772d38917a23dfbda706292070ba1. This looks great, and implementation is very clean. However, I think it could be simplified significantly if there were a scripted diff commit to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))`, which would be a good change on its own for clarity and consistency. More details in comments below.
💬 ryanofsky commented on pull request "refactor: Check translatable format strings at compile-time":
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793416404)
In commit "refactor: Mark run-time Untranslated() c_str with std::string" (fa213aa88dfe9354c314a855c72ea33b3966c5c0)
I think this change is unfortunate and doesn't make sense. It seems like it could be a avoided with a small scripted diff to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))` and that replacement would make the code clearer and more consistent anyway.
(https://github.com/bitcoin/bitcoin/pull/31061#discussion_r1793416404)
In commit "refactor: Mark run-time Untranslated() c_str with std::string" (fa213aa88dfe9354c314a855c72ea33b3966c5c0)
I think this change is unfortunate and doesn't make sense. It seems like it could be a avoided with a small scripted diff to replace `strprintf(Untranslated("..."), ...)` with `Untranslated(strprintf("...", ...))` and that replacement would make the code clearer and more consistent anyway.