💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476604650)
That said, it seems quite trivial to just change the default log level: https://github.com/Sjors/bitcoin/commit/0ecb8290eeffcd2a2172c6720a2168404870530b

So far that's not generating noise for me.

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1476608456)
force push to: 93c70287a6434c6c665a211dc4dfbbd9c3db4083
- rebase on master
- fix failing unit test

💬 brunoerg commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142437445)
nit:
```suggestion
const std::string uri{"localhost:8080/rest/headers/someresource.json"};
```

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

💬 TheCharlatan commented on pull request "build: Link `libbitcoinkernel` to `libbitcoin_util`":
(https://github.com/bitcoin/bitcoin/pull/27285#issuecomment-1476617574)
Concept NACK

> Isn't libbitcoin_util supposed to be a reusable build artifact?

Maybe, but linking in the entirety of the util library is not what the kernel project is currently trying to achieve. The goal is to prune as many files as possible out of the library. This also stated in stage 1 step 2 of the original kernel issue: https://github.com/bitcoin/bitcoin/issues/24303 . Once stage 2 is reached, there may be a discussion again of a shared build artifact containing the intersection of
...

✅ 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.