👍 hebasto approved a pull request: "guix: combine and document `enable_werror`"
(https://github.com/bitcoin/bitcoin/pull/27326)
ACK 4becee396f3bda40832138dd1aaa90368ed31857, the diff is correct.
(https://github.com/bitcoin/bitcoin/pull/27326)
ACK 4becee396f3bda40832138dd1aaa90368ed31857, the diff is correct.
💬 pablomartin4btc commented on pull request "httpserver, rest: fix segmentation fault on evhttp_uri_get_query":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1483882249)
Regarding CI failures,` ./test/functional/wallet_create_tx.py` passes locally, actioned manually re-run and I'm checking the fuzzer one `cirrus-ci-build/ci/scratch/qa-assets/fuzz_seed_corpus/http_request" failed with exit code 77`.
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1483882249)
Regarding CI failures,` ./test/functional/wallet_create_tx.py` passes locally, actioned manually re-run and I'm checking the fuzzer one `cirrus-ci-build/ci/scratch/qa-assets/fuzz_seed_corpus/http_request" failed with exit code 77`.
💬 Ayush170-Future commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1483895345)
Hello @MarcoFalke!
I'm new to Bitcoin Core development as well as Bash scripting. But, based on the requirements and the script, I believe I can tackle this.
I only have a few questions:
1) I discovered other files in the test directory, such as ci/test/04_install, also contain the CI_EXEC_ROOT and CI_EXEC alias. So, are they also to be modified?
2) After reading your comment in the script, I believe I can just remove the CI_EXEC_ROOT and CI_EXEC parts of different commands in the script
...
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1483895345)
Hello @MarcoFalke!
I'm new to Bitcoin Core development as well as Bash scripting. But, based on the requirements and the script, I believe I can tackle this.
I only have a few questions:
1) I discovered other files in the test directory, such as ci/test/04_install, also contain the CI_EXEC_ROOT and CI_EXEC alias. So, are they also to be modified?
2) After reading your comment in the script, I believe I can just remove the CI_EXEC_ROOT and CI_EXEC parts of different commands in the script
...
💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1148418379)
nit: unused variable, can be removed
```suggestion
```
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1148418379)
nit: unused variable, can be removed
```suggestion
```
💬 theStack commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1148418808)
pedantic-nit:
```suggestion
// Note that each transaction's ancestor size is 1 or 3, and each descendant size is 1, 2 or 3.
```
The descendant count of 2 is the rare case here, only applying for parent-txs txp0 and txp49
(for the fun of it, created a functional test for verifying this: https://github.com/theStack/bitcoin/blob/functional_test_mempool_zigzag_playground/test/functional/mempool_zigzag.py).
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1148418808)
pedantic-nit:
```suggestion
// Note that each transaction's ancestor size is 1 or 3, and each descendant size is 1, 2 or 3.
```
The descendant count of 2 is the rare case here, only applying for parent-txs txp0 and txp49
(for the fun of it, created a functional test for verifying this: https://github.com/theStack/bitcoin/blob/functional_test_mempool_zigzag_playground/test/functional/mempool_zigzag.py).
👍 TheCharlatan approved a pull request: "guix: combine and document `enable_werror`"
(https://github.com/bitcoin/bitcoin/pull/27326)
ACK 4becee396f3bda40832138dd1aaa90368ed31857
(https://github.com/bitcoin/bitcoin/pull/27326)
ACK 4becee396f3bda40832138dd1aaa90368ed31857
👍 Ayaan7211 approved a pull request: "MiniTapscript: port Miniscript to Tapscript"
(https://github.com/bitcoin/bitcoin/pull/27255)
(https://github.com/bitcoin/bitcoin/pull/27255)
👍 aureleoules approved a pull request: "wallet: Replace use of purpose strings with an enum"
(https://github.com/bitcoin/bitcoin/pull/27217)
ACK a48010f1fd6d8a430b8234789ac3c91ec9d06972, looks good to me.
(https://github.com/bitcoin/bitcoin/pull/27217)
ACK a48010f1fd6d8a430b8234789ac3c91ec9d06972, looks good to me.
💬 aureleoules commented on pull request "wallet: Replace use of purpose strings with an enum":
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1148451397)
Could use `purpose.value_or(is_mine ? AddressPurpose::RECEIVE : AddressPurpose::SEND)` instead.
Same for other instances.
(https://github.com/bitcoin/bitcoin/pull/27217#discussion_r1148451397)
Could use `purpose.value_or(is_mine ? AddressPurpose::RECEIVE : AddressPurpose::SEND)` instead.
Same for other instances.
💬 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 |
...