💬 l0rinc commented on pull request "optimization: Speed up Base58 encoding/decoding by 400%/200% via preliminary byte packing":
(https://github.com/bitcoin/bitcoin/pull/29473#issuecomment-2498871171)
I just found this was already attempted in a very similar way by @martinus a few years ago in https://github.com/bitcoin/bitcoin/pull/21176 - so now we have two versions, if this will turn out to be important again.
Closing, thanks for the reviews!

👍 theuni approved a pull request: "refactor: Fix remaining clang-tidy performance-inefficient-vector errors"
(https://github.com/bitcoin/bitcoin/pull/31305#pullrequestreview-2459478618)
ACK 11f3bc229ccd4b20191855fb1df882cfa6145264

💬 l0rinc commented on pull request "refactor: Fix remaining clang-tidy performance-inefficient-vector errors":
(https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2498906183)
Note that while [the benchmarks](https://corecheck.dev/bitcoin/bitcoin/pulls/31305) indicate a speedup, [they're actually the ones that were changed](https://github.com/bitcoin/bitcoin/pull/31305/files#diff-3a60bcd828090e387348302ccc8453c055170d5b5ddb828cd890228191d0fe42R90-R103):
<img width="633" alt="image" src="https://github.com/user-attachments/assets/808d3dac-d91a-4353-ac68-e078124fd7c6">

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1857275360)
> > NOLINT comments are easy searchable and effectively document the issues that have to be resolved.
>
> Sorry, but no, that is exactly the _opposite_ of what `NOLINT` documents!
>
> `NOLINT` means "I'm smarter than my static analyzer".
>
> `TODO` would effectively document an issue to be resolved.
>
> Hard NACK here due to the reasoning being applied. In fact, it rises to the level of suspicion. I'll be reviewing all of these refactors in great detail.

A bare `NOLINT`, without a
...

✅ hebasto closed a pull request: "ci: Update Clang in "tidy" job"
(https://github.com/bitcoin/bitcoin/pull/31306)

💬 theuni commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1857275666)
As mentioned [here](https://github.com/bitcoin/bitcoin/pull/31364#discussion_r1857161138), this change struck me as dangerous.

> It shouldn't be marked nolint, rather it should be fixed: drop the forward and pass by const Args&.

I understand the desire to turn on new checks and notate the false-positives, but the result should _always_ be better/safer/cleaner code.

In this case, the result would've been a latent bug explicitly marked notabug. And while a buggy test isn't the end of the
...

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1857278221)
Right.

📝 ijunxyz123 opened a pull request: "Update COPYING"
(https://github.com/bitcoin/bitcoin/pull/31368)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1857283241)
> As mentioned [here](https://github.com/bitcoin/bitcoin/pull/31364#discussion_r1857161138), this change struck me as dangerous.

My apologies.

An additional context available here: https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1846324884.

✅ fanquake closed a pull request: "Update COPYING"
(https://github.com/bitcoin/bitcoin/pull/31368)

📝 fanquake locked a pull request: "Update COPYING"
(https://github.com/bitcoin/bitcoin/pull/31368)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 theuni commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1857286619)
> A bare NOLINT, without any explanation, has never meant "I'm smarter than my static analyzer"—at least, not to me.

I'm not sure what else it could mean.

[From the docs](https://clang.llvm.org/extra/clang-tidy/): "clang-tidy diagnostics are intended to call out code that does not adhere to a coding standard, or is otherwise problematic in some way. However, if the code is known to be correct, it may be useful to silence the warning"

Does this mean there are other places in the code whe
...

📝 ijunxyz123 opened a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31369)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

👍 theuni approved a pull request: "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors"
(https://github.com/bitcoin/bitcoin/pull/31364#pullrequestreview-2459538939)
ACK the much more constrained 3305972f7bfd78181566e4297891c2dd7cae0f2b.

📝 ijunxyz123 opened a pull request: "Update developer-notes.md"
(https://github.com/bitcoin/bitcoin/pull/31370)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

💬 hebasto commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1857290156)
> I'm not sure what else it could mean.
>
> [From the docs](https://clang.llvm.org/extra/clang-tidy/): "clang-tidy diagnostics are intended to call out code that does not adhere to a coding standard, or is otherwise problematic in some way. However, if the code is known to be correct, it may be useful to silence the warning"

You are right.

✅ willcl-ark closed a pull request: "Update developer-notes.md"
(https://github.com/bitcoin/bitcoin/pull/31370)

✅ willcl-ark closed a pull request: "Update README.md"
(https://github.com/bitcoin/bitcoin/pull/31369)

💬 willcl-ark commented on pull request "Update README.md":
(https://github.com/bitcoin/bitcoin/pull/31369#issuecomment-2498948670)
Please don't open pull requests like this here, you can use your own fork for testing things out.

📝 theStack opened a pull request: "doc, test: more ephemeral dust follow-ups"
(https://github.com/bitcoin/bitcoin/pull/31371)
This small PR contains ephemeral dust follow-ups mentioned in #30329 that were not tackled in the first follow-up PR #31279:

https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1828577696
https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1825279952

Happy to add more if I missed some or anyone has concrete commits to add.