💬 sipsorcery commented on pull request "build: Enable fuzz binary in MSVC":
(https://github.com/bitcoin/bitcoin/pull/29774#issuecomment-2080067678)
> @sipsorcery
>
> > tACK [09f5a74](https://github.com/bitcoin/bitcoin/commit/09f5a74198c328c80539c17d951a70558e6b361e).
>
> Wrong commit hash?
>
> :)

Every time :( I wish there were timestamps on the Github list. If only my old brain could remember to use git log next time.

tACK #18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd

🤔 sipa reviewed a pull request: "build: Enable fuzz binary in MSVC"
(https://github.com/bitcoin/bitcoin/pull/29774#pullrequestreview-2025809867)
utACK 18fd522ca9a74cf8690a6c9b9b729e78c6ed41fd

📝 mzumsande opened a pull request: "blockstorage: Separate reindexing from saving new blocks"
(https://github.com/bitcoin/bitcoin/pull/29975)
`SaveBlockToDisk` / `FindBlockPos` are used for two purposes, depending on whether they are called during reindexing (`dbp` set,  `fKnown = true`) or in the "normal" case when adding new blocks (`dbp == nullptr`,  `fKnown = false`).
The actual tasks are quite different
- In normal mode, preparations for saving a new block are made, which is then saved: find the correct position on disk (maybe skipping to a new blk file), check for available disk space, update the blockfile info db, save the bl
...

💬 laanwj commented on pull request "depends: Remove Qt build-time dependencies":
(https://github.com/bitcoin/bitcoin/pull/29923#issuecomment-2080165498)
Okay, i implemented a script to download the upstream tarballs and extract the headers from there\*..

So everything except what is in `scripts` is generated--this makes it traightforward to change the required interface versions in [collector.py](https://github.com/laanwj/qtsowrap/blob/main/scripts/collector.py) by pointing it to a different tarball and re-running the process, and committing the changes to git.

The entire process is deterministic so can also be used to verify that the hea
...

📝 hebasto opened a pull request: "build, msvc: Drop duplicated `common\url.cpp` source file"
(https://github.com/bitcoin/bitcoin/pull/29976)
After https://github.com/bitcoin/bitcoin/pull/29904, the `common\url.cpp` source file is included into the `SOURCE_FILES` by the `msvc-autogen.py` script.

Removes a compiler [warning](https://github.com/bitcoin/bitcoin/actions/runs/8853698173/job/24315127236#step:20:1776):
```
url.obj : warning LNK4006: "class std::basic_string<char,struct std::char_traits<char>,class std::allocator<char> > __cdecl UrlDecode(class std::basic_string_view<char,struct std::char_traits<char> >)" (?UrlDecode@@YA
...

💬 TheCharlatan commented on pull request "blockstorage: Separate reindexing from saving new blocks":
(https://github.com/bitcoin/bitcoin/pull/29975#issuecomment-2080168836)
Concept ACK

💬 laanwj commented on pull request "test: switch from curl to requests for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#issuecomment-2080191475)
Yes, that's usually how it'd be done.

But gah, `requests` is an external dependency? That's unfortunate, i don't think we should be adding dependencies unless absolutely necessary. Probably better to do this with python's built in urllib or http client functionality, or keep it like this.

🤔 ryanofsky reviewed a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906#pullrequestreview-2025935950)
Updated e1be443315be6ba6c80267e0e6be59deee77de50 -> 9552619961049d7673f84c40917b0385be70b782 ([`pr/saferesult.7`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.7) -> [`pr/saferesult.8`](https://github.com/ryanofsky/bitcoin/commits/pr/saferesult.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/saferesult.7..pr/saferesult.8)) with small suggested cleanups

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581579143)
re: https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1580997920

> nit: unnecessary extra newline

Thanks, removed

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581580768)
re: https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581039091

> nit: Is this line needed?

Nope, dropped this

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581579195)
re: https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581409638

> Can you briefly explain why these `operator=` are made private as opposed to deleting them?

Changed these to `delete`. The default operators were needed in an earlier version of the PR which had an `Update()` method which called them privately.

💬 ryanofsky commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581580725)
re: https://github.com/bitcoin/bitcoin/pull/29906#discussion_r1581128440

> But, in any case, it's not a big deal if you don't think it's worth it. I also don't think this is a problem anyway.

I'm happy to go with any suggestion. I don't see shadowing as a problem in all cases, so this is just the best way to write the code that I could think of.

💬 fjahr commented on pull request "rpc: Optimize serialization disk space of dumptxoutset - Reloaded":
(https://github.com/bitcoin/bitcoin/pull/29612#issuecomment-2080224670)
Alright, ready for review again. I hope I have addressed all the ideas that were mentioned as intended. Sorry but the enhancement of the metadata is now one big commit for now, I can try to split it up if it makes review easier.

💬 davidgumberg commented on pull request "test: adds outbound eviction functional tests, updates comment in ConsiderEviction":
(https://github.com/bitcoin/bitcoin/pull/29122#issuecomment-2080236283)
reACK https://github.com/bitcoin/bitcoin/commit/d53d84834747c37f4060a9ef379e0a6b50155246

> I don't think I follow you here. `HEADERS_RESPONSE_TIME` and `CHAIN_SYNC_TIMEOUT` are constants hardcoded in `net_processing.cpp` (and mapped here). I would expect unit test to fail if those are updated.

I didn't mean to say that these tests should be responsible for enforcing the particular values of `HEADERS_RESPONSE_TIME` and `CHAIN_SYNC_TIMEOUT`, just that they would not catch a regression where
...

💬 davidgumberg commented on pull request "Lint: support running individual rust linters and improve subtree exclusion":
(https://github.com/bitcoin/bitcoin/pull/29965#issuecomment-2080239525)
I think I will leave it in here since the motivation for https://github.com/bitcoin/bitcoin/commit/54b2115bb848e1aad8a86aae858de63a851235ce is mostly the increased number of lint checks only accessible through the lint test runner. But I will split it out if anyone has an objection, or if this PR stalls.

💬 iw4p commented on pull request "test: switch from curl to urllib for HTTP requests":
(https://github.com/bitcoin/bitcoin/pull/29970#issuecomment-2080372645)
I replaced the `requests` with `urllib`, tested the script for downloading .tar.gz file and works fine for me.

💬 Umiiii commented on pull request "test: add missing comparison of node1's mempool in MempoolPackagesTest":
(https://github.com/bitcoin/bitcoin/pull/29948#issuecomment-2080373067)
Hello @fjahr , @alfonsoromanz , @kevkevinpal ,
Could you please confirm if it is ready for merging, or does it require further reviews?

💬 Karimazam commented on issue "Prune mode is incompatible with -txindex.":
(https://github.com/bitcoin/bitcoin/issues/28684#issuecomment-2080384383)
i installed ltc core wallet when i run it for the very fast time it shows same issue litecoin core wallet prune mode is incompatible with blockfilterindex does anybody know why i am facing this f**king issue/?

👍 fanquake approved a pull request: "build, msvc: Drop duplicated `common\url.cpp` source file"
(https://github.com/bitcoin/bitcoin/pull/29976#pullrequestreview-2026447201)
ACK 97a4ad5713853f51c729cced73f133fafa735ba2

🚀 fanquake merged a pull request: "build, msvc: Drop duplicated `common\url.cpp` source file"
(https://github.com/bitcoin/bitcoin/pull/29976)