💬 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

✅ martinus closed a pull request: "fix initialization in FastRandomContext: removes undefined behavior & uninitialized read"
(https://github.com/bitcoin/bitcoin/pull/23169)

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484197246)
Updated 09383b612216b63f2aec0f1fffc889989c66f212 -> 03ec5b6f9ca3af28c9ce25cf2393e28ae852d808 ([pr26642.12](https://github.com/hebasto/bitcoin/commits/pr26642.12) -> [pr26642.13](https://github.com/hebasto/bitcoin/commits/pr26642.13), [diff](https://github.com/hebasto/bitcoin/compare/pr26642.12..pr26642.13)):

- addressed @martinus's [comment](https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148594367)

💬 hebasto commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1148626538)
Thanks! [Updated](https://github.com/bitcoin/bitcoin/pull/26642#issuecomment-1484197246).

💬 Ayush170-Future commented on issue "ci: Cleanup ci/test/01_base_install.sh":
(https://github.com/bitcoin/bitcoin/issues/27321#issuecomment-1484198989)
I guess SuppressWarnings was not a good practice. It's great that you found a workaround.

Instead of giving package strings as an array, I believe the bash -c technique should be sufficient here. @MarcoFalke, I think you wanted it that way as well.

💬 theuni commented on pull request "depends: fix osx build with clang 16":
(https://github.com/bitcoin/bitcoin/pull/27328#issuecomment-1484213695)
> It fixes building `make -C depends HOST=x86_64-apple-darwin FORCE_USE_SYSTEM_CLANG=1` with clang-16.

Yes, exactly this thanks.

This can be verified very simply outside of depends:
Broken:
> $ /opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/bin/clang -c test.c -o test.o -Xclang -internal-externc-isystem/tmp
> error: unknown argument: '-internal-externc-isystem/tmp'

Working:
> $ /opt/clang+llvm-16.0.0-x86_64-linux-gnu-ubuntu-18.04/bin/clang -c test.c -o test.o -Xclang -intern
...

👍 TheCharlatan approved a pull request: "depends: fix osx build with clang 16"
(https://github.com/bitcoin/bitcoin/pull/27328)
ACK 87afcb0029b8dab933c122fb8f7263c2e7272731

💬 Empact commented on pull request "Torcontrol opt check":
(https://github.com/bitcoin/bitcoin/pull/25136#issuecomment-1484225313)
@vstoyanov No need to ask for permission to contribute. I'd say go for it.