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

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598149576)
nit: this would be easier to read and more extensible in the future
```suggestion
const CFeeRate package_feerate(m_total_modified_fees, m_total_vsize);
```

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1596847344)
These 2 things are contradictory. AFAICT `fee_rate` is ignored, delete?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598137245)
This is now in `mempool_util` (= linter complaint)

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598246153)
with rebase
```suggestion
fill_mempool(self, self.nodes[0])
```

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598211388)
Given that `fee_delta` is a multiple of `DEFAULT_FEE` and you're using `fee_delta / 2` here... I think the general readability of this test would be improved if you created a constant base unit `FEE` which was equal to, say, `DEFAULT_FEE / 10` (or `104 * 10` which would give most of the transactions whole number feerates), and then used `FEE`, `2*FEE`, `8*FEE`, etc. in all the tests.

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598248191)
Maybe there should be some v3 test coverage (including nice 0fee parent package RBF)?
```
diff --git a/test/functional/mempool_accept_v3.py b/test/functional/mempool_accept_v3.py
index 8285b82c19..990d36359e 100755
--- a/test/functional/mempool_accept_v3.py
+++ b/test/functional/mempool_accept_v3.py
@@ -579,6 +579,32 @@ class MempoolAcceptV3(BitcoinTestFramework):
)
self.check_mempool([tx_with_multi_children["txid"], tx_with_sibling3_rbf["txid"], tx_with_sibling2["txid"
...

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1598170246)
Sorry to nit my own previous suggestion, but I think "parent paying for child replacement" might still be too specific, since it could be that the child has nothing to replace but is lower feerate (like `package_txns6` in the functional test).

Maybe
```suggestion
"package RBF failed: package feerate is less than parent feerate",
strprintf("package feerate %s <= parent feerate %s", package_feerate.ToString(), parent_f
...

💬 paplorinc commented on pull request "refactor: Model the bech32 charlimit as an Enum":
(https://github.com/bitcoin/bitcoin/pull/30047#issuecomment-2107220524)
ACK d196442c015c4cbf490751a650ecb3aa29321442

✅ maflcko closed an issue: "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb"
(https://github.com/bitcoin/bitcoin/issues/29909)

💬 maflcko commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2107249656)
Closing for now, due to inactivity, but the discussion can be continued regardless.