💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1573916898)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).
done, very easy with scripted diff!
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1573916898)
> 🐙 This pull request conflicts with the target branch and [needs rebase](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#rebasing-changes).
done, very easy with scripted diff!
💬 fanquake commented on pull request "Updated the installation instructions for macOS":
(https://github.com/bitcoin/bitcoin/pull/27525#issuecomment-1573922184)
Thanks, however we are going to leave this as-is for now. Generic changes to build docs are better suited if PR'd against all platforms, not sure we need to make the other additions.
(https://github.com/bitcoin/bitcoin/pull/27525#issuecomment-1573922184)
Thanks, however we are going to leave this as-is for now. Generic changes to build docs are better suited if PR'd against all platforms, not sure we need to make the other additions.
✅ fanquake closed a pull request: "Updated the installation instructions for macOS"
(https://github.com/bitcoin/bitcoin/pull/27525)
(https://github.com/bitcoin/bitcoin/pull/27525)
💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1573923355)
FYI it appears that in addition to Luxor, AntPool and possibly one or two other pools are mining full-rbf replacements now with at least some of their hashing power. Wasabi also recently had a [double-spend](https://twitter.com/Turbolay/status/1654517651862065152) that would have been resolved with full-rbf, exactly as I and others have been arguing. Binance also appears to be [doing full-rbf replacements](https://twitter.com/mononautical/status/1664412056227921920) to bump-fees on their consoli
...
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1573923355)
FYI it appears that in addition to Luxor, AntPool and possibly one or two other pools are mining full-rbf replacements now with at least some of their hashing power. Wasabi also recently had a [double-spend](https://twitter.com/Turbolay/status/1654517651862065152) that would have been resolved with full-rbf, exactly as I and others have been arguing. Binance also appears to be [doing full-rbf replacements](https://twitter.com/mononautical/status/1664412056227921920) to bump-fees on their consoli
...
👍 fanquake approved a pull request: "doc: document json rpc endpoints"
(https://github.com/bitcoin/bitcoin/pull/27225#pullrequestreview-1457807717)
ACK 65e3abcbf2b9e818f3b9f1ba35f3cfe7df5e3811 - Seems fine. Can be improved if need be.
(https://github.com/bitcoin/bitcoin/pull/27225#pullrequestreview-1457807717)
ACK 65e3abcbf2b9e818f3b9f1ba35f3cfe7df5e3811 - Seems fine. Can be improved if need be.
✅ fanquake closed an issue: "Document JSON-RPC wallet endpoints"
(https://github.com/bitcoin/bitcoin/issues/20246)
(https://github.com/bitcoin/bitcoin/issues/20246)
🚀 fanquake merged a pull request: "doc: document json rpc endpoints"
(https://github.com/bitcoin/bitcoin/pull/27225)
(https://github.com/bitcoin/bitcoin/pull/27225)
💬 fanquake commented on pull request "guix: remove cURL from build env":
(https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1573938880)
> is the optimal shortcut.
I think I'd rather remove all the dead code. The option confusion comes from the fact that these flags seem to just fail silently if the relevant libraries haven't been compiled/installed with CMake, which I was look at for 2.6.
(https://github.com/bitcoin/bitcoin/pull/27779#issuecomment-1573938880)
> is the optimal shortcut.
I think I'd rather remove all the dead code. The option confusion comes from the fact that these flags seem to just fail silently if the relevant libraries haven't been compiled/installed with CMake, which I was look at for 2.6.
💬 ryanofsky commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214538307)
'> Thank you for your comprehensive answer. I think it boils down to: A fatal error should not necessarily have to trigger an interrupt for all kernel operations.
Yes, this is definitely a goal. Libbitcoinkernel should try to be a library, not an application or application framework. It should have a convenient way of cancelling operations and not force everything to use one shutdown variable.
But both the current PR and my branch do already support this. The main difference between the PR
...
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214538307)
'> Thank you for your comprehensive answer. I think it boils down to: A fatal error should not necessarily have to trigger an interrupt for all kernel operations.
Yes, this is definitely a goal. Libbitcoinkernel should try to be a library, not an application or application framework. It should have a convenient way of cancelling operations and not force everything to use one shutdown variable.
But both the current PR and my branch do already support this. The main difference between the PR
...
💬 virtu commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1573950719)
@0xB10C and I had a look at this today. Here's what we found so far:
1. We believe the issue is caused by [one or more of the assertions](https://github.com/bitcoin/bitcoin/blob/b22408df162a224d94ac54e8443b57ef3fd2ca72/test/functional/interface_usdt_mempool.py#L306-L307) failing in the `handle_rejected_event` callback (although we are not sure why they fail).
2. The test framework's `assert_equal()` communicates failed assertions via exceptions. Unfortunately exceptions raised in bcc callback
...
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1573950719)
@0xB10C and I had a look at this today. Here's what we found so far:
1. We believe the issue is caused by [one or more of the assertions](https://github.com/bitcoin/bitcoin/blob/b22408df162a224d94ac54e8443b57ef3fd2ca72/test/functional/interface_usdt_mempool.py#L306-L307) failing in the `handle_rejected_event` callback (although we are not sure why they fail).
2. The test framework's `assert_equal()` communicates failed assertions via exceptions. Unfortunately exceptions raised in bcc callback
...
💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214545050)
> Also in both cases, it would be better if the kernel wasn't deciding itself whether to return early, but instead asked the application through std::function callbacks. I.e. there could be a std::function callbacks for "I just a new chain tip, should I keep processing or return early?" that would be more general than having hardcoded -stopatheight logic in the kernel.
How about making `startShutdown` return a boolean then that can be interpreted by calling code to either terminate the funct
...
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214545050)
> Also in both cases, it would be better if the kernel wasn't deciding itself whether to return early, but instead asked the application through std::function callbacks. I.e. there could be a std::function callbacks for "I just a new chain tip, should I keep processing or return early?" that would be more general than having hardcoded -stopatheight logic in the kernel.
How about making `startShutdown` return a boolean then that can be interpreted by calling code to either terminate the funct
...
💬 ryanofsky commented on pull request "init: Error if ignored bitcoin.conf file is found":
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1573955807)
No I still need to figure out what the last message https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1558537251 means, and finish #27409 to fix the ["Unfortunately the behavior for bitcoin-qt is stupider than bitcoind..."](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470284856) issue
(https://github.com/bitcoin/bitcoin/pull/27302#issuecomment-1573955807)
No I still need to figure out what the last message https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1558537251 means, and finish #27409 to fix the ["Unfortunately the behavior for bitcoin-qt is stupider than bitcoind..."](https://github.com/bitcoin/bitcoin/issues/27246#issuecomment-1470284856) issue
💬 pinheadmz commented on pull request "wallet: Add wallet method to detect if a key is "active"":
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1214557458)
Thanks, both comments addressed with rebase
(https://github.com/bitcoin/bitcoin/pull/27216#discussion_r1214557458)
Thanks, both comments addressed with rebase
🚀 fanquake merged a pull request: "walletdb: Add PrefixCursor"
(https://github.com/bitcoin/bitcoin/pull/27790)
(https://github.com/bitcoin/bitcoin/pull/27790)
💬 ryanofsky commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214568318)
> How about making `startShutdown` return a boolean then that can be interpreted by calling code to either terminate the function or continue?
I think it's best if the notification interfaces just streamed notifications in one direction to the application, to update its UI, or trigger tasks like starting other services, saving state, freeing up disk space, or shutting down.
The notifications interface seems like an awkward way to send information the other direction from the application t
...
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214568318)
> How about making `startShutdown` return a boolean then that can be interpreted by calling code to either terminate the function or continue?
I think it's best if the notification interfaces just streamed notifications in one direction to the application, to update its UI, or trigger tasks like starting other services, saving state, freeing up disk space, or shutting down.
The notifications interface seems like an awkward way to send information the other direction from the application t
...
👍 pinheadmz approved a pull request: "Raise on invalid -debug and -loglevel config options"
(https://github.com/bitcoin/bitcoin/pull/27632#pullrequestreview-1457942685)
ACK 1a27bec1e9e40166d6ff7f0492115107289e7609
Reviewed code and ran tests locally. Confirmed expected behavior locally with regtest:
```
--> bcd -regtest -debug=poop
Error: Unsupported logging category -debug=poop
--> bcd -regtest -loglevel=poop
Error: Unsupported global logging level -loglevel=poop. Valid values: info, debug, trace.
```
Just had a question about workflow/testing below, not a deal breaker.
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MES
...
(https://github.com/bitcoin/bitcoin/pull/27632#pullrequestreview-1457942685)
ACK 1a27bec1e9e40166d6ff7f0492115107289e7609
Reviewed code and ran tests locally. Confirmed expected behavior locally with regtest:
```
--> bcd -regtest -debug=poop
Error: Unsupported logging category -debug=poop
--> bcd -regtest -loglevel=poop
Error: Unsupported global logging level -loglevel=poop. Valid values: info, debug, trace.
```
Just had a question about workflow/testing below, not a deal breaker.
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MES
...
💬 pinheadmz commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1214574564)
If you're touching unit tests anyway, why not add a test for invalid args here as well?
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1214574564)
If you're touching unit tests anyway, why not add a test for invalid args here as well?
💬 pinheadmz commented on pull request "net: Allow inbound whitebind connections to more aggressively evict peers when slots are full":
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1214589081)
interesting idea, so we could add `NetPermissionFlags::NoBanForce` or something. The danger will still be present for users of the permission, but existing users of `NoBan` wouldn't have to worry. And since `NoBan` is a default of `whitebind` it'll require more user attention anyway to use the more dangerous option.
In that case, the release note should still warn the user about using `NoBanForce` (or whatever we call it) but that warning can just be general, like, keep an eye on your netinfo
...
(https://github.com/bitcoin/bitcoin/pull/27600#discussion_r1214589081)
interesting idea, so we could add `NetPermissionFlags::NoBanForce` or something. The danger will still be present for users of the permission, but existing users of `NoBan` wouldn't have to worry. And since `NoBan` is a default of `whitebind` it'll require more user attention anyway to use the more dangerous option.
In that case, the release note should still warn the user about using `NoBanForce` (or whatever we call it) but that warning can just be general, like, keep an eye on your netinfo
...
💬 theuni commented on pull request "depends: modernize clang flags for Darwin":
(https://github.com/bitcoin/bitcoin/pull/27798#discussion_r1214591743)
Fixed upstream: https://github.com/llvm/llvm-project/commit/5b77e752dcd073846b89559d6c0e1a7699e58615 for clang 17.
:D
(https://github.com/bitcoin/bitcoin/pull/27798#discussion_r1214591743)
Fixed upstream: https://github.com/llvm/llvm-project/commit/5b77e752dcd073846b89559d6c0e1a7699e58615 for clang 17.
:D
💬 zkfrio commented on pull request "wallet: Deniability API (Unilateral Transaction Meta-Privacy)":
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1574030615)
> It's important to point out that this PR doesn't add functionality that couldn't be achieved manually before.
If a user wanted to split a coin and send it to themselves, they could do it with the existing Bitcoin Core (either via RPC/Console or the GUI). It's just cumbersome and error prone.
Good point and most of the people who tried this with coins involved in any incident regret doing it. Privacy in bitcoin is not as simple as doing some transactions with your own addresses automaticall
...
(https://github.com/bitcoin/bitcoin/pull/27792#issuecomment-1574030615)
> It's important to point out that this PR doesn't add functionality that couldn't be achieved manually before.
If a user wanted to split a coin and send it to themselves, they could do it with the existing Bitcoin Core (either via RPC/Console or the GUI). It's just cumbersome and error prone.
Good point and most of the people who tried this with coins involved in any incident regret doing it. Privacy in bitcoin is not as simple as doing some transactions with your own addresses automaticall
...