๐ฌ aureleoules commented on pull request "wallet: Use defined purposes instead of inlining":
(https://github.com/bitcoin/bitcoin/pull/26858#issuecomment-1483953418)
Closing in favor of #27217.
(https://github.com/bitcoin/bitcoin/pull/26858#issuecomment-1483953418)
Closing in favor of #27217.
โ
aureleoules closed a pull request: "wallet: Use defined purposes instead of inlining"
(https://github.com/bitcoin/bitcoin/pull/26858)
(https://github.com/bitcoin/bitcoin/pull/26858)
โ ๏ธ EthanHeilman opened an issue: "Bitcoin fails to build on MSVC: Libevent and BCryptGenRandom errors "
(https://github.com/bitcoin/bitcoin/issues/27332)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When running `msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minimal` I get the following two errors in the build.
`C:\Users\e0\Documents\GitHub\bitcoin\src\httpserver.cpp(637,9): error C2664: 'void evhttp_connection_get_peer(evhttp_connection *,const char **,uint16_t *)': cannot convert argument 2 from 'char **' to 'const char **' [C:\Users\e0\Do
...
(https://github.com/bitcoin/bitcoin/issues/27332)
### Is there an existing issue for this?
- [X] I have searched the existing issues
### Current behaviour
When running `msbuild build_msvc\bitcoin.sln -property:Configuration=Release -maxCpuCount -verbosity:minimal` I get the following two errors in the build.
`C:\Users\e0\Documents\GitHub\bitcoin\src\httpserver.cpp(637,9): error C2664: 'void evhttp_connection_get_peer(evhttp_connection *,const char **,uint16_t *)': cannot convert argument 2 from 'char **' to 'const char **' [C:\Users\e0\Do
...
๐ TheCharlatan approved a pull request: "build, qt: Fix handling of `CXX=clang++` when building `qt` package"
(https://github.com/bitcoin/bitcoin/pull/27314)
tACK 25e8fe70c6e65c07cf602568f5c2cf5535fca995
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
daa94946aae7d4826d6f329337791a6bda0f01ac73f27003d1677dc8de031071 guix-build-25e8fe70c6e6/output/aarch64-linux-gnu/SHA256SUMS.part
38e9dcb99e329ad02837fd08559a8408bd7ba698f65163d0834d52dc9eefceae guix-build-25e8fe70c6e6/output/aarch64-linux-gnu/bitcoin-25e8fe70c6e6-aarch64-linux-gnu-debug.tar.gz
36d48b93724b112e5553a8d04
...
(https://github.com/bitcoin/bitcoin/pull/27314)
tACK 25e8fe70c6e65c07cf602568f5c2cf5535fca995
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
daa94946aae7d4826d6f329337791a6bda0f01ac73f27003d1677dc8de031071 guix-build-25e8fe70c6e6/output/aarch64-linux-gnu/SHA256SUMS.part
38e9dcb99e329ad02837fd08559a8408bd7ba698f65163d0834d52dc9eefceae guix-build-25e8fe70c6e6/output/aarch64-linux-gnu/bitcoin-25e8fe70c6e6-aarch64-linux-gnu-debug.tar.gz
36d48b93724b112e5553a8d04
...
๐ vstoyanov opened a pull request: "ci: cleanup of CI_EXEC & CI_EXEC_ROOT (refs #27321)"
(https://github.com/bitcoin/bitcoin/pull/27333)
Basically it removes the above-mentioned env-vars as per @MarcoFalke's instructions. The only liberty I took was in double-quoting the last instance of $ANDROID_HOME for the sake of consistency and future-proofing.
References #27321
(https://github.com/bitcoin/bitcoin/pull/27333)
Basically it removes the above-mentioned env-vars as per @MarcoFalke's instructions. The only liberty I took was in double-quoting the last instance of $ANDROID_HOME for the sake of consistency and future-proofing.
References #27321
๐ฌ vstoyanov commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484085076)
Hello,
I think I just completed this one. My understanding was that it only covers 01_base_install.sh and not instances after test/04_install.sh as from there on it also redefines $PATH and current directory.
Best,
Vasil Stoyanov
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484085076)
Hello,
I think I just completed this one. My understanding was that it only covers 01_base_install.sh and not instances after test/04_install.sh as from there on it also redefines $PATH and current directory.
Best,
Vasil Stoyanov
๐ฌ vstoyanov commented on pull request "Torcontrol opt check":
(https://github.com/bitcoin/bitcoin/pull/25136#issuecomment-1484090918)
@amadeuszpawlik @fanquake May I take over and wrap this one up?
(https://github.com/bitcoin/bitcoin/pull/25136#issuecomment-1484090918)
@amadeuszpawlik @fanquake May I take over and wrap this one up?
๐ furszy approved a pull request: "bumpfee: avoid making bumped transactions with too low fee when replacing outputs"
(https://github.com/bitcoin/bitcoin/pull/27308)
ACK 26b4c664
Was checking the same few days ago.
(https://github.com/bitcoin/bitcoin/pull/27308)
ACK 26b4c664
Was checking the same few days ago.
๐ฌ furszy commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1148550612)
Missed to remove the `wtx` function's arg. It's not needed anymore.
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1148550612)
Missed to remove the `wtx` function's arg. It's not needed anymore.
๐ฌ furszy commented on pull request "bumpfee: avoid making bumped transactions with too low fee when replacing outputs":
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1148550959)
while you are here, could rename `mtx` so it doesn't shadow the function's arg (or rename the argument).
(https://github.com/bitcoin/bitcoin/pull/27308#discussion_r1148550959)
while you are here, could rename `mtx` so it doesn't shadow the function's arg (or rename the argument).
๐ martinus opened a pull request: "util: implement `noexcept` move assignment & move ctor for `prevector`"
(https://github.com/bitcoin/bitcoin/pull/27334)
`prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector.
The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it.
merge-base:
| ns/op | op/s |
...
(https://github.com/bitcoin/bitcoin/pull/27334)
`prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector.
The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it.
merge-base:
| ns/op | op/s |
...
๐ฌ Ayush170-Future commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484123445)
Hey @vstoyanov!
No problem at all. You seem to understand this far better than I do, so no worries. I will at the very least do a code review for your PR (as I mentioned in my comment). And will look for some other issues to contribute to.
P.S. Thanks for clearing my test/04_install.sh doubt. I got a little confused there.
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484123445)
Hey @vstoyanov!
No problem at all. You seem to understand this far better than I do, so no worries. I will at the very least do a code review for your PR (as I mentioned in my comment). And will look for some other issues to contribute to.
P.S. Thanks for clearing my test/04_install.sh doubt. I got a little confused there.
๐ฌ vstoyanov commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484135897)
@Ayush170-Future Thanks for understanding. Also thanks for the complement, but maybe this is not the case :) - I got to the point where `$CI_BASE_PACKAGES `and `$CI_PACKAGES` should pass lint. This means either convert them to bash arrays or use SuppressWarnings to switch lint of. The first seems much cleaner to do, the second is a smaller change and I am confused as to what to do with that.
In about 45 minutes I will push the array conversion in a second commit I could squash later if we dec
...
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484135897)
@Ayush170-Future Thanks for understanding. Also thanks for the complement, but maybe this is not the case :) - I got to the point where `$CI_BASE_PACKAGES `and `$CI_PACKAGES` should pass lint. This means either convert them to bash arrays or use SuppressWarnings to switch lint of. The first seems much cleaner to do, the second is a smaller change and I am confused as to what to do with that.
In about 45 minutes I will push the array conversion in a second commit I could squash later if we dec
...
๐ฌ Ferrydh commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484136940)
่ฟๆฏๆฅ่ชQQ้ฎ็ฎฑ็ๅๆ่ชๅจๅๅค้ฎไปถใ
ๆจๅฅฝ๏ผๆๆ่ฟๆญฃๅจไผๅไธญ๏ผๆ ๆณไบฒ่ชๅๅคๆจ็้ฎไปถใๆๅฐๅจๅๆ็ปๆๅ๏ผๅฐฝๅฟซ็ปๆจๅๅคใ
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484136940)
่ฟๆฏๆฅ่ชQQ้ฎ็ฎฑ็ๅๆ่ชๅจๅๅค้ฎไปถใ
ๆจๅฅฝ๏ผๆๆ่ฟๆญฃๅจไผๅไธญ๏ผๆ ๆณไบฒ่ชๅๅคๆจ็้ฎไปถใๆๅฐๅจๅๆ็ปๆๅ๏ผๅฐฝๅฟซ็ปๆจๅๅคใ
๐ฌ hebasto commented on pull request "util: implement `noexcept` move assignment & move ctor for `prevector`":
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1484141370)
Concept ACK. Thanks for picking [this](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108186877) up.
(https://github.com/bitcoin/bitcoin/pull/27334#issuecomment-1484141370)
Concept ACK. Thanks for picking [this](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108186877) up.
๐ฌ martinus commented on pull request "log: Check that the timestamp string is non-empty to avoid undefined behavior":
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1148585918)
I've seen this happening to computers before, but never when bitcoind was running. Usually that happens when time synchronization goes wrong for whatever reason.
(https://github.com/bitcoin/bitcoin/pull/27317#discussion_r1148585918)
I've seen this happening to computers before, but never when bitcoind was running. Usually that happens when time synchronization goes wrong for whatever reason.
๐ฌ martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148594367)
nit: maybe add `+ 2` to the reserve because when the next `if` is true 2 more will be added. Otherwise the next `emplace_back` will most likely cause a capacity change
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148594367)
nit: maybe add `+ 2` to the reserve because when the next `if` is true 2 more will be added. Otherwise the next `emplace_back` will most likely cause a capacity change
๐ฌ vstoyanov commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484167539)
False alarm. The `bash -c` hack also worked for dnf the same way it did for apt, so now it is both not a big change and has no SuppressWarnings BS. I could still push the arrayification of $PACKAGES and $CI_BASE_PACKAGES though?
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484167539)
False alarm. The `bash -c` hack also worked for dnf the same way it did for apt, so now it is both not a big change and has no SuppressWarnings BS. I could still push the arrayification of $PACKAGES and $CI_BASE_PACKAGES though?
๐ฌ martinus commented on pull request "fix initialization in FastRandomContext: removes undefined behavior & uninitialized read":
(https://github.com/bitcoin/bitcoin/pull/23169#issuecomment-1484180043)
Now that #26153 was merged this is no relevant
(https://github.com/bitcoin/bitcoin/pull/23169#issuecomment-1484180043)
Now that #26153 was merged this is no relevant
โ
martinus closed a pull request: "fix initialization in FastRandomContext: removes undefined behavior & uninitialized read"
(https://github.com/bitcoin/bitcoin/pull/23169)
(https://github.com/bitcoin/bitcoin/pull/23169)