💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1472679470)
@theStack: Thanks for the suggestion. I tried implementing this and found that it does save the six lines here, but it also required special casing two places downstream in my follow-up PR. While the fixes were simple, I feel that it makes the interface slightly worse for the downstream consumer, so I’m ambivalent on the change.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1139311306)
If I bring it back I won't be able to access to m_uri_parsed from the helper function, unless there's another alternative, I'll have to roll-back to the original approach of the PR.

💬 stickies-v commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#discussion_r1139348308)
Every line of code that you change needs review. If those changed lines of code are unrelated to what you're trying to achieve, you risk unnecessarily slowing doing the PR or even getting NACKs for reasons unrelated to what you're really trying to do. I'd strongly suggest keeping the scope as small as possible.

I'm not saying it's necessarily a bad idea to use enumerated HTTP status codes, but it would be better off in a separate PR/issue (and have you checked if it's been suggested before?)

💬 BitcoinErrorLog commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1472729383)
Just reviving this. Four months have passed. What are the current stats on people flagging mempoolfullrbf=1?

My guess is that literally only the hostile devs in this thread run it, and whichever miners they managed to persuade personally, and that there is no significant signal of support or usage of the feature in the wild, or am I wrong?

It would be nice for the remaining thousands of users and merchants to not have to look over our shoulder and wonder if Core will sneak it on by defaul
...

💬 fanquake commented on pull request "p2p: Improve diversification of new connections":
(https://github.com/bitcoin/bitcoin/pull/27264#issuecomment-1472730802)
cc @naumenkogs @amitiuttarwar @sdaftuar

💬 fanquake commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1472733580)
```bash
test 2023-03-16T20:06:03.567000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "/private/var/folders/v7/fs2b0v3s0lz1n57gj9y4xb5m0000gn/T/cirrus-ci-build/ci/scratch/build/bitcoin-arm64-apple-darwin/test/functional/test_framework/test_framework.py", line 132, in main
self.run_test()

...

💬 fanquake commented on pull request "doc: fix typo in interface_usdt_utxocache.py":
(https://github.com/bitcoin/bitcoin/pull/27268#issuecomment-1472741835)
Can you update the commit message to the same as what I've changed the PR title too: `doc: fix typo in interface_usdt_utxocache.py`.

💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1472748621)
@fanquake that's something I've introduced with my last push, I'm on it, thanks.

💬 achow101 commented on pull request "test: fix race condition in encrypted wallet rescan tests":
(https://github.com/bitcoin/bitcoin/pull/27199#issuecomment-1472757290)
ACK dbeca792a9980085d00be0f9d78187ca3a4d7cdc

🚀 achow101 merged a pull request: "test: fix race condition in encrypted wallet rescan tests"
(https://github.com/bitcoin/bitcoin/pull/27199)

💬 MarcoFalke commented on issue "Issue with `wallet_importdescriptors.py --descriptors` under valgrind":
(https://github.com/bitcoin/bitcoin/issues/27229#issuecomment-1472769773)
Fixed in https://github.com/bitcoin/bitcoin/pull/27199

✅ MarcoFalke closed an issue: "Issue with `wallet_importdescriptors.py --descriptors` under valgrind"
(https://github.com/bitcoin/bitcoin/issues/27229)

⚠️ dergoegge opened an issue: "Increase fuzz coverage in the wallet"
(https://github.com/bitcoin/bitcoin/issues/27272)
The wallet has (almost) no coverage: https://marcofalke.github.io/btc_cov/fuzz.coverage/src/wallet/index-sort-l.html. We should change that!

Issues like https://github.com/bitcoin/bitcoin/pull/27271 should be quite easy for high level fuzz targets to find.

(For people that are interested but unfamiliar with the wallet (like me): I suspect that looking at the unit tests could be helpful for figuring out where to start)

📝 pinheadmz opened a pull request: "system: allow GUI to initialize default data dir"
(https://github.com/bitcoin/bitcoin/pull/27273)
closes #27246

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1139401775)
Even being a comment in `LookupHost`, you're right, the change is done in the next commit. Thanks

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1139404408)
Seems a leftover from my part, didn't did it intentionally, thanks for noticing it. Fixed.

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1139406701)
Nice find, I think it's better to leave `BOOST_CHECK_MESSAGE()` and using `return addr.value_or(CNetAddr{});`. Addressing it.

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1139407547)
Nice!

💬 brunoerg commented on pull request "p2p: cleanup `LookupIntern`, `Lookup` and `LookupHost`":
(https://github.com/bitcoin/bitcoin/pull/26261#discussion_r1139409510)
Much more cleaner, nice!

💬 mzumsande commented on pull request "test: Make the unlikely race in p2p_invalid_messages impossible?":
(https://github.com/bitcoin/bitcoin/pull/27212#discussion_r1139398466)
I'd just remove this line, otherwise it will stay there for a long time.
After all we have an explanation for why this has failed that makes sense with the log of the failed run, plus a fix that avoid this.

💬 achow101 commented on pull request "mempool: Add mempool tracepoints":
(https://github.com/bitcoin/bitcoin/pull/26531#discussion_r1139414366)
It would probably make sense to include the time for all of the tracepoints added in this PR?