💬 dergoegge commented on pull request "[Silent Payments]: Base functionality":
(https://github.com/bitcoin/bitcoin/pull/27827#discussion_r1262496803)
Same as other suggestion
```suggestion
CKey AddTweak(const uint256& tweak32) const;

//! Tweak a secret key by multiplying it by a scalar value.
CKey MultiplyTweak(const uint256& tweak32) const;
```

💬 dergoegge commented on pull request "[Silent Payments]: Base functionality":
(https://github.com/bitcoin/bitcoin/pull/27827#discussion_r1262483480)
In my opinion, this flag doesn't belong here because only the receiver/sender of a silent payment would care about it, which would make it a piece of wallet meta-data.

Without knowing much about the wallet my guess would be that it should be stored on `CWalletTx`?

💬 MarcoFalke commented on pull request "Small refactoring of iterator type declaration":
(https://github.com/bitcoin/bitcoin/pull/28073#issuecomment-1634193795)
Thank you for your contribution. While this stylistic change makes sense on its own, it comes at a cost and risk for the project as a whole. The weak motivation for the change does not justify the burden that it places on the project. A burden could be any of the following:

* Time spent on review
* Accidental introduction of bugs
* (Silent) merge conflicts, either in the branch or a backport branch. Those conflicts demand further developer and reviewer time or introduce bugs.

For more in
...

💬 MarcoFalke commented on pull request "Small refactoring of iterator type declaration":
(https://github.com/bitcoin/bitcoin/pull/28073#discussion_r1262519160)
Should probably use the C++17 structured bindings, but again, probably not worth to change it, unless there are other changes

💬 MarcoFalke commented on pull request "Small refactoring of iterator type declaration":
(https://github.com/bitcoin/bitcoin/pull/28073#issuecomment-1634198701)
Closing for now. If you are looking for something else to work on, see:


## Getting started to contribute to Bitcoin Core

### Setting up your development environment

New developers are very welcome and needed. There are a lot of open issues of any difficulty waiting to be fixed. However, before you start contributing, familiarize yourself with the Bitcoin Core build system and tests. Refer to the documentation in the repository on how to build Bitcoin Core and how to run the unit and f
...

✅ MarcoFalke closed a pull request: "Small refactoring of iterator type declaration"
(https://github.com/bitcoin/bitcoin/pull/28073)

💬 MarcoFalke commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#issuecomment-1634208153)
> I find it slightly unsatisfying that we can't break any 8-byte repetitive pattern.

I just find it difficult to think about without a use-case. The only thing it may affect is that it makes on-disk compression harder, but 8-byte XOR already makes that hard, so I don't think this is an argument (for or against) either?

💬 fanquake commented on issue "valgrind: Syscall param ppoll(ufds.events) points to uninitialised byte(s)":
(https://github.com/bitcoin/bitcoin/issues/28072#issuecomment-1634233433)
Opened a bug report with Valgrind: https://bugs.kde.org/show_bug.cgi?id=472219

👍 theStack approved a pull request: "Make poly1305 support incremental computation + modernize"
(https://github.com/bitcoin/bitcoin/pull/27993#pullrequestreview-1528432509)
ACK 4e5c933f6a40c07d1c68115f7979b89a5b2ba580

💬 sipa commented on pull request "Make poly1305 support incremental computation + modernize":
(https://github.com/bitcoin/bitcoin/pull/27993#issuecomment-1634237988)
@MarcoFalke You may be interested in the `Span<std::byte>` usage here.

💬 hebasto commented on pull request "ci: Update "Win64 native" task":
(https://github.com/bitcoin/bitcoin/pull/26891#issuecomment-1634247573)
> FWIW, the recent [ccache 4.8.1](https://ccache.dev/releasenotes.html#_ccache_4_8_1) seems broken. Note the cacheable calls ratio:

I found the reason that ccache >= 4.8 is not working as expected. We need to have `<ObjectFileName>$(IntDir)%(FileName).obj</ObjectFileName>` in every project file.

However, in the light of upcoming CMake-based build system, I see no reasons to change our current `build_msvc` directory.

📝 Ayush170-Future opened a pull request: "fuzz: wallet, add target for `Crypter`"
(https://github.com/bitcoin/bitcoin/pull/28074)
This PR adds fuzz coverage for `wallet/crypter`.

Motivation: Issue [27272](https://github.com/bitcoin/bitcoin/issues/27272#issue-1628327906)


I ran this for a long time with Sanitizers on and had no crashes; the average exec/sec also looks good to me. However, I would really appreciate it if some of the reviewers could try it on their machines too, and give their feedback.

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#issuecomment-1634282656)
In addition to this and `CoinControl`, I'm working on the `FeeBumper` and `Spend` files. The majority of my work in FeeBumper is finished, and I'll open a PR for it as soon as we reach a conclusion on this PR.
I'm currently working on the Spend file, which should be finished this week or next, hopefully.

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262608123)
thanks

💬 MarcoFalke commented on pull request "fuzz: wallet, add target for `Crypter`":
(https://github.com/bitcoin/bitcoin/pull/28074#discussion_r1262622915)
```suggestion
/*nDerivationMethod=*/ nDerivationMethod);

```

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262624822)
ah great thanks

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262625689)
:+1:

💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262632689)
good catch thanks

💬 kristapsk commented on pull request "RPC: Add universal options argument to listtransactions":
(https://github.com/bitcoin/bitcoin/pull/22807#issuecomment-1634333616)
Now it builds, but backwards compatibility with passing number instead of object as a third parameter is broken, will look at it later.

👍 brunoerg approved a pull request: "fuzz: wallet, add target for `Crypter`"
(https://github.com/bitcoin/bitcoin/pull/28074#pullrequestreview-1528586326)
ACK 6c98f4c137c9d557c78ebd52379711ebbd23e24a

non blocker, just an idea: Perhaps we could have encrypt/decrypt stuff outside of `LIMITED_WHILE` and then we could have a call just to fill them. I'm suppose this way we would have more chance of decrypting a previous encrypted thing. Just an example:

```diff
diff --git a/src/wallet/test/fuzz/crypter.cpp b/src/wallet/test/fuzz/crypter.cpp
index 7ba43821b..03ad97a35 100644
--- a/src/wallet/test/fuzz/crypter.cpp
+++ b/src/wallet/test/fuzz/cry
...