💬 MarcoFalke commented on issue "Bitcoin Core v25.0 Crashes":
(https://github.com/bitcoin/bitcoin/issues/28119#issuecomment-1645658066)
> 2023-07-21T03:41:15Z Setting file arg: dbcache = "4096"

Is there a reason why this is larger? IIRC, there is no data available that increasing it will improve performance on all available hardware. If other programs use 4 GB of memory, Bitcoin Core may run out of memory and crash.

Can you try again with less and then also check if block connection is faster? Currently it takes almost 2 minutes, which seems slow:

```
2023-07-21T05:59:12Z [bench] - Connect block: 45491.16ms [7951.41s (
...

💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1645661021)
It is possible for a connection to have more than one request (see: https://github.com/libevent/libevent/issues/1383#issuecomment-1383249281) or `evhttp_associate_new_request_with_connection` in the libevent code. It appears though that the requests are sequential so when one completes the next one can run. I modified the python script to test this, but it doesn't lead to a deadlock or crash because when the next request comes in, it uses the same `evhttp_connection*` and so the insert into `g_h
...

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1645705020)
Updated 03e06e589a24f0bcb7dbd28a0aaa925b18b87b9d -> 0498ea61f4ce1d73e84f866f420a2afb209fb01c ([kernelRmUnivalue_0](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_0) -> [kernelRmUnivalue_1](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_0..kernelRmUnivalue_1))

* Addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1269708017), fixed include
* Add
...

📝 pinheadmz opened a pull request: "include verbose debug messages in testmempoolaccept reject-reason"
(https://github.com/bitcoin/bitcoin/pull/28121)

👋 pinheadmz's pull request is ready for review: "include verbose debug messages in testmempoolaccept reject-reason"
(https://github.com/bitcoin/bitcoin/pull/28121)

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1270787350)
Yes. My idea is to complement the functional tests, not obsolete them.

In general, when the test is run from inside the same process then we have greater control of what happens - we can inspect variables and check state in more detail than a functional test can do from outside of `bitcoind` - it has to use the RPC. Also, from inside the process we can alter the state by modifying variables or calling certain functions that is not possible from outside of the process.

💬 MarcoFalke commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1270790467)
Missing newline after self-include?

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1270799400)
Ups, done.

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1645757452)
Updated 0498ea61f4ce1d73e84f866f420a2afb209fb01c -> 05477b5566dc6f9c3d5c9609268002c0fbe1e1aa ([kernelRmUnivalue_1](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_1) -> [kernelRmUnivalue_2](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_2), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_1..kernelRmUnivalue_2))

* Addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1270790467), fixed include spacin
...

💬 fanquake commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1645764514)
Concept ACK. cc @theuni

💬 fanquake commented on pull request "util: Don't derive secure_allocator from std::allocator":
(https://github.com/bitcoin/bitcoin/pull/27930#issuecomment-1645766067)
@CaseyCarter did you also want to pickup the change to `zero_after_free_allocator` ?
Otherwise, we can have someone cherry-pick you're commit into a new PR, and make the additional changes.

👋 dergoegge's pull request is ready for review: "util: Type-safe transaction identifiers"
(https://github.com/bitcoin/bitcoin/pull/28107)

💬 dergoegge commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#issuecomment-1645768047)
Ready for review

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1270815944)
Taken, thanks

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1270816338)
Done

💬 hebasto commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1645806444)
Putting new parsing helpers into the `libbitcoin_common` library is a bit questionable for me.

I was expecting to see them in the `libbitcoin_util`. However, it is not possible with the suggested approach as the `univalue_helpers.cpp` depends on `SIGHASH_DEFAULT`.

💬 TheCharlatan commented on pull request "util: Type-safe transaction identifiers":
(https://github.com/bitcoin/bitcoin/pull/28107#issuecomment-1645952454)
Concept ACK

💬 MarcoFalke commented on pull request "Fuzz: a more efficient descriptor parsing target":
(https://github.com/bitcoin/bitcoin/pull/27888#issuecomment-1645976254)
Did quick check to see how long it would take to find my injected bug with libfuzzer:

* `-use_value_profile=1`:

* `-use_value_profile=0`:

🤔 ryanofsky reviewed a pull request: "refactor: Add util::Result failure values, multiple error and warning messages"
(https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1541470201)
Rebased 28a954c7034077ac3a45083dd5e2b5cdb4d4cdde -> 40f09de73e61e7ae62d6639a49b7c7ac48d514d9 ([`pr/bresult2.33`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.33) -> [`pr/bresult2.34`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.34), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/bresult2.33-rebase..pr/bresult2.34)) due to conflict with various PR and making many suggested changes from review #25722 (which is based on this PR)

re: https://github.com/bitcoin/
...

💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1270866000)
re: https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1234264247

> Can you add an example for when the Result error type in a chain of function calls remains the same, but the value type changes? Is there a better way to do this conversion than manually constructing the result again (like below)?

Yes, there is definitely a better way to pass along errors and warnings from one result to another without calling `ErrorString` and flattening them. The `util::Result` constructor will mo
...