💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560776211)
> I can reorder them if you think it's worth it.

I don't understand what you are trying to say.

I am saying that the order was correct before and you changed it to be wrong.

💬 paplorinc commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560780178)
I added a new import and the imports were reorganized as such. If you don't like the order, I can change it, I don't mind, though I would prefer the CI telling me these in the PR instead of wasting time reviewing styles.

💬 sipa commented on pull request "feefrac: avoid explicitly computing diagram; compare based on chunks":
(https://github.com/bitcoin/bitcoin/pull/29757#issuecomment-2049368065)
@dergoegge Do those fuzz seeds trigger issues on master too?

💬 paplorinc commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560781776)
> I don't understand what you are trying to say.

I'm trying to find out if my contributions are welcome, I've only received hostility so far in return for my invested effort.
It's a decentralized, open source project, I'll contribute what I can and want. If that's not welcome, I'll find a different project where my optimizations are welcome.

💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560795419)
> I'm trying to find out if my contributions are welcome, I've only received hostility so far in return for my invested effort.

Contributions are welcome and contributors are needed. However, refactoring changes are usually the wrong pick for a newcomer to tackle. This is explained https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring

💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560796570)
See also https://bitcoincore.academy/

If you are looking for useful contributions to help out with, you can

* Search through the [good first issue](https://github.com/bitcoin/bitcoin/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22)s or the ones that are [up for grabs](https://github.com/bitcoin/bitcoin/issues?utf8=%E2%9C%93&q=label%3A%22Up+for+grabs%22). Some of them might no longer be applicable. So if you are interested, but unsure, you might want to leave a comment on the i
...

💬 paplorinc commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560798732)
I was looking for introductory issues, couldn't find any that was a good fit for me.
But just looking at the code I found several optimization opportunities in relevant parts of the code, so I contributed those instead, e.g. [the 4x speedup of base58](https://github.com/bitcoin/bitcoin/pull/29473).

💬 laanwj commented on issue "contrib: add symbol-check test for non-existence of `vmova` instructions in Windows build":
(https://github.com/bitcoin/bitcoin/issues/28413#issuecomment-2049395693)
Wouldn't checking the instructions be more of a security-check instead of a symbol-check?

Assigning myself as I'm interested on working on this.

💬 paplorinc commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560805246)
Do you consider optimizations useless refactorings? Most project consider them features.
For me it's a gentle introduction to the codebase, isolated from other complexities, I poke it left-and-right and see how the code behaves.
Is this not a valuable contribution? Especially since I usually cover the corner cases I find with new test cases.

💬 maflcko commented on pull request "refactor/test: add a few more base32/64 calculation corner cases":
(https://github.com/bitcoin/bitcoin/pull/29847#discussion_r1560850757)
Tests are generally a good way to start contributing.

Just generally for each pull request, it needs review before merge, and each reviewer will have to trade-off whether it is worth it to review. There are 316 other pull requests open waiting for review, so that is another thing for reviewers to trade-off on.

💬 instagibbs commented on pull request "feefrac: avoid explicitly computing diagram; compare based on chunks":
(https://github.com/bitcoin/bitcoin/pull/29757#issuecomment-2049475920)
looks to be happening on master and I think both reports are from the same underlying issue. Investigating.

💬 S3RK commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#discussion_r1560921100)
This would conflate user entered data with programmatically generated data. Address book today is managed by the user. If we want to store a list of all addresses we sent to we should pick a new storage for that.

📝 hebasto opened a pull request: "Fix typos in `subprocess.hpp`"
(https://github.com/bitcoin/bitcoin/pull/29849)
Resolves one item in the https://github.com/bitcoin/bitcoin/pull/28981#pullrequestreview-1991272752:
> - Remove linter exclusions and fix all issues.

Based on upstream https://github.com/arun11299/cpp-subprocess/pull/101.

💬 S3RK commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-2049553912)
I'm still not convinced about the approach of storing programmatically generated data in the address book, which is supposed to store user entered data.

For that reason I'm leaning Approach NACK.

💬 hebasto commented on pull request "Fix typos in `subprocess.hpp`":
(https://github.com/bitcoin/bitcoin/pull/29849#issuecomment-2049556946)
cc @theStack

👍 fanquake approved a pull request: "Fix typos in `subprocess.hpp`"
(https://github.com/bitcoin/bitcoin/pull/29849#pullrequestreview-1994047870)
ACK 13f5391bbb45cd8aebc6ae70cad08aff632ebd55

💬 laanwj commented on issue "guix: re-enable exported symbol checking for RISC-V":
(https://github.com/bitcoin/bitcoin/issues/28095#issuecomment-2049584868)
i'll try to figure out why it exports those three symbols, and if it is indication of a problem, and if not, add them to the exceptions

demangled, these are:
```
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::basic_string<std::allocator<char> >(char const*, std::allocator<char> const&)
std::__cxx11::basic_stringbuf<char, std::char_traits<char>, std::allocator<char> >::~basic_stringbuf()
std::ctype<char>::do_widen(char) const
```

💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1560951419)
updated the exit error message

💬 theuni commented on pull request "ci: use Clang 16 for Valgrind":
(https://github.com/bitcoin/bitcoin/pull/29848#issuecomment-2049594993)
Concept ACK

💬 theuni commented on pull request "guix: replace GCC unaligned VMOV patch with binutils patch":
(https://github.com/bitcoin/bitcoin/pull/29846#issuecomment-2049597893)
Concept ACK, looks like a strict improvement.