💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146166)
Done in #29770
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146166)
Done in #29770
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146312)
Done in #29770
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146312)
Done in #29770
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146409)
done in #29770
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146409)
done in #29770
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146466)
done in #29770
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146466)
done in #29770
💬 fjahr commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146524)
sounds good, taken as you suggested in #29770
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1676146524)
sounds good, taken as you suggested in #29770
💬 hebasto commented on pull request "depends: build libevent with CMake":
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2225902746)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/262.
(https://github.com/bitcoin/bitcoin/pull/29835#issuecomment-2225902746)
Ported to the CMake-based build system in https://github.com/hebasto/bitcoin/pull/262.
🚀 fanquake merged a pull request: "#28984 package rbf followups"
(https://github.com/bitcoin/bitcoin/pull/30295)
(https://github.com/bitcoin/bitcoin/pull/30295)
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676161918)
@paplorinc what do you think a better name for this parameter would be?
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676161918)
@paplorinc what do you think a better name for this parameter would be?
✅ fanquake closed a pull request: "Allow to configure custom libzmq prefix"
(https://github.com/bitcoin/bitcoin/pull/30256)
(https://github.com/bitcoin/bitcoin/pull/30256)
💬 fanquake commented on pull request "Allow to configure custom libzmq prefix":
(https://github.com/bitcoin/bitcoin/pull/30256#issuecomment-2225910922)
I agree. Thanks for the contribution, but we can revisit this (if needed) post-CMake.
(https://github.com/bitcoin/bitcoin/pull/30256#issuecomment-2225910922)
I agree. Thanks for the contribution, but we can revisit this (if needed) post-CMake.
💬 brunoerg commented on pull request "fuzz: wallet: add target for `CreateTransaction`":
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2225945198)
Force-pushed adding `SeedRandomForTest`
(https://github.com/bitcoin/bitcoin/pull/29936#issuecomment-2225945198)
Force-pushed adding `SeedRandomForTest`
👍 tdb3 approved a pull request: "rpc: Use CHECK_NONFATAL over Assert"
(https://github.com/bitcoin/bitcoin/pull/30429#pullrequestreview-2175319485)
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f
> Can you add exact steps to reproduce, starting from a fresh install of your operating system? Otherwise there is nothing that can be done. For a fresh install of Ubuntu 24.04 LTS (noble) or Debian GNU/Linux 12 (bookworm) it passes:
Was unable to reproduce the original issue.
I had manually changed the `CHECK_NONFATAL`s back to `Assert`. It's possible it was manual error, but not 100% sure. Either way, it doesn't seem to happen now on a non
...
(https://github.com/bitcoin/bitcoin/pull/30429#pullrequestreview-2175319485)
ACK fa6270737eb9655bfb4e29b7070ecb6cd2087b7f
> Can you add exact steps to reproduce, starting from a fresh install of your operating system? Otherwise there is nothing that can be done. For a fresh install of Ubuntu 24.04 LTS (noble) or Debian GNU/Linux 12 (bookworm) it passes:
Was unable to reproduce the original issue.
I had manually changed the `CHECK_NONFATAL`s back to `Assert`. It's possible it was manual error, but not 100% sure. Either way, it doesn't seem to happen now on a non
...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676216295)
In https://github.com/bitcoin/bitcoin/blob/dddb59a63d90a108e7acefacdb6689a07324a6d7/src/coins.cpp#L193, we're calling `Next` with `erase`, which simply return `current.second.Next()` without any mutation, because of https://github.com/bitcoin/bitcoin/blob/dddb59a63d90a108e7acefacdb6689a07324a6d7/src/coins.h#L245, right?
This is the part I don't understand, especially since later in https://github.com/bitcoin/bitcoin/blob/dddb59a63d90a108e7acefacdb6689a07324a6d7/src/coins.cpp#L207, we're movi
...
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676216295)
In https://github.com/bitcoin/bitcoin/blob/dddb59a63d90a108e7acefacdb6689a07324a6d7/src/coins.cpp#L193, we're calling `Next` with `erase`, which simply return `current.second.Next()` without any mutation, because of https://github.com/bitcoin/bitcoin/blob/dddb59a63d90a108e7acefacdb6689a07324a6d7/src/coins.h#L245, right?
This is the part I don't understand, especially since later in https://github.com/bitcoin/bitcoin/blob/dddb59a63d90a108e7acefacdb6689a07324a6d7/src/coins.cpp#L207, we're movi
...
📝 brunoerg opened a pull request: "test: addrman: tried 3 times and never a success so `isTerrible=true`"
(https://github.com/bitcoin/bitcoin/pull/30445)
This PR adds test coverage for the following verification:
```cpp
if (TicksSinceEpoch<std::chrono::seconds>(m_last_success) == 0 && nAttempts >= ADDRMAN_RETRIES) { // tried N times and never a success
return true;
}
```
If we've tried an address for 3 or more times and were unsuccessful, this address should be pointed out as "terrible".
-------
You can test this by applying:
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index 054a9bee32..93a9521b59 100644
--- a/src
...
(https://github.com/bitcoin/bitcoin/pull/30445)
This PR adds test coverage for the following verification:
```cpp
if (TicksSinceEpoch<std::chrono::seconds>(m_last_success) == 0 && nAttempts >= ADDRMAN_RETRIES) { // tried N times and never a success
return true;
}
```
If we've tried an address for 3 or more times and were unsuccessful, this address should be pointed out as "terrible".
-------
You can test this by applying:
```diff
diff --git a/src/addrman.cpp b/src/addrman.cpp
index 054a9bee32..93a9521b59 100644
--- a/src
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676224407)
Great question!
When `erase = true`, we will be erasing the entire map at the end of `BatchWrite`. When `erase = false`, we want to keep all unspent coins in the map and have their flags cleared.
While looping through `CCoinsMap::iterator`s, we can just do `it = erase ? mapCoins.erase(true) : std::next(it)`, and then deleting all spent coins and clear their flags right after we return from `BatchWrite` when `erase = false`.
Now that we are looping through `CoinsCachePair`s though, we ca
...
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676224407)
Great question!
When `erase = true`, we will be erasing the entire map at the end of `BatchWrite`. When `erase = false`, we want to keep all unspent coins in the map and have their flags cleared.
While looping through `CCoinsMap::iterator`s, we can just do `it = erase ? mapCoins.erase(true) : std::next(it)`, and then deleting all spent coins and clear their flags right after we return from `BatchWrite` when `erase = false`.
Now that we are looping through `CoinsCachePair`s though, we ca
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2226020087)
Some benchmarks with the lastest changes. Did 2 IBD with `prune=550` and 2 `-reindex-chainstate` with `prune=0`. Results were similar to before. This time no prune was 1% faster on this branch with default `dbache`, and 1% slower on this branch with max `dbcache`. This just means there's no regression IMO.
It's 34% faster pruned with max `dbcache` and 13% faster with default `dbcache` on this branch vs a pruned node on master with max `dbcache`.
| | prune | dbcache | mean time (s) | speed
...
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2226020087)
Some benchmarks with the lastest changes. Did 2 IBD with `prune=550` and 2 `-reindex-chainstate` with `prune=0`. Results were similar to before. This time no prune was 1% faster on this branch with default `dbache`, and 1% slower on this branch with max `dbcache`. This just means there's no regression IMO.
It's 34% faster pruned with max `dbcache` and 13% faster with default `dbcache` on this branch vs a pruned node on master with max `dbcache`.
| | prune | dbcache | mean time (s) | speed
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676251092)
> , we're moving when erase = true because we've already erased it - right?
We're moving because we are going to erase it in either case, not that we have yet. We can't move an erased entry because it will have already been destroyed. And we can't use a moved entry later because it is invalidated by the move. So when we want to keep the entry, we need to copy it and not move out of it.
When `erase = true`, we will be erasing the entry after we return, so all entries can be moved. When `coin
...
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676251092)
> , we're moving when erase = true because we've already erased it - right?
We're moving because we are going to erase it in either case, not that we have yet. We can't move an erased entry because it will have already been destroyed. And we can't use a moved entry later because it is invalidated by the move. So when we want to keep the entry, we need to copy it and not move out of it.
When `erase = true`, we will be erasing the entry after we return, so all entries can be moved. When `coin
...
💬 paplorinc commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676259699)
I will have to read that again tomorrow, it's the end of the day here :/
`will_erase` could _help_, but I find it hard to keep track of so many independent mutations, I'm hoping there's a simpler way to do this...
Do you think you could add a test that at least doesn't pass for my suggested code above? Maybe that will help me understand why something this simple (iteration with/without simple/bulk cleanup) has this many loose parts, where the exact same parameter means the opposite from one li
...
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676259699)
I will have to read that again tomorrow, it's the end of the day here :/
`will_erase` could _help_, but I find it hard to keep track of so many independent mutations, I'm hoping there's a simpler way to do this...
Do you think you could add a test that at least doesn't pass for my suggested code above? Maybe that will help me understand why something this simple (iteration with/without simple/bulk cleanup) has this many loose parts, where the exact same parameter means the opposite from one li
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676292349)
You're right, the logic for moving or not should not leak into BatchWrite. Perhaps we can get rid of `erase` in BatchWrite and have the cursor return a pair - the iterator and a bool for whether it can move out or not?
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1676292349)
You're right, the logic for moving or not should not leak into BatchWrite. Perhaps we can get rid of `erase` in BatchWrite and have the cursor return a pair - the iterator and a bool for whether it can move out or not?
💬 pinheadmz commented on pull request "Stratum v2 connman":
(https://github.com/bitcoin/bitcoin/pull/30332#issuecomment-2226108900)
I am working on a libevent-replacing HTTP server using netbase primitives and yeah, I [have some functions](https://github.com/pinheadmz/bitcoin/blob/http-netbase/src/http.cpp#L311) that look a lot like the sv2connamn in this branch.
I think it would be cool if possible to abstract the mechanisms in `ThreadSocketHandler` to work on "abstract clients" and I'd be happy to review and collaborate on that.
(https://github.com/bitcoin/bitcoin/pull/30332#issuecomment-2226108900)
I am working on a libevent-replacing HTTP server using netbase primitives and yeah, I [have some functions](https://github.com/pinheadmz/bitcoin/blob/http-netbase/src/http.cpp#L311) that look a lot like the sv2connamn in this branch.
I think it would be cool if possible to abstract the mechanisms in `ThreadSocketHandler` to work on "abstract clients" and I'd be happy to review and collaborate on that.