💬 1440000bytes commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141045136)
glozow how things works....

💬 glozow commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141047457)
> Rather I think this is a good idea because it has become more common that blocks contain these transactions

See PR description

💬 ryanofsky commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2246172625)
In commit "descriptor: Add CanSelfExpand()" (01b72fd4f5e5bcbbf851c217a895b09204a760a3)

I was wondering whether it's possible to provide a generic implementation of CanSelfExpand() to avoid needing all these specializations. Would the following work?

<details><summary>diff</summary>
<p>

```diff
--- a/src/script/descriptor.cpp
+++ b/src/script/descriptor.cpp
@@ -786,6 +786,16 @@ public:
}
}

+ bool CanSelfExpand() const override {
+ for (const auto& key :
...

🤔 w0xlt reviewed a pull request: "kernel: improve BlockChecked ownership semantics"
(https://github.com/bitcoin/bitcoin/pull/33078#pullrequestreview-3076606483)
ACK https://github.com/bitcoin/bitcoin/pull/33078/commits/742f2b54358a9b27f0d9a5a7304fc650caf8704d

💬 w0xlt commented on pull request "kernel: improve BlockChecked ownership semantics":
(https://github.com/bitcoin/bitcoin/pull/33078#discussion_r2246179740)
```suggestion
virtual void BlockChecked(const std::shared_ptr<const CBlock>, const BlockValidationState&) {}
```

🤔 stratospher reviewed a pull request: "qa: test that we do not disconnect a peer for submitting an invalid compact block"
(https://github.com/bitcoin/bitcoin/pull/33083#pullrequestreview-3076623531)
ACK c1574381168573c561ebddf1945d2debefb340f7.

nice coverage! I guess we had some coverage for it before #32646 since we used to call a different check in FillBlock().

💬 Retropex commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141117458)
NACK, without proper spam filters it will mostly be used by harmful spam.

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3141132363)
> I'm wondering if there's a different way we can go about this without adding another table of arguments that need special treatment.

I'm also not a fan of separate tables and suggested the following change to unify them earlier: b998cc52d51b48db9271fdba0bd69e9aaccb7999 ([tag](https://github.com/ryanofsky/bitcoin/commits/review.32821.4-edit.1)). This change is just a refactoring and could be a followup.

> Perhaps we could move named argument handling and string to json conversion server s
...

💬 stickies-v commented on pull request "kernel: improve BlockChecked ownership semantics":
(https://github.com/bitcoin/bitcoin/pull/33078#discussion_r2246254900)
What's the benefit of passing a `const std::shared_ptr`?

💬 jlopp commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141183635)
Concept ACK; this is effectively increasing economic scalability. If we can do so without reopening DoS vectors, that's a win.

💬 caesrcd commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141192328)
Concept ACK

The economic incentives are already evident: over 80% of the hashrate is mining transactions below 1 sat/vB, and the number of noderunners relaying transactions with fees below the current standard is increasing every day.

👍 pablomartin4btc approved a pull request: "test: Perform backup filename checks in migrate_and_get_rpc in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3076775979)
utACK 4b80147feb97300e92e1f940b8d989a0af331e06

💬 luke-jr commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141253053)
Again, Concept NACK. Everything below 1s/vB is spam. There's no reason to change the default.

>Over that period the USD price of BTC has risen by roughly 2-3 orders of magnitude,

It's the USD that has fallen, Bitcoin has only increased *relative to* it. Actually, Bitcoin probably hasn't even kept up its value, so if anything we should be looking to increase the default relay fee, if maintaining the same actual-value cost is the goal.

>The minimum relay feerate is a DoS protection rule,
...

💬 w0xlt commented on pull request "kernel: improve BlockChecked ownership semantics":
(https://github.com/bitcoin/bitcoin/pull/33078#discussion_r2246327956)
Mostly for clarity and intent (with this change, cannot reassign or mutate the local shared_ptr copy). Other than that, no real effect.
It is a nit, not blocking.

💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3141265710)
> because the current syntax for distinguishing named parameters is inherently ambiguous.

But since the server knows the names of the named parameters, it can also check whether the incoming string starts with the name of a parameter.

💬 w0xlt commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141277334)
Concept ACK

💬 mzumsande commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2246301270)
(source location is unrelated, just wanted to get into thread mode)
I wanted to add something to this [this thread](https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2115526293).

I didn't review the original PR, but I would appreciate an `-ratelimitlogging` option for the sake of testing, undocumented / `DEBUG_ONLY` would be fine for me:
- already now, multiple functional tests hit the limit (`feature_taproot.py`, `p2p_headers_sync_with_minchainwork.py`, `wallet_avoidreuse.py`, `wal
...

💬 caesrcd commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141298823)
@luke-jr
> Everything below 1s/vB is spam.

That’s false. I recently made several consolidation transactions myself, all confirming at <1 sat/vB. Labeling everything below 1 sat/vB as "spam" ignores legitimate use cases like UTXO consolidation, which is actually beneficial for the network.

🚀 glozow merged a pull request: "descriptors: MuSig2"
(https://github.com/bitcoin/bitcoin/pull/31244)

💬 ryanofsky commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3141318636)
> Alternatively, maybe we can make the rpc client aware of all of the rpcs so it can do that conversion?

That's basically what the PR does implicitly and what my refactoring of the PR does more explicitly with string/json formats in
[rpc/client.cpp](https://github.com/bitcoin/bitcoin/blob/b998cc52d51b48db9271fdba0bd69e9aaccb7999/src/rpc/client.cpp#L16).

I think if you look at that file you will see that the the parsing logic is not that complicated and can be well explained. The code the
...