👍 stickies-v approved a pull request: "log: Enforce trailing newline"
(https://github.com/bitcoin/bitcoin/pull/30929#pullrequestreview-2338066360)
ACK fa6444fe628fc33b77d45773c13b27a5a557cd2f , I much prefer this new approach with less code complexity and a more user-friendly logging interface, that allows for a gradual transition to not having `\n`-terminated log statements.
I wonder if it would make more sense to switch the commit order so the `bitcoin-tidy` check is removed after auto-appending the newline, instead of before?
As a quick belt-and-suspenders way to check that there's no behaviour change, I ran a `-debug=all` node fo
...
(https://github.com/bitcoin/bitcoin/pull/30929#pullrequestreview-2338066360)
ACK fa6444fe628fc33b77d45773c13b27a5a557cd2f , I much prefer this new approach with less code complexity and a more user-friendly logging interface, that allows for a gradual transition to not having `\n`-terminated log statements.
I wonder if it would make more sense to switch the commit order so the `bitcoin-tidy` check is removed after auto-appending the newline, instead of before?
As a quick belt-and-suspenders way to check that there's no behaviour change, I ran a `-debug=all` node fo
...
💬 stickies-v commented on pull request "log: Enforce trailing newline":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1781470805)
nit: I think this comment should now also be removed: https://github.com/bitcoin/bitcoin/blob/f3c74c4a7e120ea363abe4d4aa034b85c1d71919/src/logging.h#L256
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1781470805)
nit: I think this comment should now also be removed: https://github.com/bitcoin/bitcoin/blob/f3c74c4a7e120ea363abe4d4aa034b85c1d71919/src/logging.h#L256
💬 stickies-v commented on pull request "log: Enforce trailing newline":
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1781444088)
Suppression for this file can be cleaned up now:
<details>
<summary>git diff on fa6444fe62</summary>
```diff
diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py
index a809851ec6..86a17fb0f8 100755
--- a/test/lint/lint-format-strings.py
+++ b/test/lint/lint-format-strings.py
@@ -62,7 +62,7 @@ def main():
matching_files_filtered = []
for matching_file in matching_files:
- if not re.search('^src/(leveldb|secp256k1|minisketc
...
(https://github.com/bitcoin/bitcoin/pull/30929#discussion_r1781444088)
Suppression for this file can be cleaned up now:
<details>
<summary>git diff on fa6444fe62</summary>
```diff
diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py
index a809851ec6..86a17fb0f8 100755
--- a/test/lint/lint-format-strings.py
+++ b/test/lint/lint-format-strings.py
@@ -62,7 +62,7 @@ def main():
matching_files_filtered = []
for matching_file in matching_files:
- if not re.search('^src/(leveldb|secp256k1|minisketc
...
💬 stickies-v commented on pull request "[28.x] backports and finalize":
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2383735956)
It seems to have been the process for 26.0 and 27.0 at least:
- https://github.com/bitcoin/bitcoin/pull/29780/files#diff-e04cb1e9f29f76897c6b84334238f621ff45ba13cf180ad1d0d442d50ebe51f7
- https://github.com/bitcoin/bitcoin/pull/28959/files#diff-e04cb1e9f29f76897c6b84334238f621ff45ba13cf180ad1d0d442d50ebe51f7
(https://github.com/bitcoin/bitcoin/pull/30959#issuecomment-2383735956)
It seems to have been the process for 26.0 and 27.0 at least:
- https://github.com/bitcoin/bitcoin/pull/29780/files#diff-e04cb1e9f29f76897c6b84334238f621ff45ba13cf180ad1d0d442d50ebe51f7
- https://github.com/bitcoin/bitcoin/pull/28959/files#diff-e04cb1e9f29f76897c6b84334238f621ff45ba13cf180ad1d0d442d50ebe51f7
💬 achow101 commented on pull request "docs: Add instructions on how to self-sign bitcoin-core binaries for macOS":
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2383736840)
Should also add this to the current release notes draft: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft
(https://github.com/bitcoin/bitcoin/pull/30982#issuecomment-2383736840)
Should also add this to the current release notes draft: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Notes-Draft
✅ torkelrogstad closed a pull request: "rpc: validate fee estimation mode case insensitive"
(https://github.com/bitcoin/bitcoin/pull/29175)
(https://github.com/bitcoin/bitcoin/pull/29175)
💬 torkelrogstad commented on pull request "rpc: validate fee estimation mode case insensitive":
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-2383747798)
No, not really. Feel free to pick it up and get it in a mergeable state.
(https://github.com/bitcoin/bitcoin/pull/29175#issuecomment-2383747798)
No, not really. Feel free to pick it up and get it in a mergeable state.
💬 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
...