👍 TheCharlatan approved a pull request: "Return EXIT_FAILURE on post-init fatal errors"
(https://github.com/bitcoin/bitcoin/pull/27708#pullrequestreview-1474247182)
ACK 61c569ab6069d04079a0831468eb713983919636
(https://github.com/bitcoin/bitcoin/pull/27708#pullrequestreview-1474247182)
ACK 61c569ab6069d04079a0831468eb713983919636
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1226247485)
This will set `WALLET_FLAG_GLOBAL_HD_KEY` for non-descriptor wallets as well, which is a minor bug in my opinion.
I think we should set the flag in `CreateWallet` function in the same place we force sqlite for descriptor wallets. This line begs the question why `wallet_creation_flags` doesn't have the `WALLET_FLAG_GLOBAL_HD_KEY` set already.
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1226247485)
This will set `WALLET_FLAG_GLOBAL_HD_KEY` for non-descriptor wallets as well, which is a minor bug in my opinion.
I think we should set the flag in `CreateWallet` function in the same place we force sqlite for descriptor wallets. This line begs the question why `wallet_creation_flags` doesn't have the `WALLET_FLAG_GLOBAL_HD_KEY` set already.
💬 fanquake commented on pull request "[DEMO] Integrate `bitcoin-tidy` clang-tidy plugin - warn on uninitialized members of aggregate types":
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1586840400)
Rebased, dropped the no-longer needed load patch. Will swap out for the log lint plugin when it's ready.
(https://github.com/bitcoin/bitcoin/pull/26296#issuecomment-1586840400)
Rebased, dropped the no-longer needed load patch. Will swap out for the log lint plugin when it's ready.
🚀 fanquake merged a pull request: "ci: Nuke Android APK task, Use credits for tsan"
(https://github.com/bitcoin/bitcoin/pull/27834)
(https://github.com/bitcoin/bitcoin/pull/27834)
🤔 glozow reviewed a pull request: "p2p: Stop relaying non-mempool txs"
(https://github.com/bitcoin/bitcoin/pull/27625#pullrequestreview-1474386514)
code review ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4
(https://github.com/bitcoin/bitcoin/pull/27625#pullrequestreview-1474386514)
code review ACK faa2976a56ea7cdfd77ce2580a89ce493b57b5d4
🤔 glozow reviewed a pull request: "fuzz: Fix mini_miner_selection running out of coin"
(https://github.com/bitcoin/bitcoin/pull/27806#pullrequestreview-1474388954)
ACK de5d8626184f6189d07137ca935da8703b8a78a3
(https://github.com/bitcoin/bitcoin/pull/27806#pullrequestreview-1474388954)
ACK de5d8626184f6189d07137ca935da8703b8a78a3
💬 fanquake commented on pull request "contrib: docs fix --import-keys flag on verify.py":
(https://github.com/bitcoin/bitcoin/pull/27840#issuecomment-1586879040)
@Tguntenaar note that I've edited your comment here, to avoid an @ mention in the commit message.
(https://github.com/bitcoin/bitcoin/pull/27840#issuecomment-1586879040)
@Tguntenaar note that I've edited your comment here, to avoid an @ mention in the commit message.
🚀 fanquake merged a pull request: "contrib: docs fix --import-keys flag on verify.py"
(https://github.com/bitcoin/bitcoin/pull/27840)
(https://github.com/bitcoin/bitcoin/pull/27840)
🚀 fanquake merged a pull request: "ci: Use podman stop over podman kill"
(https://github.com/bitcoin/bitcoin/pull/27844)
(https://github.com/bitcoin/bitcoin/pull/27844)
👍 dergoegge approved a pull request: "fuzz: Fix mini_miner_selection running out of coin"
(https://github.com/bitcoin/bitcoin/pull/27806#pullrequestreview-1474402151)
Code review ACK de5d8626184f6189d07137ca935da8703b8a78a3
(https://github.com/bitcoin/bitcoin/pull/27806#pullrequestreview-1474402151)
Code review ACK de5d8626184f6189d07137ca935da8703b8a78a3
💬 MarcoFalke commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#issuecomment-1586889040)
I presume this is a fix for https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59385, which was closed unexpectedly and incorrectly by OSS-Fuzz. Also, the issue should ideally reproduce outside of msan.
To fix both, I've just enabled `-D_LIBCPP_ENABLE_ASSERTIONS=1` on OSS-Fuzz, so I'd recommend to wait 2-3 days before merging this to give OSS-Fuzz a final chance to re-detect this issue.
(https://github.com/bitcoin/bitcoin/pull/27806#issuecomment-1586889040)
I presume this is a fix for https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=59385, which was closed unexpectedly and incorrectly by OSS-Fuzz. Also, the issue should ideally reproduce outside of msan.
To fix both, I've just enabled `-D_LIBCPP_ENABLE_ASSERTIONS=1` on OSS-Fuzz, so I'd recommend to wait 2-3 days before merging this to give OSS-Fuzz a final chance to re-detect this issue.
💬 glozow commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#issuecomment-1586893382)
> To fix both, I've just enabled -D_LIBCPP_ENABLE_ASSERTIONS=1 on OSS-Fuzz, so I'd recommend to wait 2-3 days before merging this to give OSS-Fuzz a final chance to re-detect this issue.
Noted, thanks!
(https://github.com/bitcoin/bitcoin/pull/27806#issuecomment-1586893382)
> To fix both, I've just enabled -D_LIBCPP_ENABLE_ASSERTIONS=1 on OSS-Fuzz, so I'd recommend to wait 2-3 days before merging this to give OSS-Fuzz a final chance to re-detect this issue.
Noted, thanks!
💬 MarcoFalke commented on pull request "fuzz: Fix mini_miner_selection running out of coin":
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1226328291)
Any reason this is set to `3`, or previously to `2`, when it could be `1` to allow for transactions with one output?
I don't think we should reduce test coverage with the rationale "belt-and-suspenders".
(https://github.com/bitcoin/bitcoin/pull/27806#discussion_r1226328291)
Any reason this is set to `3`, or previously to `2`, when it could be `1` to allow for transactions with one output?
I don't think we should reduce test coverage with the rationale "belt-and-suspenders".
📝 ismaelsadeeq opened a pull request: "Mempool: persist mempoolminfee accross restarts"
(https://github.com/bitcoin/bitcoin/pull/27859)
Follow-up [#27622](https://github.com/bitcoin/bitcoin/pull/27622) depends on [#27622](https://github.com/bitcoin/bitcoin/pull/27622)
See –> [Comment](https://github.com/bitcoin/bitcoin/issues/27555#issuecomment-1538504504)
Currently, there is no persistence of mempoolminfee
As mentioned in the comment
> persist `mempoolminfee` across restarts to at least tell user a plausible value to stay in the mempool for a bit
The suggestions outlined in the comment are implemented in this pull
...
(https://github.com/bitcoin/bitcoin/pull/27859)
Follow-up [#27622](https://github.com/bitcoin/bitcoin/pull/27622) depends on [#27622](https://github.com/bitcoin/bitcoin/pull/27622)
See –> [Comment](https://github.com/bitcoin/bitcoin/issues/27555#issuecomment-1538504504)
Currently, there is no persistence of mempoolminfee
As mentioned in the comment
> persist `mempoolminfee` across restarts to at least tell user a plausible value to stay in the mempool for a bit
The suggestions outlined in the comment are implemented in this pull
...
💬 MarcoFalke commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1226387994)
Might be better to write this to `mempool.dat`, or is there a benefit to create a new file?
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1226387994)
Might be better to write this to `mempool.dat`, or is there a benefit to create a new file?
💬 ajtowns commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1226384032)
Why add this as a separate file, rather than adding it to the end of the fee estimates data file which is already updated periodically and ignored if out of date?
Adding a method to the mempool to set the min fee based on that seems plausible? (Perhaps make it conditional on the new value being higher than the current minfee, and the mempool being less than 95% full?)
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1226384032)
Why add this as a separate file, rather than adding it to the end of the fee estimates data file which is already updated periodically and ignored if out of date?
Adding a method to the mempool to set the min fee based on that seems plausible? (Perhaps make it conditional on the new value being higher than the current minfee, and the mempool being less than 95% full?)
💬 ajtowns commented on pull request "Mempool: persist mempoolminfee accross restarts":
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1226391232)
The idea is to update it periodically, and writing the full mempool to disk regularly may be unappealing.
(https://github.com/bitcoin/bitcoin/pull/27859#discussion_r1226391232)
The idea is to update it periodically, and writing the full mempool to disk regularly may be unappealing.
🚀 fanquake merged a pull request: "p2p: Stop relaying non-mempool txs"
(https://github.com/bitcoin/bitcoin/pull/27625)
(https://github.com/bitcoin/bitcoin/pull/27625)
💬 dergoegge commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1586988190)
What are the reasons for not having this as a separate piece of software that calls the Bitcoin Core RPCs? From glancing at the code here it seems like `getblocktemplate` and `submitblock` would be the only needed RPCs. I've seen the discussion about separation (multi-process) in https://github.com/bitcoin/bitcoin/pull/23049 but wasn't convinced by the arguments there (e.g. https://github.com/bitcoin/bitcoin/pull/23049#issuecomment-926009122). I am worried about the maintenance burden of this an
...
(https://github.com/bitcoin/bitcoin/pull/27854#issuecomment-1586988190)
What are the reasons for not having this as a separate piece of software that calls the Bitcoin Core RPCs? From glancing at the code here it seems like `getblocktemplate` and `submitblock` would be the only needed RPCs. I've seen the discussion about separation (multi-process) in https://github.com/bitcoin/bitcoin/pull/23049 but wasn't convinced by the arguments there (e.g. https://github.com/bitcoin/bitcoin/pull/23049#issuecomment-926009122). I am worried about the maintenance burden of this an
...
💬 hebasto commented on pull request "Make `CCheckQueue` RAII-styled":
(https://github.com/bitcoin/bitcoin/pull/26762#issuecomment-1586988860)
Rebased b023c26eb8eda4c7f80ad2e7ebe1fb046e87d2ee -> e89273e80765219ff545ee5c26a257aef7d69f9c ([pr26762.10](https://github.com/hebasto/bitcoin/commits/pr26762.10) -> [pr26762.11](https://github.com/hebasto/bitcoin/commits/pr26762.11)) due to the conflict with #27576.
(https://github.com/bitcoin/bitcoin/pull/26762#issuecomment-1586988860)
Rebased b023c26eb8eda4c7f80ad2e7ebe1fb046e87d2ee -> e89273e80765219ff545ee5c26a257aef7d69f9c ([pr26762.10](https://github.com/hebasto/bitcoin/commits/pr26762.10) -> [pr26762.11](https://github.com/hebasto/bitcoin/commits/pr26762.11)) due to the conflict with #27576.