💬 l0rinc commented on pull request "build: Revert "Minimize I/O operations in `GenerateHeaderFrom{Json,Raw}.cmake`"":
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346925411)
Fixed in https://github.com/bitcoin/bitcoin/pull/30888
(https://github.com/bitcoin/bitcoin/pull/30883#issuecomment-2346925411)
Fixed in https://github.com/bitcoin/bitcoin/pull/30888
💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1757341947)
I've implemented it with separate errors for block and undo missing.
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1757341947)
I've implemented it with separate errors for block and undo missing.
💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1757342405)
done, good idea!
(https://github.com/bitcoin/bitcoin/pull/30410#discussion_r1757342405)
done, good idea!
💬 mzumsande commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2346937904)
[92236f4 ](https://github.com/bitcoin/bitcoin/commit/92236f43ff92c931f3a099e03d7851b890bff263)to [ccee8a2](https://github.com/bitcoin/bitcoin/commit/ccee8a274770a060fa917f24828f2e2f5c1875ee):
Changed the last commit such that now it throws an error if our block index indicates we should have block/undo data but we can't read it.
Also changed the functional test: Before, it would try to imitate a "pruning" situation by just deleting the undo file. This made little sense to me, because when we
...
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2346937904)
[92236f4 ](https://github.com/bitcoin/bitcoin/commit/92236f43ff92c931f3a099e03d7851b890bff263)to [ccee8a2](https://github.com/bitcoin/bitcoin/commit/ccee8a274770a060fa917f24828f2e2f5c1875ee):
Changed the last commit such that now it throws an error if our block index indicates we should have block/undo data but we can't read it.
Also changed the functional test: Before, it would try to imitate a "pruning" situation by just deleting the undo file. This made little sense to me, because when we
...
💬 katesalazar commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2346939428)
```
$ valgrind ./src/qt/bitcoin-qt -datadir=/mnt/sdb1/chains/bitcoin/main/datadir/ -proxy=127.0.0.1:9050 -maxmempool=$((1024 * 5)) -dbcache=$((1024 * 7))
bash: valgrind: command not found
$ sudo aptitude install valgrind && valgrind ./src/qt/bitcoin-qt -datadir=/mnt/sdb1/chains/bitcoin/main/datadir/ -proxy=127.0.0.1:9050 -maxmempool=$((1024 * 5)) -dbcache=$((1024 * 7))
[sudo] password for administrator:
The following NEW packages will be installed:
libc6-dbg{a} valgrind valgrind-dbg{a}
...
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2346939428)
```
$ valgrind ./src/qt/bitcoin-qt -datadir=/mnt/sdb1/chains/bitcoin/main/datadir/ -proxy=127.0.0.1:9050 -maxmempool=$((1024 * 5)) -dbcache=$((1024 * 7))
bash: valgrind: command not found
$ sudo aptitude install valgrind && valgrind ./src/qt/bitcoin-qt -datadir=/mnt/sdb1/chains/bitcoin/main/datadir/ -proxy=127.0.0.1:9050 -maxmempool=$((1024 * 5)) -dbcache=$((1024 * 7))
[sudo] password for administrator:
The following NEW packages will be installed:
libc6-dbg{a} valgrind valgrind-dbg{a}
...
👍 itornaza approved a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2301097820)
ACK 1be749c771cd9fd80361ebb69c87482920b25cd1
Built and run all tests including the extended functional tests and all of them pass locally. Went through the code changes and read all comments of the pr mostly to educate myself better on the ipc domain. Thanks to all the reviewers and the author for being so explicit.
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2301097820)
ACK 1be749c771cd9fd80361ebb69c87482920b25cd1
Built and run all tests including the extended functional tests and all of them pass locally. Went through the code changes and read all comments of the pr mostly to educate myself better on the ipc domain. Thanks to all the reviewers and the author for being so explicit.
💬 itornaza commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757372497)
The argument list is really difficult to read due to the nature of the arguments. However, I understand that it can be no better than that anyway!
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1757372497)
The argument list is really difficult to read due to the nature of the arguments. However, I understand that it can be no better than that anyway!
👍 maflcko approved a pull request: "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake"
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2301100325)
Concept ACK, but it would be more consistent and easier to maintain to also transform the json one.
Seems a bit absurd that the regex replacment is faster than appending to a string, so it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
(https://github.com/bitcoin/bitcoin/pull/30888#pullrequestreview-2301100325)
Concept ACK, but it would be more consistent and easier to maintain to also transform the json one.
Seems a bit absurd that the regex replacment is faster than appending to a string, so it would be good to test this on Windows and Linux with the minimum and maximum supported cmake version to make sure this is really the last time this needs to be touched.
💬 maflcko commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757374195)
I think it is fine to drop comment around self-explanatory and self-documenting code
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757374195)
I think it is fine to drop comment around self-explanatory and self-documenting code
💬 maflcko commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757374567)
Same, etc.
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757374567)
Same, etc.
💬 maflcko commented on issue "Closing a wallet using the fa46088440 28.x QT client segfaults":
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2346976210)
Looks like this may have been changed recently in https://github.com/bitcoin-core/gui/commit/e682e7db7e399ce888e492eab8f99c389aae05b5. However, I do not know if that change is related.
(https://github.com/bitcoin/bitcoin/issues/30887#issuecomment-2346976210)
Looks like this may have been changed recently in https://github.com/bitcoin-core/gui/commit/e682e7db7e399ce888e492eab8f99c389aae05b5. However, I do not know if that change is related.
💬 l0rinc commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2346985310)
> Seems a bit absurd that the regex replacment is faster than appending to a string
I did some simple profiling and this regex version likely works with some mutable stringbuilder in the background, while the previous string solution was likely quadratic since it's immutable, so it was recreated after each byte.
And the file write is likely faster than that since it uses some buffering.
> also transform the json one
I'm still working on that one, but given the concept ACK, I'll do that
...
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2346985310)
> Seems a bit absurd that the regex replacment is faster than appending to a string
I did some simple profiling and this regex version likely works with some mutable stringbuilder in the background, while the previous string solution was likely quadratic since it's immutable, so it was recreated after each byte.
And the file write is likely faster than that since it uses some buffering.
> also transform the json one
I'm still working on that one, but given the concept ACK, I'll do that
...
📝 maflcko opened a pull request: " log: Use ConstevalFormatString "
(https://github.com/bitcoin/bitcoin/pull/30889)
This changes all logging (including the wallet logging) to produce a
`ConstevalFormatString` at compile time, so that the format string can be
validated at compile-time.
(https://github.com/bitcoin/bitcoin/pull/30889)
This changes all logging (including the wallet logging) to produce a
`ConstevalFormatString` at compile time, so that the format string can be
validated at compile-time.
💬 achow101 commented on pull request "test: Wait for local services to update in feature_assumeutxo":
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2346986883)
ACK 19f4a7c95a99162122068d4badffeea240967a65
(https://github.com/bitcoin/bitcoin/pull/30880#issuecomment-2346986883)
ACK 19f4a7c95a99162122068d4badffeea240967a65
💬 brunoerg commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1757397347)
Done!
(https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1757397347)
Done!
💬 brunoerg commented on pull request "test: addrman: tried 3 times and never a success so `isTerrible=true`":
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2347004914)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1756250067
(https://github.com/bitcoin/bitcoin/pull/30445#issuecomment-2347004914)
Force-pushed addressing https://github.com/bitcoin/bitcoin/pull/30445#discussion_r1756250067
✅ achow101 closed an issue: "ci: failure in feature_assumeutxo.py"
(https://github.com/bitcoin/bitcoin/issues/30878)
(https://github.com/bitcoin/bitcoin/issues/30878)
🚀 achow101 merged a pull request: "test: Wait for local services to update in feature_assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/30880)
(https://github.com/bitcoin/bitcoin/pull/30880)
💬 hebasto commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347015700)
Concept ACK on increasing performance.
(https://github.com/bitcoin/bitcoin/pull/30888#issuecomment-2347015700)
Concept ACK on increasing performance.
💬 l0rinc commented on pull request "build: Minimize I/O operations in GenerateHeaderFromRaw.cmake":
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757406024)
Done
(https://github.com/bitcoin/bitcoin/pull/30888#discussion_r1757406024)
Done