✅ hebasto closed a pull request: "build: Link `libbitcoinkernel` to `libbitcoin_util`"
(https://github.com/bitcoin/bitcoin/pull/27285)

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142447667)
I wanted to make sure then to use the free up function for the request in the `HTTPRequest` destructor: `evhttp_request_free`. This was something I was doing while investigating the previous failure of the ASan tests and possible memory leaks. I'll remove it if you think unnecessary,

💬 jamesob commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476625515)
> That said, it seems quite trivial to just change the default log level: [Sjors@0ecb829](https://github.com/Sjors/bitcoin/commit/0ecb8290eeffcd2a2172c6720a2168404870530b)
>
> So far that's not generating noise for me.

I'm happy to do that and revert the revert if people will ACK.

💬 MarcoFalke commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1476626064)
> Yes, I need to get a better scheduling workflow for PR's requiring interaction.

I was planning to put it up for hi-prio, but I keep missing the meeting :sweat_smile:

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142449820)
ok, let me add some helper(s) there.

👍 sipa approved a pull request: "Add pool based memory resource"
(https://github.com/bitcoin/bitcoin/pull/25325)
ACK d87cb99bb37637e26a9e00b9f7de4bc6f44cb79d

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142466765)
Yeah, putting it back, before the compiler was complaining as in the previous approach we were modifying m_uri_parsed in this function.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142467803)
Similar reason to the above comment on`GetURIParsed()` function, back again.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142472126)
I'll take it back.

👍 Sjors approved a pull request: "Log new headers"
(https://github.com/bitcoin/bitcoin/pull/27278)
tACK 2c3a90f663a61ee147d785167a2902494d81d34d

Prefer if you leave it like this and leave log level tweaks to a followup.

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142467276)
Funny enough this message comes _after_ "Saw new header". So from the point of view of making the log a bit more intuitive it does make sense to handle logging in net_processing. That might also solve the IBD "problem", since we could log only headers that we didn't request ourselves.

All that said: from a review point of view I prefer punting this to a followup.

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142453679)
This creates a nice flood of messages when running with `-debug=validation` right after our headers presync succeeds. But that's probably fine if someone is actively debugging IBD.

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1142438454)
re: [#27224 (comment)](https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137458690)

Instead of extending the PR to modify GUI code, I've kept changes here internal to the wallet and added a note in `setAddressReceiveRequest` about the wallet/gui interface and say how it could be improved in the future.

💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1142496680)
> re: [#27224 (comment)](https://github.com/bitcoin/bitcoin/pull/27224#discussion_r1137647274)

This is done now

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142506066)
done.

💬 jonatack commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476698491)
> I should have tested that, because it actually _does_ print `[validation:warning]` in the log.
>
> Maybe we should just stick to `fprintf` and change the TODO to say this should be `validation` level `info` once `LogPrintf` prints those by default.

Perhaps just go for `info`. Making info unconditionally logged is near the top of the next steps in the parent PR #25203 i.e. https://github.com/bitcoin/bitcoin/pull/25203/commits/118c7567f62df2b882877590f232242d7c627a05. Until then, -debug=va
...

👍 ryanofsky approved a pull request: "wallet: Replace use of purpose strings with an enum"
(https://github.com/bitcoin/bitcoin/pull/27217)
Code review ACK a48010f1fd6d8a430b8234789ac3c91ec9d06972. Just comment and whitespace changes since last review

💬 Sjors commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476753978)
It's not limited to the test. Running that commit in regtest, creating a wallet and then exiting triggers it too. That should make debugging easier...

💬 martinus commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1476761563)
Code review ACK ebfc2a2dddedcbf384f435716d30d8418a77505b and ran `git rebase --interactive upstream/master --exec 'make clean && make -j14 check'`. Btw. I really links to the changes in your PR updates. Do you use any script to help with that?

👍 hebasto approved a pull request: "refactor: Extract util/fs from util/system"
(https://github.com/bitcoin/bitcoin/pull/27254)
ACK 3ceb8dde48fc12f4f16372f661a4cccf7104e194

The commit "_Add missing fs.h include..._" could address another IWYU [suggestion](https://api.cirrus-ci.com/v1/task/6080090677706752/logs/ci.log):
```diff
--- a/src/node/chainstate.cpp
+++ b/src/node/chainstate.cpp
@@ -16,6 +16,7 @@
#include <tinyformat.h>
#include <txdb.h>
#include <uint256.h>
+#include "util/fs.h"
#include <util/time.h>
#include <util/translation.h>
#include <validation.h>
```