💬 maflcko commented on issue "Move from Static Dust Limit [330 / 546 sats] to Variable Dust Limit [= to TxFee]":
(https://github.com/bitcoin/bitcoin/issues/29423#issuecomment-1947951310)
> a more compatible solution might be to define it as a UTXO that cannot be economically spent at the fee level of the transaction creating it (or a multiplier of said fee)
That doesn't work either, because it is trivial to side-step. For example, one could create the transaction at a lower fee rate to create the dust, then raise the fee by creating a descendant transaction with a higher fee. (The result would be that the opposite of what this proposal is trying to achieve, is achieved). Anot
...
(https://github.com/bitcoin/bitcoin/issues/29423#issuecomment-1947951310)
> a more compatible solution might be to define it as a UTXO that cannot be economically spent at the fee level of the transaction creating it (or a multiplier of said fee)
That doesn't work either, because it is trivial to side-step. For example, one could create the transaction at a lower fee rate to create the dust, then raise the fee by creating a descendant transaction with a higher fee. (The result would be that the opposite of what this proposal is trying to achieve, is achieved). Anot
...
✅ maflcko closed an issue: "Move from Static Dust Limit [330 / 546 sats] to Variable Dust Limit [= to TxFee]"
(https://github.com/bitcoin/bitcoin/issues/29423)
(https://github.com/bitcoin/bitcoin/issues/29423)
💬 maflcko commented on issue "Move from Static Dust Limit [330 / 546 sats] to Variable Dust Limit [= to TxFee]":
(https://github.com/bitcoin/bitcoin/issues/29423#issuecomment-1947953302)
Not sure what the goal of this issue is. Anyone wanting to change the dust relay fee can already do so with the `-dustrelayfee` setting. In any case, if there is a technical sound proposal with proper motivation and a complete discussion of the effects on wallet software as well as P2P relay policy interactions, no one is holding anyone back from proposing those changes as a pull request, a BIP, or as a discussion with the broader ecosystem to gather feedback, for example https://groups.google.c
...
(https://github.com/bitcoin/bitcoin/issues/29423#issuecomment-1947953302)
Not sure what the goal of this issue is. Anyone wanting to change the dust relay fee can already do so with the `-dustrelayfee` setting. In any case, if there is a technical sound proposal with proper motivation and a complete discussion of the effects on wallet software as well as P2P relay policy interactions, no one is holding anyone back from proposing those changes as a pull request, a BIP, or as a discussion with the broader ecosystem to gather feedback, for example https://groups.google.c
...
💬 BrandonOdiwuor commented on issue "gen-manpages output depends on build options, so needs to check them":
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1947968598)
@theStack @hebasto @laanwj is the goal of this to generate just the missing docs or to generate all docs
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1947968598)
@theStack @hebasto @laanwj is the goal of this to generate just the missing docs or to generate all docs
💬 maflcko commented on issue "gen-manpages output depends on build options, so needs to check them":
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1947980558)
The goal is to add a check to the script, as described in this issue.
(https://github.com/bitcoin/bitcoin/issues/17506#issuecomment-1947980558)
The goal is to add a check to the script, as described in this issue.
🤔 vasild reviewed a pull request: "Improve new LogDebug/Trace/Info/Warning/Error Macros"
(https://github.com/bitcoin/bitcoin/pull/29256#pullrequestreview-1884614153)
> Make them all accept log categories, to make it possible to filter or highlight log messages from a particular component.
This seems like a step in the right direction. Currently we log messages unconditionally that don't have a category. But this is mixing two separate things - facility and severity. I have always found this confusing.
They got it right decades ago with [syslog(3)](https://www.man7.org/linux/man-pages/man3/syslog.3.html). Every log message has a facility and a severity.
...
(https://github.com/bitcoin/bitcoin/pull/29256#pullrequestreview-1884614153)
> Make them all accept log categories, to make it possible to filter or highlight log messages from a particular component.
This seems like a step in the right direction. Currently we log messages unconditionally that don't have a category. But this is mixing two separate things - facility and severity. I have always found this confusing.
They got it right decades ago with [syslog(3)](https://www.man7.org/linux/man-pages/man3/syslog.3.html). Every log message has a facility and a severity.
...
💬 vasild commented on issue "Gathering Priorities of 28.0":
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1948001052)
Broadcast own transactions only via short-lived Tor or I2P connections https://github.com/bitcoin/bitcoin/pull/29415
(https://github.com/bitcoin/bitcoin/issues/29439#issuecomment-1948001052)
Broadcast own transactions only via short-lived Tor or I2P connections https://github.com/bitcoin/bitcoin/pull/29415
💬 bigspider commented on pull request "Reenable OP_CAT":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-1948016168)
> **Rationale:** It explicitly disables open ended second order effects by removing the ability to assemble a SIGHASH or CTV template on the stack for more detailed introspection than intended. If such introspection behavior is desired in the future it can be explicitly enabled by specialized and more efficient opcodes.
Script is already expressive enough to (awkwardly, expensively) compute arbitrary SHA256 hashes on the stack, except that the result would be broken in smaller pieces of at mo
...
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-1948016168)
> **Rationale:** It explicitly disables open ended second order effects by removing the ability to assemble a SIGHASH or CTV template on the stack for more detailed introspection than intended. If such introspection behavior is desired in the future it can be explicitly enabled by specialized and more efficient opcodes.
Script is already expressive enough to (awkwardly, expensively) compute arbitrary SHA256 hashes on the stack, except that the result would be broken in smaller pieces of at mo
...
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1948024853)
Updated 75931055b84d1f0e9f44d3d3efc28785fd23e66c -> 316c7c845036cbffa22b9e44f31dca8573ffb639 ([noGlobalSignals_17](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_17) -> [noGlobalSignals_18](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_18), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_17..noGlobalSignals_18))
* Addressed @furszy's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1491488517), correcting docstring.
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1948024853)
Updated 75931055b84d1f0e9f44d3d3efc28785fd23e66c -> 316c7c845036cbffa22b9e44f31dca8573ffb639 ([noGlobalSignals_17](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_17) -> [noGlobalSignals_18](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_18), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_17..noGlobalSignals_18))
* Addressed @furszy's [comment](https://github.com/bitcoin/bitcoin/pull/28960#discussion_r1491488517), correcting docstring.
🤔 naumenkogs requested changes to a pull request: "net: attempts to connect to all resolved addresses when connecting to a node"
(https://github.com/bitcoin/bitcoin/pull/28834#pullrequestreview-1884670140)
Mostly looks good, let me know what you think about the suggestions.
(https://github.com/bitcoin/bitcoin/pull/28834#pullrequestreview-1884670140)
Mostly looks good, let me know what you think about the suggestions.
💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492195357)
nit: would be nice to say `XYZ (pszDest) is provided` instead of `pszDest is provided`
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492195357)
nit: would be nice to say `XYZ (pszDest) is provided` instead of `pszDest is provided`
💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492202202)
nit:
I'd suggest to clarify the commit message saying that "a valid one is found" refers to what exactly (getting up to here and `connected = true`).
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492202202)
nit:
I'd suggest to clarify the commit message saying that "a valid one is found" refers to what exactly (getting up to here and `connected = true`).
💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492192396)
nit: do we still need `Shuffle(resolved.begin(), resolved.end(), FastRandomContext());`? It certainly doesn't achieve the same goal as before... I'd suggest either dropping it altogether, or document this new purpose (either inline or in the commit message).
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492192396)
nit: do we still need `Shuffle(resolved.begin(), resolved.end(), FastRandomContext());`? It certainly doesn't achieve the same goal as before... I'd suggest either dropping it altogether, or document this new purpose (either inline or in the commit message).
💬 naumenkogs commented on pull request "net: attempts to connect to all resolved addresses when connecting to a node":
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492205690)
nit: can you add a comment to justify early termination here (instead of trying going through other addrs)
(https://github.com/bitcoin/bitcoin/pull/28834#discussion_r1492205690)
nit: can you add a comment to justify early termination here (instead of trying going through other addrs)
💬 Sjors commented on pull request "Improve display address handling for external signer":
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1948087608)
Tested with HWI 2.4.0
When a Ledger Nano X is in screen saver mode, I now get the following error:
```
'...hwi' error: Could not open client or get fingerprint information: ('0x5515', 'Error in <DefaultInsType.GET_VERSION: 1> command', '')
```
That's what HWI returns, so can't make it much better...
When the Bitcoin app is not opened on the device the error is more clear:
```
'...hwi' error: Could not open client or get fingerprint information: Ledger is not in either the Bitco
...
(https://github.com/bitcoin/bitcoin/pull/24313#issuecomment-1948087608)
Tested with HWI 2.4.0
When a Ledger Nano X is in screen saver mode, I now get the following error:
```
'...hwi' error: Could not open client or get fingerprint information: ('0x5515', 'Error in <DefaultInsType.GET_VERSION: 1> command', '')
```
That's what HWI returns, so can't make it much better...
When the Bitcoin app is not opened on the device the error is more clear:
```
'...hwi' error: Could not open client or get fingerprint information: Ledger is not in either the Bitco
...
💬 Sjors commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1948108652)
> Also please note that I find the announcement period super short.
Removal happens at some unknown time after deprecation, but from what I can tell, not before the 28.0 release (~November 2024). The v27.* releases will be maintained for a while longer too: https://bitcoincore.org/en/lifecycle/
> the child transaction must be verified before signing their parents
What exactly do you mean by "verified"? Are you checking proof-of-work from the headers? The signature?
I'm not familiar e
...
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1948108652)
> Also please note that I find the announcement period super short.
Removal happens at some unknown time after deprecation, but from what I can tell, not before the 28.0 release (~November 2024). The v27.* releases will be maintained for a while longer too: https://bitcoincore.org/en/lifecycle/
> the child transaction must be verified before signing their parents
What exactly do you mean by "verified"? Are you checking proof-of-work from the headers? The signature?
I'm not familiar e
...
💬 maflcko commented on pull request "fuzz: Generate with random libFuzzer settings":
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1948169568)
cc @murchandamus You may be interested in the rss_limit_mb (last commit). Not sure if you set it in your fuzz script, or if you ever ran into OOM.
(https://github.com/bitcoin/bitcoin/pull/28178#issuecomment-1948169568)
cc @murchandamus You may be interested in the rss_limit_mb (last commit). Not sure if you set it in your fuzz script, or if you ever ran into OOM.
💬 vasild commented on issue "Clang 14 emits `-Wunreachable-code` warnings":
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948185285)
The code in question can be improved:
```cpp
#ifdef USE_BDB
if constexpr (true) {
return MakeBerkeleyDatabase(path, options, status, error);
} else
#endif
{
error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
status = DatabaseStatus::FAILED_BAD_FORMAT;
return nullptr;
}
```
If `USE_BDB` is defined then this becomes:
```cpp
if c
...
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948185285)
The code in question can be improved:
```cpp
#ifdef USE_BDB
if constexpr (true) {
return MakeBerkeleyDatabase(path, options, status, error);
} else
#endif
{
error = Untranslated(strprintf("Failed to open database path '%s'. Build does not support Berkeley DB database format.", fs::PathToString(path)));
status = DatabaseStatus::FAILED_BAD_FORMAT;
return nullptr;
}
```
If `USE_BDB` is defined then this becomes:
```cpp
if c
...
💬 maflcko commented on issue "Clang 14 emits `-Wunreachable-code` warnings":
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948192976)
Sure, but that is simply a revert of the commit mentioned in the description. Thus, the code would no longer be compiled unconditionally, and compile errors may be missed.
(https://github.com/bitcoin/bitcoin/issues/29334#issuecomment-1948192976)
Sure, but that is simply a revert of the commit mentioned in the description. Thus, the code would no longer be compiled unconditionally, and compile errors may be missed.
🤔 hebasto reviewed a pull request: "lint: Check for missing bitcoin-config.h includes"
(https://github.com/bitcoin/bitcoin/pull/29408#pullrequestreview-1884987863)
Tested 48562917172accd911e4c86f1032e4282fd321f5 on Ubuntu 23.10.
Missing `#include <config/bitcoin-config.h>` are reported correctly.
What are we supposed to do to automatically avoid redundant / unneeded includes?
(https://github.com/bitcoin/bitcoin/pull/29408#pullrequestreview-1884987863)
Tested 48562917172accd911e4c86f1032e4282fd321f5 on Ubuntu 23.10.
Missing `#include <config/bitcoin-config.h>` are reported correctly.
What are we supposed to do to automatically avoid redundant / unneeded includes?