💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1476552873)
Updated changes:
- Updated the code with a new workaround to avoid failures on unit tests (`httpserver_tests`).

Thanks @stickies-v for your prompt response and your support on quick workarounds on issues raised.

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142370781)
A simple getter is best implemented in the header

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142379157)
Why remove `const`?

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142382188)
I think this would benefit from helper function(s) to avoid duplicating all the parsing/memclearing logic and make the tests easier to read.

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142376537)
What's the rationale behind moving all this code? I don't think it's necessary?

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142386023)
These changes are unrelated to the segmentation fault and at the very least need to be in a separate commit

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142378663)
seems unrelated to this PR, and no need to touch this otherwise?

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142371332)
```suggestion
const evhttp_uri* GetURIParsed() const;
```

💬 josibake commented on pull request "RPC: Fix fund transaction crash when at 0-value, 0-fee":
(https://github.com/bitcoin/bitcoin/pull/27271#issuecomment-1476558883)
> Good catch.
>
> I agree to fail when the user disallowed automatic coin selection (`add_inputs=false`) but not seeing why we should fail when it's enabled. The wallet can craft the tx by selecting any available coin.

What scenario are you envisioning where we have a `selection_target` of 0, a fee rate of 0, and no pre-selected inputs that we would then want to call `SelectCoins`? With how it currently works, this would lead to `SelectCoins` assuming we DID have pre-selected and trying to
...

💬 achow101 commented on pull request "mempool: Add mempool tracepoints":
(https://github.com/bitcoin/bitcoin/pull/26531#issuecomment-1476570364)
ACK 4b7aec2951fe4595946cdc804b0dec1921d79d05

💬 prusnak commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1476574236)
I think it's quite unfortunate there is no interest in this feature. How can we expect the Bitcoin Core wallet to behave reasonably if it can't predict the fees well?

💬 Sjors commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#issuecomment-1476580792)
I [wrote](https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1141986299):

> using `warning` for now seems fine, since we don't actually print the word "warning" in the log.

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.

🚀 achow101 merged a pull request: "mempool: Add mempool tracepoints"
(https://github.com/bitcoin/bitcoin/pull/26531)

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142420455)
Sorry, forgot to remove all that.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1142421537)
Was unintentional, going to place it on its original location.

🚀 achow101 merged a pull request: "p2p: set `-dnsseed` and `-listen` false if `maxconnections=0`"
(https://github.com/bitcoin/bitcoin/pull/26899)

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

🚀 achow101 merged a pull request: "guix: use osslsigncode 2.5"
(https://github.com/bitcoin/bitcoin/pull/27179)

💬 achow101 commented on pull request "rpc: In `utxoupdatepsbt` also look for the tx in the txindex":
(https://github.com/bitcoin/bitcoin/pull/25939#issuecomment-1476594222)
ACK 6e9f8bb050785dbc754b6bb493aad9139908ef98

💬 TheCharlatan commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1476594464)
Light code review ACK 5555a336019848d2e19e4533b3d2f34e438e6367

Confirmed that it indeed caches these additional packages. That said, I am not familiar with the build flow through the various scripts and cannot really comment if the code was moved to the correct place, or if conventions were followed.

> Would be good to make progress here.

Yes, I need to get a better scheduling workflow for PR's requiring interaction.