💬 ajtowns commented on pull request "rpc: allow submitpackage to be called outside of regtest":
(https://github.com/bitcoin/bitcoin/pull/27609#issuecomment-1586532229)
> > without #26711 we are still letting things into mempool that are possibly problematic, no?
>
> You're right. Every time we think about it a bit longer, another not-quite-ideal edge case comes up. Especially with full mempools and eviction being something to consider.

I think this should have remained as a separate PR, and should have been okay to merge as-is, rather than adding only loosely-related commits to #26711. Two reasons:

- after #26933 this doesn't allow any problematic be
...

💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226035519)
Should probably mark it as EXPERIMENTAL in the rpc docs -- see "sendall" eg in wallet/rpc/spend.cpp

Not really sure this should be advertised in the release notes at this stage at all, though.

💬 ajtowns commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1226063546)
This check doesn't feel like it makes sense to me -- it should either be unnecessary (because dealing with subpackages first takes care of it), or else it seems insufficient (because a grandparent might be paying for parent and child, and child paying for parent; but grandparents alone are good enough, but after they're accepted parent and child combined aren't good enough).

💬 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
...

📝 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
...

✅ fanquake closed a pull request: "Update config.yml Dennis Louis Babcock jr added to Statoshinakomoto"
(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
...

👍 TheCharlatan approved a pull request: "Return EXIT_FAILURE on post-init fatal errors"
(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.

💬 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.

🚀 fanquake merged a pull request: "ci: Nuke Android APK task, Use credits for tsan"
(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

🤔 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

💬 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.

🚀 fanquake merged a pull request: "contrib: docs fix --import-keys flag on verify.py"
(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)

👍 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

💬 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.

💬 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!

💬 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".