💬 sipa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353868210)
I don't know if this is really a concern in practice, because it's already compensated for by the fact that after a long time, transactions near the top of the mempool have had many chances of being mined already.
We could just get rid of expiration entirely if a time-based limit wasn't valuable, and just rely on eviction due to fullness. But if time-based expiration is valuable, then it's inevitable that a chance exists that a soon-to-be mined transaction expires.
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353868210)
I don't know if this is really a concern in practice, because it's already compensated for by the fact that after a long time, transactions near the top of the mempool have had many chances of being mined already.
We could just get rid of expiration entirely if a time-based limit wasn't valuable, and just rely on eviction due to fullness. But if time-based expiration is valuable, then it's inevitable that a chance exists that a soon-to-be mined transaction expires.
💬 polespinasa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353874982)
> We could just get rid of expiration entirely if a time-based limit wasn't valuable
I might agree with this.
@sipa do you know the reason why this was implemented in the first place?
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353874982)
> We could just get rid of expiration entirely if a time-based limit wasn't valuable
I might agree with this.
@sipa do you know the reason why this was implemented in the first place?
💬 achow101 commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353912094)
Can the same optimization be applied to `SipHashUint256` as well? Those are used by the other `Salted*Hasher`s.
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353912094)
Can the same optimization be applied to `SipHashUint256` as well? Those are used by the other `Salted*Hasher`s.
💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353915093)
Yes, will do it in the next push!
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3353915093)
Yes, will do it in the next push!
💬 achow101 commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#issuecomment-3353916233)
ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
(https://github.com/bitcoin/bitcoin/pull/33476#issuecomment-3353916233)
ACK 9968b15937cbc2071c3474bd83f9c9fb4bb3a970
💬 davidgumberg commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353946106)
### Archeology
Time-based mempool expiry was added here: https://github.com/bitcoin/bitcoin/commit/49b6fd5663dfe081d127cd1eb11407c4d3eaf93d as part of https://github.com/bitcoin/bitcoin/pull/6722, AFAICT most of the relevant discussion happened in https://github.com/bitcoin/bitcoin/pull/6455. There are also two relevant irc discussions: [here](https://web.archive.org/web/20151016174215/http://bitcoinstats.com/irc/bitcoin-dev/logs/2015/09/28#l1443480600.0) and [here](https://bitcoin-irc.chaincod
...
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3353946106)
### Archeology
Time-based mempool expiry was added here: https://github.com/bitcoin/bitcoin/commit/49b6fd5663dfe081d127cd1eb11407c4d3eaf93d as part of https://github.com/bitcoin/bitcoin/pull/6722, AFAICT most of the relevant discussion happened in https://github.com/bitcoin/bitcoin/pull/6455. There are also two relevant irc discussions: [here](https://web.archive.org/web/20151016174215/http://bitcoinstats.com/irc/bitcoin-dev/logs/2015/09/28#l1443480600.0) and [here](https://bitcoin-irc.chaincod
...
💬 achow101 commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3353970233)
ACK 451ba9ada41f687c0e4bb34d5925374a68a8f8a3
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3353970233)
ACK 451ba9ada41f687c0e4bb34d5925374a68a8f8a3
🚀 achow101 merged a pull request: "docs: Undeprecate datacarrier and datacarriersize configuration options"
(https://github.com/bitcoin/bitcoin/pull/33453)
(https://github.com/bitcoin/bitcoin/pull/33453)
💬 fanquake commented on pull request "build: Drop support for EOL macOS 13":
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3354000703)
This will likely fix the Guix build:
```diff
--- a/contrib/guix/symbol-check.py
+++ b/contrib/guix/symbol-check.py
@@ -249,7 +249,7 @@ def check_MACHO_libraries(binary) -> bool:
return ok
def check_MACHO_min_os(binary) -> bool:
- if binary.build_version.minos == [13,0,0]:
+ if binary.build_version.minos == [14,0,0]:
return True
return False
```
(https://github.com/bitcoin/bitcoin/pull/33489#issuecomment-3354000703)
This will likely fix the Guix build:
```diff
--- a/contrib/guix/symbol-check.py
+++ b/contrib/guix/symbol-check.py
@@ -249,7 +249,7 @@ def check_MACHO_libraries(binary) -> bool:
return ok
def check_MACHO_min_os(binary) -> bool:
- if binary.build_version.minos == [13,0,0]:
+ if binary.build_version.minos == [14,0,0]:
return True
return False
```
💬 polespinasa commented on issue "Mempool Expiry eviction might remove txs that could be mined in the next block":
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3354019309)
Thanks for the context @davidgumberg
> > How about we make the expiration time dynamic based on the position (computer by fee rate) in the mempool?
> > Txs more likely to be mined have a bigger expiry time
>
> I think this should be the opposite, the transactions that are the best candidates for time-based expiry, are the ones that are at the top of our mempool, but block after block remain unconfirmed.
Yep, you are right, makes more sense in the opposite 😅
> but I suppose the current beha
...
(https://github.com/bitcoin/bitcoin/issues/33510#issuecomment-3354019309)
Thanks for the context @davidgumberg
> > How about we make the expiration time dynamic based on the position (computer by fee rate) in the mempool?
> > Txs more likely to be mined have a bigger expiry time
>
> I think this should be the opposite, the transactions that are the best candidates for time-based expiry, are the ones that are at the top of our mempool, but block after block remain unconfirmed.
Yep, you are right, makes more sense in the opposite 😅
> but I suppose the current beha
...
💬 achow101 commented on pull request "Fuzz: extend CConnman tests":
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-3354037928)
ACK 0802398e749c5e16fa7085cd87c91a31bbe043bd
(https://github.com/bitcoin/bitcoin/pull/28584#issuecomment-3354037928)
ACK 0802398e749c5e16fa7085cd87c91a31bbe043bd
🚀 achow101 merged a pull request: "Fuzz: extend CConnman tests"
(https://github.com/bitcoin/bitcoin/pull/28584)
(https://github.com/bitcoin/bitcoin/pull/28584)
🤔 w0xlt reviewed a pull request: "test: sock: Enable socket pair tests on Windows"
(https://github.com/bitcoin/bitcoin/pull/33506#pullrequestreview-3287055021)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/33506#pullrequestreview-3287055021)
Concept ACK
🤔 pablomartin4btc reviewed a pull request: "wallet: reduce unconditional logging during load"
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3287139328)
post-merge ACK https://github.com/bitcoin/bitcoin/commit/fc861332b351c9390400054ff74193ce26eb0713
(https://github.com/bitcoin/bitcoin/pull/33299#pullrequestreview-3287139328)
post-merge ACK https://github.com/bitcoin/bitcoin/commit/fc861332b351c9390400054ff74193ce26eb0713
📝 l0rinc opened a pull request: "Use number of dirty cache entries in flush warnings/logs"
(https://github.com/bitcoin/bitcoin/pull/33512)
Revival of https://github.com/bitcoin/bitcoin/pull/31703 with all outstanding review feedback addressed.
<details>
<summary>Changes compared to the original</summary>
* `EmplaceCoinInternalDANGER` is tested now and memory accounting is fixed
* Warning and logging moved from `FlushStateToDisk` to `CCoinsViewDB::BatchWrite`, ensuring AssumeUTXO snapshot writes also display progress warnings
* Removed the redundant changed counter in `BatchWrite` since we now track `dirty_count` directly
...
(https://github.com/bitcoin/bitcoin/pull/33512)
Revival of https://github.com/bitcoin/bitcoin/pull/31703 with all outstanding review feedback addressed.
<details>
<summary>Changes compared to the original</summary>
* `EmplaceCoinInternalDANGER` is tested now and memory accounting is fixed
* Warning and logging moved from `FlushStateToDisk` to `CCoinsViewDB::BatchWrite`, ensuring AssumeUTXO snapshot writes also display progress warnings
* Removed the redundant changed counter in `BatchWrite` since we now track `dirty_count` directly
...
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/31703#issuecomment-3354466151)
Addressed all outstanding feedback in https://github.com/bitcoin/bitcoin/pull/33512
(https://github.com/bitcoin/bitcoin/pull/31703#issuecomment-3354466151)
Addressed all outstanding feedback in https://github.com/bitcoin/bitcoin/pull/33512
💬 l0rinc commented on pull request "[IBD] precalculate SipHash constant salt XORs":
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3354492388)
Thanks @sipa for the review, @achow101 for the comment.
I have pushed an updated version, extended the optimization from just `SipHashUint256Extra` to cover all `SipHash` operations on `uint256`:
* Renamed and unified: `Uint256ExtraSipHasher` → `PresaltedSipHasher`
* All `Salted*Hasher` classes now use `PresaltedSipHasher` internally, so the compact block short ID computations should also be slightly faster after this.
(https://github.com/bitcoin/bitcoin/pull/30442#issuecomment-3354492388)
Thanks @sipa for the review, @achow101 for the comment.
I have pushed an updated version, extended the optimization from just `SipHashUint256Extra` to cover all `SipHash` operations on `uint256`:
* Renamed and unified: `Uint256ExtraSipHasher` → `PresaltedSipHasher`
* All `Salted*Hasher` classes now use `PresaltedSipHasher` internally, so the compact block short ID computations should also be slightly faster after this.
💬 davidgumberg commented on pull request "depends: static libxcb-cursor":
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3354496291)
Tested ACK https://github.com/bitcoin/bitcoin/pull/33434/commits/eca50854e1cb04e20478bd3df4762e18520a3611
I tested doing a guix build of this branch on another x86_64 device running Fedora 41, and on an x86 Fedora 41 virtual machine and an x86 Fedora 42 virtual machine (both hosted on the troublesome machine mentioned above).
The flickering issue described in #33432 occurs on both Fedora 41 and Fedora 42 on both master and the distro-packaged 29.1, but a guix build of this branch fixes the
...
(https://github.com/bitcoin/bitcoin/pull/33434#issuecomment-3354496291)
Tested ACK https://github.com/bitcoin/bitcoin/pull/33434/commits/eca50854e1cb04e20478bd3df4762e18520a3611
I tested doing a guix build of this branch on another x86_64 device running Fedora 41, and on an x86 Fedora 41 virtual machine and an x86 Fedora 42 virtual machine (both hosted on the troublesome machine mentioned above).
The flickering issue described in #33432 occurs on both Fedora 41 and Fedora 42 on both master and the distro-packaged 29.1, but a guix build of this branch fixes the
...
💬 fanquake commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354509363)
https://github.com/bitcoin/bitcoin/actions/runs/18149289661/job/51656823259?pr=33512#step:9:2059:
```bash
Run coins_view_db with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1436700023
INFO: Loaded 1 modules (611994 inline 8-bit counters): 611994 [0x6082fc8a27f8, 0x6082fc937e92),
INFO: Loaded 1
...
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354509363)
https://github.com/bitcoin/bitcoin/actions/runs/18149289661/job/51656823259?pr=33512#step:9:2059:
```bash
Run coins_view_db with args ['/home/admin/actions-runner/_work/_temp/build/bin/fuzz', '-runs=1', PosixPath('/home/admin/actions-runner/_work/_temp/ci/scratch/qa-assets/fuzz_corpora/coins_view_db')]INFO: Running with entropic power schedule (0xFF, 100).
INFO: Seed: 1436700023
INFO: Loaded 1 modules (611994 inline 8-bit counters): 611994 [0x6082fc8a27f8, 0x6082fc937e92),
INFO: Loaded 1
...
💬 l0rinc commented on pull request "Use number of dirty cache entries in flush warnings/logs":
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354512593)
@fanquake the fuzz test is fixed in a [separate PR](https://github.com/bitcoin/bitcoin/pull/32313), I have reverted a similar attempt here
(https://github.com/bitcoin/bitcoin/pull/33512#issuecomment-3354512593)
@fanquake the fuzz test is fixed in a [separate PR](https://github.com/bitcoin/bitcoin/pull/32313), I have reverted a similar attempt here