Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2245743993)
Fixed in latest push. Only modification to https://github.com/stickies-v/bitcoin/commit/3d96ff75f161419654b14a7e9fd884e52aec26c4 was that I removed the `LogInstance().EnableCategory(BCLog::LogFlags::NONE);` line.
💬 Crypt-iQ commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#issuecomment-3140442210)
Latest push d434155db5f9c34b8ab3e19038d824d161b34195 -> f57496e:
- ensures the category mask does not leak across tests
- modifies `logging_filesize_rate_limit` to disable `BCLog::LogFlags::LOCK` if `DEBUG_LOCKCONTENTION` is defined

This gets rid of the test flake without hacking around `ReadDebugLogLines`.
💬 maflcko commented on pull request "ci: limit max stack size to 512 KiB":
(https://github.com/bitcoin/bitcoin/pull/33079#issuecomment-3140480780)
Though, it fails on GHA: https://github.com/maflcko/bitcoin-core-with-ci/actions/runs/16649916226/job/47119624805#step:8:30398

Without a way to reproduce interactively, it makes it hard to debug :(
💬 fanquake commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3140526097)
> TSAN failure

Didn't fail with a re-run: https://github.com/bitcoin/bitcoin/actions/runs/16651908244/job/47133023619.
💬 stickies-v commented on pull request "log: rate limiting followups":
(https://github.com/bitcoin/bitcoin/pull/33011#discussion_r2245830190)
Yeah sorry that should've been `LogInstance().DisableCategory(BCLog::LogFlags::ALL);`. I still think that's worth keeping, so we can always assume `LogTest` starts without any categories (as is default)?
💬 TheCharlatan commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3140574746)
Concept ACK

Implemented the same thing some time ago https://github.com/TheCharlatan/bitcoin/commit/de7e3a973ef5773714cd15ead499fbf734547cf1#diff-ac5fbeb5de6f28370bc348a579fc465fe7f7b91df0e0483c6edbf10273278c0d.
💬 BitcoinMechanic commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3140588217)
>It's irrelevant whether or not anyone considers any transaction "spam", all that matters is if miners are mining them, and they are.

Of course it's relevant if we're discussing what we relay and what we don't.

>All you accomplish by rejecting transactions that are getting mined is slowing down block propagation, and potentially wasting CPU cycles and bandwidth by downloading and processing the transaction twice.

Completely false. It's sad to see such obvious dishonesty in here.
🤔 enirox001 reviewed a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3076132085)
ACK d801926

Confirmed this fixes the timing race condition. The test was failing due to different timestamps being captured in the test vs inside `migrate_and_get_rpc()`, causing backup filename mismatches in slow environments.

Tested locally - reproduces the failure without the fix and passes with it. Good solution that maintains backward compatibility while centralizing mocktime management.
🤔 enirox001 reviewed a pull request: "test: Use the same mocktime when migrating in wallet_migration.py"
(https://github.com/bitcoin/bitcoin/pull/33104#pullrequestreview-3076154925)
ACK
💬 l0rinc commented on pull request "refactor: inline constant return values from `dbwrapper` write methods":
(https://github.com/bitcoin/bitcoin/pull/33042#issuecomment-3140630235)
The [IRC meeting](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2025-07-31#1140025) contained feedback on the overall direction:
> \<achow101> #topic dbwrapper read/write asymmetry (l0rinc)
> \<l0rinc> dbwrapper writes currently pretend to return bool but always return true, while real errors surface via exceptions – #33042 makes that explicit, but we need to decide the canonical error path
> \<l0rinc> `read` catches the exception and returns, but `write` always returned `true`, while th
...
👍 l0rinc approved a pull request: "test: Add and use ElapseTime helper"
(https://github.com/bitcoin/bitcoin/pull/32430#pullrequestreview-3076264451)
code review fa80e750e702e2f0429fc6b83e9af2b8ed9b7b3c
🤔 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.