💬 l0rinc commented on pull request "Introduce per-txin sighash midstate cache for legacy/p2sh/segwitv0 scripts":
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-3140801943)
> I ran two identically compiled nodes on two identical VMs on the same host
I did something similar, measuring two reindex runs until 900k blocks for master vs current PR vs enabling all `use_sighash_cache` values.
<details>
<summary>patch of last commit</summary>
```patch
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 7167d13af3..6d57c80de3 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -321,7 +321,7 @@ bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, unsi
...
(https://github.com/bitcoin/bitcoin/pull/32473#issuecomment-3140801943)
> I ran two identically compiled nodes on two identical VMs on the same host
I did something similar, measuring two reindex runs until 900k blocks for master vs current PR vs enabling all `use_sighash_cache` values.
<details>
<summary>patch of last commit</summary>
```patch
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 7167d13af3..6d57c80de3 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -321,7 +321,7 @@ bool PSBTInputSignedAndVerified(const PartiallySignedTransaction psbt, unsi
...
💬 achow101 commented on pull request "test: Use the same mocktime when migrating in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2246020199)
Moved to `migrate_and_get_rpc`.
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2246020199)
Moved to `migrate_and_get_rpc`.
💬 achow101 commented on pull request "test: Use the same mocktime when migrating in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2246021885)
That's a good point. Moving the `listwalletdir` check into `migrate_and_get_rpc` also lets us drop the part where this test needs to pass in the mocktime too.
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2246021885)
That's a good point. Moving the `listwalletdir` check into `migrate_and_get_rpc` also lets us drop the part where this test needs to pass in the mocktime too.
📝 glozow opened a pull request: "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee"
(https://github.com/bitcoin/bitcoin/pull/33106)
Draft until tests look good. Seeking feedback on where else we might be assuming 1sat/vB floor and what the number should be. TODO: ML discussion link
This PR is inspired by #13922 and #32959 to lower the minimum relay feerate in response to bitcoin's exchange rate changes in the last ~7 years. It lowers the default `-minrelaytxfee` and `-incrementalrelayfee`, and knocks `-blockmintxfee` down to the minimum nonzero setting. Also adds some tests for the settings and pulls in #32750 for better
...
(https://github.com/bitcoin/bitcoin/pull/33106)
Draft until tests look good. Seeking feedback on where else we might be assuming 1sat/vB floor and what the number should be. TODO: ML discussion link
This PR is inspired by #13922 and #32959 to lower the minimum relay feerate in response to bitcoin's exchange rate changes in the last ~7 years. It lowers the default `-minrelaytxfee` and `-incrementalrelayfee`, and knocks `-blockmintxfee` down to the minimum nonzero setting. Also adds some tests for the settings and pulls in #32750 for better
...
💬 delta1 commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3140907056)
Concept ACK
I have a wip branch where I’ve somewhat fixed a bunch of tests for this too, which I can share if you like @glozow
I was stuck on the mockmempool feerate before having to do some dayjob work
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3140907056)
Concept ACK
I have a wip branch where I’ve somewhat fixed a bunch of tests for this too, which I can share if you like @glozow
I was stuck on the mockmempool feerate before having to do some dayjob work
💬 1440000bytes commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3140988868)
Concept NACK
This author has not answered things.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3140988868)
Concept NACK
This author has not answered things.
✅ fanquake closed a pull request: "Reduce minrelaytxfee to 100 sats/kvB"
(https://github.com/bitcoin/bitcoin/pull/32959)
(https://github.com/bitcoin/bitcoin/pull/32959)
💬 fanquake commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3141013114)
Closing this, in favour of #33106 (this can't be merged as is, and author is unavailable).
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3141013114)
Closing this, in favour of #33106 (this can't be merged as is, and author is unavailable).
💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3141017084)
I'm wondering if there's a different way we can go about this without adding another table of arguments that need special treatment. Perhaps we could move named argument handling and string to json conversion server side? That's a much more significant refactor, but the RPC server already knows every parameter type and name, so it could do the conversion and named argument check as well.
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3141017084)
I'm wondering if there's a different way we can go about this without adding another table of arguments that need special treatment. Perhaps we could move named argument handling and string to json conversion server side? That's a much more significant refactor, but the RPC server already knows every parameter type and name, so it could do the conversion and named argument check as well.
💬 glozow commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141020410)
> I have a wip branch where I’ve somewhat fixed a bunch of tests for this too, which I can share if you like @glozow
Thanks. Though I think I already fixed them, unless a fuzzer trips.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141020410)
> I have a wip branch where I’ve somewhat fixed a bunch of tests for this too, which I can share if you like @glozow
Thanks. Though I think I already fixed them, unless a fuzzer trips.
💬 1440000bytes commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141025268)
Concept NACK
Even Russian users are bitcoin users.
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141025268)
Concept NACK
Even Russian users are bitcoin users.
💬 benthecarman commented on pull request "[WIP] policy: lower the default blockmintxfee, incrementalrelayfee, minrelaytxfee":
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141043842)
Concept ACK but I believe that the justification for this being the price went up is weak. This suggests also we may just lower it again in the future if the price goes down. Rather I think this is a good idea because it has become more common that blocks contain these transactions and this change makes bitcoin core's mempool better reach it's [goals](https://bitcoincore.org/en/2025/06/06/relay-statement/)
(https://github.com/bitcoin/bitcoin/pull/33106#issuecomment-3141043842)
Concept ACK but I believe that the justification for this being the price went up is weak. This suggests also we may just lower it again in the future if the price goes down. Rather I think this is a good idea because it has become more common that blocks contain these transactions and this change makes bitcoin core's mempool better reach it's [goals](https://bitcoincore.org/en/2025/06/06/relay-statement/)
💬 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....
(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
(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 :
...
(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
(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&) {}
```
(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().
(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.
(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
...
(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
...