💬 fanquake commented on pull request "build: Bump minimum supported macOS to 13.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2416412158)
Guix Build
```bash
d454a81e02e24f26f4d163f27ee2b16b83c8f1dfcfe0384ab544b704e27fed09 guix-build-a0e089a71dc4/output/aarch64-linux-gnu/SHA256SUMS.part
f5b3771bbb5e2c14e6d91d6a76711df0a9b8ab1b66e118eb6c5aba5686c91902 guix-build-a0e089a71dc4/output/aarch64-linux-gnu/bitcoin-a0e089a71dc4-aarch64-linux-gnu-debug.tar.gz
229f0eef5808a7266ac48b9b02dd84b7a7960353125ed7052510a8014074c0da guix-build-a0e089a71dc4/output/aarch64-linux-gnu/bitcoin-a0e089a71dc4-aarch64-linux-gnu.tar.gz
0dc7197f0284cf783
...

💬 instagibbs commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1802841920)
I'd rather not; we almost immediately handle singletons via `AcceptSubPackage` internally, I'm not convinced this is more risky than adding more complexity on top.

💬 maflcko commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1802848071)
Thanks, clarified the name, because it wasn't in a namespace itself (only the global one).

🤔 sdaftuar reviewed a pull request: "Ephemeral Dust"
(https://github.com/bitcoin/bitcoin/pull/30239#pullrequestreview-2368652846)
Read through the code (except for the fuzz test, which I plan to go back and review). Concept ACK.

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1800758237)
Do we need this `assert()`? Can we just do an`Assume()` instead?

My general philosophy is that we should avoid calling `assert()` in codepaths that run on data received from the network, unless it really would be better for (potentially) the ENTIRE network to go down rather than just continue, in the event that the assert() can be remotely triggered.

I see that we might crash further down if we somehow there is a nullptr here, so my suggestion for error handling would be to `Assume()` eve
...

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1800930436)
I don't think it makes sense to gate this on `!bypass_limits` -- doing so only has an effect for transactions that spend a non-dust output that are coming from the same block as a transaction that creates a dust output, which seems like both (a) a very small effect that (b) we don't obviously want to do...

Specifically, it seems strange that we would only accept a transaction that was reorged out of a block that has a dust output if it has exactly 1 such dust output and exactly 0 fee, yet we
...

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1802693519)
nit: s/spend/spent/ ?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1800908033)
I would love to merge the codepaths for the package and non-package case, so that we have no concerns over whether all the right checks have been run. What if we did something like this:

- Add all passed-in transactions to a map: Txid -> CTransactionRef. This will allow us to look up transactions either locally (from a package) or in the mempool.

- For every transaction passed in:
* For every input in the transaction:
- Look up the input in the local map. If not found, look up in
...

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1802744124)
Can we assert that this transaction is different from the one already in the mempool?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1802833358)
Did you mean `const CTransactionRef& tx_ref` here?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1802727338)
Just want to make sure I understand this comment, does "any size" refer to the value in the output?

💬 sdaftuar commented on pull request "Ephemeral Dust":
(https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1802745380)
Might be nice to assert that this transaction has a higher feerate than the one in the mempool, just to be sure that this would be a successful RBF if not for the ephemeral dust issue.

💬 dergoegge commented on issue "Disallow building fuzz binary without `-DBUILD_FOR_FUZZING`":
(https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2416431400)
I opened #31093, implementing the suggestion from https://github.com/bitcoin/bitcoin/issues/31057#issuecomment-2400559220.

💬 stickies-v commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416432329)
> Using RPC requires an authenticated connection.

Could you please elaborate on why RPC or authentication is not a feasible option for your use case?

🤔 stickies-v reviewed a pull request: "rest: Support transaction broadcast in REST interface"
(https://github.com/bitcoin/bitcoin/pull/31065#pullrequestreview-2372023562)
I'd prefer not extending the REST interface if we don't have to. This functionality is already well supported in the RPC, so this seems like purely a convenience endpoint?

I've asked for clarification in https://github.com/bitcoin/bitcoin/issues/31017, but until there's a good reason to add this I'm Concept NACK.

💬 glozow commented on pull request "Package validation: accept packages of size 1":
(https://github.com/bitcoin/bitcoin/pull/31096#discussion_r1802861498)
This is what testmempoolaccept does

🤔 instagibbs reviewed a pull request: "wallet: BIP 326 sequence based anti-fee-snipe for taproot inputs"
(https://github.com/bitcoin/bitcoin/pull/24128#pullrequestreview-2372024041)
concept ACK, appears to adhere to the BIP on first glance

will do deeper review if tests are applied

💬 maflcko commented on pull request "fees: Remove CLIENT_VERSION serialization":
(https://github.com/bitcoin/bitcoin/pull/29702#discussion_r1802863026)
Either should be fine, because it just ends up in the debug log (not really for end-users). What about:

```cpp
throw std::runtime_error(strprintf("File version (%d) too high to be read.", nVersionRequired));
```

if `up-version` is too vague?

💬 maflcko commented on issue "Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/issues/31017#issuecomment-2416454459)
> > Using RPC requires an authenticated connection.
>
> Could you please elaborate on why RPC or authentication is not a feasible option for your use case?

I am not familiar with electrum protocol, but the issue description says:

> An indexer like the ones for the electrum protocol should work without having access to the RPC interface

💬 maflcko commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2416456307)
I don't think the REST interface will be removed any time soon, or ever, so if users want this, it seems fine to add. But no strong opinion.