💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226067022)
I worry that this approach can cause O(n^2) validation costs in some way, if you've got n txs and end up doing work that results in multiple soft-rejects for each tx.
I wonder if it wouldn't be better to do something more like:
* topologically sort the package
* accept each tx in order into a mem-pond -- a special temporary mini pool just for this package, that enforces consensus rules, but doesn't care about minimum fees
* any txs that weren't valid obviously fail at this point and a
...
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226067022)
I worry that this approach can cause O(n^2) validation costs in some way, if you've got n txs and end up doing work that results in multiple soft-rejects for each tx.
I wonder if it wouldn't be better to do something more like:
* topologically sort the package
* accept each tx in order into a mem-pond -- a special temporary mini pool just for this package, that enforces consensus rules, but doesn't care about minimum fees
* any txs that weren't valid obviously fail at this point and a
...
📝 Lildeebo2002 opened a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
✅ fanquake closed a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(https://github.com/bitcoin/bitcoin/pull/27858)
(https://github.com/bitcoin/bitcoin/pull/27858)
📝 fanquake locked a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
(https://github.com/bitcoin/bitcoin/pull/27858)
[Dennis Louis Babcock ](statoshinakomotod@gmail.com) Added creator name Dennis Louis Babcock jr Statoshinakomoto
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user
...
👍 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?)