Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 fjahr reviewed a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3075780570)
tACK 95c11728f423e1c655439f9248a314f083ef68ef
💬 fjahr commented on pull request "test: Use the same mocktime when migrating in wallet_migration.py":
(https://github.com/bitcoin/bitcoin/pull/33104#discussion_r2245633370)
Could also move this one into `migrate_and_get_rpc` and replace the one there since this one is a bit more detailed.
💬 achow101 commented on pull request "wallet: Set descriptor cache upgraded flag for migrated wallets":
(https://github.com/bitcoin/bitcoin/pull/33031#discussion_r2245990018)
oops fixed
💬 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
...
💬 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`.
💬 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.
📝 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
...
💬 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
💬 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.
fanquake closed a pull request: "Reduce minrelaytxfee to 100 sats/kvB"
(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).
💬 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.
💬 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.
💬 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.
💬 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/)
💬 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 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&) {}
```