💬 brunoerg commented on issue "docs: "Fuzzing the Bitcoin Core P2P layer using Honggfuzz NetDriver" is outdated":
(https://github.com/bitcoin/bitcoin/issues/30957#issuecomment-2383748628)
> Yes I think so but similar to the PoW check it never makes sense to have this check in the way. The p2p_transport_serialization harness currently has [a bunch of extra logic](https://github.com/bitcoin/bitcoin/blob/d812cf11896a2214467b6fa72d7b763bac6077c5/src/test/fuzz/p2p_transport_serialization.cpp#L43-L44) for reaching the unhappy/happy paths of these checks. We can achieve the same with the macro (without having to create valid checksum in the test), e.g.:
Looks good to me!
(https://github.com/bitcoin/bitcoin/issues/30957#issuecomment-2383748628)
> Yes I think so but similar to the PoW check it never makes sense to have this check in the way. The p2p_transport_serialization harness currently has [a bunch of extra logic](https://github.com/bitcoin/bitcoin/blob/d812cf11896a2214467b6fa72d7b763bac6077c5/src/test/fuzz/p2p_transport_serialization.cpp#L43-L44) for reaching the unhappy/happy paths of these checks. We can achieve the same with the macro (without having to create valid checksum in the test), e.g.:
Looks good to me!
💬 torkelrogstad commented on pull request "doc: update signet documentation related to build directories":
(https://github.com/bitcoin/bitcoin/pull/30996#discussion_r1781498059)
This is the build directory used by the [unix](https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#to-build) and [macOS](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#2-compile) build docs. IMO it's therefore not necessary to mention that here. If people are clever enough to change the build directories they'll also understand that this changes the location of binaries.
(https://github.com/bitcoin/bitcoin/pull/30996#discussion_r1781498059)
This is the build directory used by the [unix](https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#to-build) and [macOS](https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md#2-compile) build docs. IMO it's therefore not necessary to mention that here. If people are clever enough to change the build directories they'll also understand that this changes the location of binaries.
💬 achow101 commented on pull request "[28.x] backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2383758464)
Hmm ok, will add those soon. Still some changes to be made on the release notes.
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2383758464)
Hmm ok, will add those soon. Still some changes to be made on the release notes.
💬 pablomartin4btc commented on pull request "doc: update signet documentation related to build directories":
(https://github.com/bitcoin/bitcoin/pull/30996#discussion_r1781504068)
ok, just to be consistent on other places in the documentation where this is clarified
(https://github.com/bitcoin/bitcoin/pull/30996#discussion_r1781504068)
ok, just to be consistent on other places in the documentation where this is clarified
💬 jonatack commented on issue "V2 Only Option":
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2383823827)
> @jonatack I gotta say, looking up the background of the author on a pretty ordinary feature request feels a bit creepy to me.
@petertodd The rationale was mentioned in my comment, but to iterate on it, the proposal came out of left field from an unknown entity who insisted repeatedly in an odd manner on it. Also, the proposal may be potentially dangerous for the network (cf https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-1994415042 and https://github.com/bitcoin/bitcoin/pull/30
...
(https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-2383823827)
> @jonatack I gotta say, looking up the background of the author on a pretty ordinary feature request feels a bit creepy to me.
@petertodd The rationale was mentioned in my comment, but to iterate on it, the proposal came out of left field from an unknown entity who insisted repeatedly in an odd manner on it. Also, the proposal may be potentially dangerous for the network (cf https://github.com/bitcoin/bitcoin/issues/29618#issuecomment-1994415042 and https://github.com/bitcoin/bitcoin/pull/30
...
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1781550951)
Thanks! Done.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1781550951)
Thanks! Done.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2383847641)
@Sjors
> Building depends fails for me, maybe something wrong with `qttools_skip_dependencies.patch`?
The issue should be fixed now :)
(https://github.com/bitcoin/bitcoin/pull/30997#issuecomment-2383847641)
@Sjors
> Building depends fails for me, maybe something wrong with `qttools_skip_dependencies.patch`?
The issue should be fixed now :)
💬 stillhated commented on something "":
(https://github.com/bitcoin/bitcoin/commit/f5a2000579b140a1f51fc433706c775ca560c62c#commitcomment-147395454)
These blocks dont valadate
@limited
(https://github.com/bitcoin/bitcoin/commit/f5a2000579b140a1f51fc433706c775ca560c62c#commitcomment-147395454)
These blocks dont valadate
@limited
📝 Sjors opened a pull request: "Introduce waitFeesChanged() mining interface"
(https://github.com/bitcoin/bitcoin/pull/31003)
The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it's time to do so.
I split the implementation into two parts because I'm not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.
The first commit contains a mock implementation very similiar to how longpolling in `getblocktemplate` works. It calls checks `getTransactionsUpdated` every 10 seconds. This is triggered anytime a tra
...
(https://github.com/bitcoin/bitcoin/pull/31003)
The Stratum v2 protocol allows pushing out templates as fees in the mempool increase. This interface lets us know when it's time to do so.
I split the implementation into two parts because I'm not sure if we should include the second commit now, or wait until cluster mempool #30289 is merged.
The first commit contains a mock implementation very similiar to how longpolling in `getblocktemplate` works. It calls checks `getTransactionsUpdated` every 10 seconds. This is triggered anytime a tra
...
💬 Sjors commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1781581459)
a87589abfde4f2f1dfba0f3f1f5745f7acfef365: @ryanofsky or should I make this `@9` and leave the other numbers untouched?
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1781581459)
a87589abfde4f2f1dfba0f3f1f5745f7acfef365: @ryanofsky or should I make this `@9` and leave the other numbers untouched?
💬 fanquake commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1781587591)
In 57b8139a9c146f1146b9d3c92be35a7fb606585f (guix: Adjust for Qt 6): Not sure this is the right direction.
1. Why does Qt need special treatment here? It should be finding the compiler, the same as all other packages we build.
2. We shouldn't be hard-coding GCC usage, otherwise you're requiring a GCC toolchain to build releases for macOS, where it shouldn't be needed. Clang is capable of building native bins. i.e #30206.
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1781587591)
In 57b8139a9c146f1146b9d3c92be35a7fb606585f (guix: Adjust for Qt 6): Not sure this is the right direction.
1. Why does Qt need special treatment here? It should be finding the compiler, the same as all other packages we build.
2. We shouldn't be hard-coding GCC usage, otherwise you're requiring a GCC toolchain to build releases for macOS, where it shouldn't be needed. Clang is capable of building native bins. i.e #30206.
💬 hebasto commented on pull request "build: Switch to Qt 6":
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1781643646)
1. On the master branch, building in Guix for macOS does use a GCC toolchain to compile native Qt tools.
This PR does the same.
I agree that Clang could be used instead. However, I'd prefer to leave this change of of this PR scope.
2. Special treatment is needed for every native package that uses CMake. For now, this applies only to Qt.
By default, CMake looks for the `cc` and `c++` executables available in `PATH`. A GCC toolchain creates `bin/gcc` and `bin/c++` symlinks, while a Cl
...
(https://github.com/bitcoin/bitcoin/pull/30997#discussion_r1781643646)
1. On the master branch, building in Guix for macOS does use a GCC toolchain to compile native Qt tools.
This PR does the same.
I agree that Clang could be used instead. However, I'd prefer to leave this change of of this PR scope.
2. Special treatment is needed for every native package that uses CMake. For now, this applies only to Qt.
By default, CMake looks for the `cc` and `c++` executables available in `PATH`. A GCC toolchain creates `bin/gcc` and `bin/c++` symlinks, while a Cl
...
💬 ryanofsky commented on pull request "Add waitFeesChanged() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1781650328)
> [a87589a](https://github.com/bitcoin/bitcoin/commit/a87589abfde4f2f1dfba0f3f1f5745f7acfef365): @ryanofsky or should I make this `@9` and leave the other numbers untouched?
You can but it shouldn't matter, as long as nothing outside the build is depending on the interface. So far I've just renumbered everything like your current PR does. I even have a hacky renumbering script `capnp-renum`, that I pipe iinto from vim to renumber structs and interfaces:
```python
#!/usr/bin/env python
...
(https://github.com/bitcoin/bitcoin/pull/31003#discussion_r1781650328)
> [a87589a](https://github.com/bitcoin/bitcoin/commit/a87589abfde4f2f1dfba0f3f1f5745f7acfef365): @ryanofsky or should I make this `@9` and leave the other numbers untouched?
You can but it shouldn't matter, as long as nothing outside the build is depending on the interface. So far I've just renumbered everything like your current PR does. I even have a hacky renumbering script `capnp-renum`, that I pipe iinto from vim to renumber structs and interfaces:
```python
#!/usr/bin/env python
...
💬 achow101 commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2384063267)
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2384063267)
ACK 5c7cacf649a6b474b876a7d219c7dc683a25e33d
✅ achow101 closed an issue: "Add IPv6 pinhole support using UPnP / NAT-PMP"
(https://github.com/bitcoin/bitcoin/issues/17012)
(https://github.com/bitcoin/bitcoin/issues/17012)
🚀 achow101 merged a pull request: "net: Replace libnatpmp with built-in PCP+NATPMP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043)
(https://github.com/bitcoin/bitcoin/pull/30043)
💬 furszy commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2384109994)
> Just as a note, it should already be possible to pick the device via an env var, see https://en.cppreference.com/w/cpp/filesystem/temp_directory_path#Notes.
Hmm, thats a good observation.
Even when that may be possible now, I'm not sure we should start recommending the mixture of env vars and binary args at the user interface level. It seems more natural to plug the already existing `-testdatadir` arg to the benchmark framework. Still, if we move towards the env var direction, the next ste
...
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2384109994)
> Just as a note, it should already be possible to pick the device via an env var, see https://en.cppreference.com/w/cpp/filesystem/temp_directory_path#Notes.
Hmm, thats a good observation.
Even when that may be possible now, I'm not sure we should start recommending the mixture of env vars and binary args at the user interface level. It seems more natural to plug the already existing `-testdatadir` arg to the benchmark framework. Still, if we move towards the env var direction, the next ste
...
💬 achow101 commented on pull request "init: Remove retry for loop":
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2384121576)
ACK e9d60af9889c12b4d427adefa53fd234e417f8f6
(https://github.com/bitcoin/bitcoin/pull/30968#issuecomment-2384121576)
ACK e9d60af9889c12b4d427adefa53fd234e417f8f6
💬 achow101 commented on pull request "test: switch MiniWallet padding unit from weight to vsize":
(https://github.com/bitcoin/bitcoin/pull/30718#issuecomment-2384137098)
ACK 940edd6ac24e921f639b1376fe7d35dc14c1887d
(https://github.com/bitcoin/bitcoin/pull/30718#issuecomment-2384137098)
ACK 940edd6ac24e921f639b1376fe7d35dc14c1887d
🚀 achow101 merged a pull request: "init: Remove retry for loop"
(https://github.com/bitcoin/bitcoin/pull/30968)
(https://github.com/bitcoin/bitcoin/pull/30968)