💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262421705)
What is the meaning of this comment here?

💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262436716)
The `Proxy::proxy` member is still accessed directly from some high level code:

```
src/net.cpp:494: std::make_unique<i2p::sam::Session>(proxy.proxy, &interruptNet);
src/qt/optionsdialog.cpp:416: ui->proxyReachIPv4->setChecked(has_proxy && proxy.proxy == ui_proxy);
src/qt/optionsdialog.cpp:419: ui->proxyReachIPv6->setChecked(has_proxy && proxy.proxy == ui_proxy);
src/qt/optionsdialog.cpp:422: ui->proxyReachTor->setChecked(has_proxy && proxy.proxy == ui_
...

📝 Lehonti opened a pull request: "Small refactoring of iterator type declaration"
(https://github.com/bitcoin/bitcoin/pull/28073)
There are a few functions in `httpserver.cpp` (`HTTPBindAddresses` and `UnregisterHTTPHandler`) that are declaring `std::vector::iterator` explicitly, which is really verbose and visually noisy. My guess is that they were written pre-C++11 and they would be written differently today, so I modified them a little and used `auto` instead.

Also, I inverted the logic of an `if` block in `UnregisterHTTPHandler` in order to remove a nesting level.

💬 dergoegge commented on pull request "[Silent Payments]: Base functionality":
(https://github.com/bitcoin/bitcoin/pull/27827#discussion_r1262495049)
Maybe make this a uint256&? Passing a raw pointer without a size (expecting it to be 32 byte) seems a little bug prone.
```suggestion
XOnlyPubKey AddTweak(const uint256& tweak32) const;
```

💬 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: