Bitcoin Core Github
44 subscribers
120K links
Download Telegram
๐Ÿ’ฌ 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.
๐Ÿ’ฌ 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.
โœ… aureleoules closed a pull request: "wallet: Use defined purposes instead of inlining"
(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
...
๐Ÿ‘ 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
...
๐Ÿ“ 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
๐Ÿ’ฌ 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
๐Ÿ’ฌ 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?
๐Ÿ‘ 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.
๐Ÿ’ฌ 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.
๐Ÿ’ฌ 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).
๐Ÿ“ 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 |
...
๐Ÿ’ฌ 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.
๐Ÿ’ฌ 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
...
๐Ÿ’ฌ Ferrydh commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(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.
๐Ÿ’ฌ 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.
๐Ÿ’ฌ 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
๐Ÿ’ฌ 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?
๐Ÿ’ฌ 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