👍 paplorinc approved a pull request: "refactor: Model the bech32 charlimit as an Enum"
(https://github.com/bitcoin/bitcoin/pull/30047#pullrequestreview-2052206587)
nice!

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598184096)
nit: to be consistent with the casing below
```suggestion
/** Character limits for Bech32(m) encoded strings. Character limits are how we provide error location guarantees.
```

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598185043)
nit:
```suggestion
BECH32 = 90, //!< BIP173/350 imposed a 90 character limit on Bech32(m) encoded addresses. This guarantees finding up to 4 errors.
```

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598199292)
Keeping it present tense feels more correct, perhaps:

```suggestion
BECH32 = 90, //!< BIP173/350 imposed character limit for Bech32(m) encoded addresses. This guarantees finding up to 4 errors.
```
?

📝 paplorinc opened a pull request: "Optimize memory allocation for transaction outputs"
(https://github.com/bitcoin/bitcoin/pull/30093)
Added a reserve operation to `txNew.vout` to pre-allocate memory based on the expected number of transaction outputs.

Split out of https://github.com/bitcoin/bitcoin/pull/30050/files#r1597631104

🤔 TheCharlatan reviewed a pull request: "fuzz: a new target for the coins database"
(https://github.com/bitcoin/bitcoin/pull/28216#pullrequestreview-2052152342)
lgtm, but I think it would be good to rebase this so the CI can run through it again.

💬 TheCharlatan commented on pull request "fuzz: a new target for the coins database":
(https://github.com/bitcoin/bitcoin/pull/28216#discussion_r1598150932)
Unrelated to this PR, but is there a reason this is using the full testing setup?
```diff
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 8f3e357a84..c7563ae9f4 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -30 +30 @@ namespace {
-const TestingSetup* g_setup;
+const BasicTestingSetup* g_setup;
@@ -42 +42 @@ void initialize_coins_view()
- static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>();
+
...

💬 paplorinc commented on pull request "refactor, wallet: get serialized size of `CRecipient`s directly":
(https://github.com/bitcoin/bitcoin/pull/30050#discussion_r1598202690)
Split it out to a new PR so that we don't forget it: https://github.com/bitcoin/bitcoin/pull/30093/files

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598204445)
Even better!

💬 Sjors commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598210040)
Filtering < 546 dust makes no difference on the regular index size (https://github.com/bitcoin/bitcoin/commit/411f035f0e09142311b6a829490f175582b07096)

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598216013)
> Filtering < 546 dust makes no difference on the regular index size (unless I did something wrong in [411f035](https://github.com/bitcoin/bitcoin/commit/411f035f0e09142311b6a829490f175582b07096))

That's interesting you could compare with [Blindbit Oracle](https://github.com/setavenger/blindbit-oracle). There is an exposed endpoint that filters based on a defined dust limit. There definitely is a difference at ~1000. I'm not sure whether I've tested <546 yet.

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598219770)
Updated.

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#discussion_r1598219971)
Fixed.

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2107169922)
Updated comment per @paplorinc 's suggestions.

💬 josibake commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598222539)
Not super surprising since this is a standardness limit, no? I don't think you can broadcast transactions without the output amount being greater than the dust limit (unless it is an `OP_RETURN`).

💬 setavenger commented on pull request "Silent payment index (for light wallets and consistency check)":
(https://github.com/bitcoin/bitcoin/pull/28241#discussion_r1598234205)
I did an [analysis](https://github.com/setavenger/BIP0352-light-client-specification?tab=readme-ov-file#dust-limits) a while back which revealed that 10% of taproot UTXOs are <= 330. I'm not sure about the details but I believe it was the entire UTXO set collected by the indexer up to 834761. At the time it collected the entire set and not only SP-eligible UTXOs.

I guess it's technically possible that either none of those 10% were in eligible transactions and/or they were combined with a bigg
...

💬 josibake commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2107197805)
@paplorinc I noticed you approved the PR, but we don't really use github's approval/review flow for determining the review status of a PR.

Can you instead leave an `ACK <commit>` message per https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#code-review ? This way, intent is clear to the maintainers and DrahtBot can track the status and update the summary comment.

🤔 glozow reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2050218566)
some test review

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1596843725)
I don't really understand what's being tested here - this doesn't have anything to do with package RBF? This isn't a topologically valid package since the transactions aren't related to each other in addition to being double spends. #30066 seems closer to what this is going for?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1596837120)
nit: you change the fee in the last successful one, which is a little bit sus since fee shouldn't matter in this test

Maybe create 2 constants for the parent `fee_per_output` and child fee/feerate, and use them for each round?