✅ MarcoFalke closed a pull request: "Small refactoring of iterator type declaration"
(https://github.com/bitcoin/bitcoin/pull/28073)
(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?
(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
(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
(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.
(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.
(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.
(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.
(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
(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);
```
(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
(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:
(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
(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.
(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
...
(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
...
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262645801)
ah, thanks. I had a hard time with this one for some reason because casting to `sockaddr` kept truncating the unix socket path to 14 bytes before it got to `Connect()`. Anyway this works and I don't know why I didn't try it like this at first.
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262645801)
ah, thanks. I had a hard time with this one for some reason because casting to `sockaddr` kept truncating the unix socket path to 14 bytes before it got to `Connect()`. Anyway this works and I don't know why I didn't try it like this at first.
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262653604)
Ok thanks I added the preprocessor logic to the function, I'll look into adding afunix.h for windows, maybe in compat.h? also add it to the first commit to test for AF_UNIX in configure.ac
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262653604)
Ok thanks I added the preprocessor logic to the function, I'll look into adding afunix.h for windows, maybe in compat.h? also add it to the first commit to test for AF_UNIX in configure.ac
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262664857)
great catch thanks yeah I saw that in your commit but I was confused. makes sense now
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262664857)
great catch thanks yeah I saw that in your commit but I was confused. makes sense now
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262667486)
got it thanks, fixed this when changing sockaddr_storage to sockaddr_un and it's now `addrun`
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262667486)
got it thanks, fixed this when changing sockaddr_storage to sockaddr_un and it's now `addrun`
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262690201)
ok fixed this, just using default (with auth) for the test
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1262690201)
ok fixed this, just using default (with auth) for the test