π¬ ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1194002643)
In commit "refactor: Pass FatalError in options to ChainstateManager" (2f9c2d245360b3fad6d469a76c2916d75b027b57)
I think this signature should be cleaned up and not be added to kernel like this. Would suggest:
- Changing `bool` return type to `void`. The return value is never used and the only value that is ever returned is false. Having the return value makes the interface confusing because it's unclear how the value even could be used.
- Renaming `strMessage` parameter to `debug_message
...
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1194002643)
In commit "refactor: Pass FatalError in options to ChainstateManager" (2f9c2d245360b3fad6d469a76c2916d75b027b57)
I think this signature should be cleaned up and not be added to kernel like this. Would suggest:
- Changing `bool` return type to `void`. The return value is never used and the only value that is ever returned is false. Having the return value makes the interface confusing because it's unclear how the value even could be used.
- Renaming `strMessage` parameter to `debug_message
...
π¬ ryanofsky commented on pull request "kernel: Remove interface_ui, util/system from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193969953)
In commit "refactor: Pass DoWarning in options to ChainstateManager" (bcd417ad1847cd4c93bffbc9d8a32255e7f2b156)
Behavior isn't changing here, but it seems strange that `fWarned` check is only letting the warning notification to be sent and the GUI progress bar to be updated once over the lifetime of the application. May be something to look into in a followup.
(https://github.com/bitcoin/bitcoin/pull/27636#discussion_r1193969953)
In commit "refactor: Pass DoWarning in options to ChainstateManager" (bcd417ad1847cd4c93bffbc9d8a32255e7f2b156)
Behavior isn't changing here, but it seems strange that `fWarned` check is only letting the warning notification to be sent and the GUI progress bar to be updated once over the lifetime of the application. May be something to look into in a followup.
:lock: fanquake locked an issue: "v0.6.5 on iPad mini 6 Security Center button incorrent"
(https://github.com/bitcoin/bitcoin/issues/27654)
(https://github.com/bitcoin/bitcoin/issues/27654)
:lock: fanquake locked an issue: "Mine bitcoin using FPGA"
(https://github.com/bitcoin/bitcoin/issues/27650)
(https://github.com/bitcoin/bitcoin/issues/27650)
:lock: fanquake locked an issue: "React-native. const { STATUS_CODES } = require('http'); Unable to resolve module http"
(https://github.com/bitcoin/bitcoin/issues/27644)
(https://github.com/bitcoin/bitcoin/issues/27644)
:lock: fanquake locked an issue: "Keep it simple"
(https://github.com/bitcoin/bitcoin/issues/27618)
(https://github.com/bitcoin/bitcoin/issues/27618)
:lock: fanquake locked an issue: "BIP 39 words update"
(https://github.com/bitcoin/bitcoin/issues/27649)
(https://github.com/bitcoin/bitcoin/issues/27649)
:lock: fanquake locked an issue: "."
(https://github.com/bitcoin/bitcoin/issues/27567)
(https://github.com/bitcoin/bitcoin/issues/27567)
π¬ Xekyo commented on pull request "Switch hardened derivation marker to h":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1194046865)
The βnorβ here seems misplaced:
```suggestion
For legacy wallets the `hdkeypath` field in `getaddressinfo`
and the serialization format of wallet dumps remain unchanged. (#26076)
```
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1194046865)
The βnorβ here seems misplaced:
```suggestion
For legacy wallets the `hdkeypath` field in `getaddressinfo`
and the serialization format of wallet dumps remain unchanged. (#26076)
```
π¬ stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1548177296)
so sorry for the delay and thank you for the suggestions! the CI is all green now. i've updated the PR to include:
- addrman new/tried/total count for "all networks"
- test coverage for "all networks" and for total (total refers to both new/tried table count)
I'd want this RPC to be the alternative way to access the actual count of all addresses in the addrman.
(https://github.com/bitcoin/bitcoin/pull/27511#issuecomment-1548177296)
so sorry for the delay and thank you for the suggestions! the CI is all green now. i've updated the PR to include:
- addrman new/tried/total count for "all networks"
- test coverage for "all networks" and for total (total refers to both new/tried table count)
I'd want this RPC to be the alternative way to access the actual count of all addresses in the addrman.
π¬ instagibbs commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548180934)
@glozow ok so the tree thing is a bandaid in lieu of #26711 , and you'd rather just to that
@joostjager
> If I understood the various threads here and in other prs correctly, I think the consequence of this is that the mempool will be sub-optimal and lead to a block template that earns fewer fees for the miner exposing the endpoint?
Also can have weird effects like your mempool min rate going higher than it should normally.
I think we can easily get #26711 in next release
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548180934)
@glozow ok so the tree thing is a bandaid in lieu of #26711 , and you'd rather just to that
@joostjager
> If I understood the various threads here and in other prs correctly, I think the consequence of this is that the mempool will be sub-optimal and lead to a block template that earns fewer fees for the miner exposing the endpoint?
Also can have weird effects like your mempool min rate going higher than it should normally.
I think we can easily get #26711 in next release
π¬ furszy commented on pull request "wallet: fix deadlock in bdb read write operation":
(https://github.com/bitcoin/bitcoin/pull/27556#issuecomment-1548189712)
rebased, conflicts solved.
(https://github.com/bitcoin/bitcoin/pull/27556#issuecomment-1548189712)
rebased, conflicts solved.
π¬ Sjors commented on pull request "Improve performance of p2p inv to send queues":
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1194086866)
@dergoegge `INVENTORY_BROADCAST_MAX` has been poorly named for a while. It applies to inbound peers, but to outbounds we send 5 / 2 more.
(https://github.com/bitcoin/bitcoin/pull/27610#discussion_r1194086866)
@dergoegge `INVENTORY_BROADCAST_MAX` has been poorly named for a while. It applies to inbound peers, but to outbounds we send 5 / 2 more.
π¬ brunoerg commented on pull request "fuzz: wallet, add target for `fees`":
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1548198822)
CI failure seems unrelated
(https://github.com/bitcoin/bitcoin/pull/27647#issuecomment-1548198822)
CI failure seems unrelated
π¬ joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548199779)
>Also can have weird effects like your mempool min rate going higher than it should normally.
Is this something that #26711 prevents? Because it seems to me that that PR is less restrictive than the tree-only allowance in this PR.
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548199779)
>Also can have weird effects like your mempool min rate going higher than it should normally.
Is this something that #26711 prevents? Because it seems to me that that PR is less restrictive than the tree-only allowance in this PR.
π¬ instagibbs commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548203219)
To be clear, the tree structure in this PR "should" also stop it.
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548203219)
To be clear, the tree structure in this PR "should" also stop it.
π¬ joostjager commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548207594)
By the way, about the diamond example in https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542695316
This isn't a "child with parents" package and also wouldn't be accepted on regtest with the current master branch because the child has no direct link to the grandparent? (https://github.com/bitcoin/bitcoin/blob/d7700d3a26478d9b1648463c188648c7047b1c60/src/policy/packages.cpp#L68)
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548207594)
By the way, about the diamond example in https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542695316
This isn't a "child with parents" package and also wouldn't be accepted on regtest with the current master branch because the child has no direct link to the grandparent? (https://github.com/bitcoin/bitcoin/blob/d7700d3a26478d9b1648463c188648c7047b1c60/src/policy/packages.cpp#L68)
π¬ brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1194103566)
Make sense, going to address it.
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1194103566)
Make sense, going to address it.
π¬ glozow commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548213087)
> By the way, about the diamond example in https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542695316
> This isn't a "child with parents" package
Sorry, that was a bad diagram (of the unit test I had added). The child is also spending the grandparent. I've updated the comment now.
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1548213087)
> By the way, about the diamond example in https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1542695316
> This isn't a "child with parents" package
Sorry, that was a bad diagram (of the unit test I had added). The child is also spending the grandparent. I've updated the comment now.
π¬ stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1194105588)
noted. not convinced how desirable the additional code complexity is. would like to hear how other people think about code complexity too before deciding what to do next.
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1194105588)
noted. not convinced how desirable the additional code complexity is. would like to hear how other people think about code complexity too before deciding what to do next.
π¬ stratospher commented on pull request "cli: rework -addrinfo cli to use addresses which arenβt filtered for quality/recency":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1548215577)
Rebased. Looking for feedback on [this comment](https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173966799).
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1548215577)
Rebased. Looking for feedback on [this comment](https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1173966799).