💬 TysonsTech commented on issue "Wallet passphrases silently ignore everything after a null character":
(https://github.com/bitcoin/bitcoin/issues/27067#issuecomment-1427355467)
From what I understand the issue occurs because the string is terminated by a null character and anything after the null character is ignored. This means that if the passphrase is used to secure the wallet and includes a null character, the software may be processing only a portion of the intended passphrase, potentially leading to the wallet being unlocked using an incorrect or malicious passphrase.
I agree using a SecureString can help mitigate the issue of passphrases silently ignoring ever
...

💬 TysonsTech commented on issue "Request support for HRNG on ARM using their new RNDR / RNDRRS instructions.":
(https://github.com/bitcoin/bitcoin/issues/26796#issuecomment-1427377914)
Hope this helps and is a correct example of using the RNDR and RNDRRS instructions in ARM assembly language for bitcoin mining:

mov x0, #0
mrs x1, cntpct_el0
mov x2, #32
rndr x3, x0, x1, x2

; Use the generated random number as a nonce in a hash function
mov x4, x3
bl hash_function

; Compare the resulting hash with the target difficulty
mov x5, TARGET_DIFFICULTY
cmp x4, x5
bgt retry

; If the hash is less than the target, it is a valid soluti
...

💬 S3RK commented on pull request "Wallet: Zero out wallet master key upon locking so it doesn't persist in memory":
(https://github.com/bitcoin/bitcoin/pull/27080#issuecomment-1427481658)
Code review ACK 3a11adc7004d21b3dfe028b190d83add31691c55

💬 johnnypham commented on issue "make check FAIL: qt/test/test_bitcoin-qt.exe WSL":
(https://github.com/bitcoin/bitcoin/issues/22248#issuecomment-1427482030)
Disabling the gui tests works for me in WSL. Thanks!

💬 MarcoFalke commented on issue "24.0.1 crash on restart":
(https://github.com/bitcoin/bitcoin/issues/27088#issuecomment-1427509199)
Maybe upload the full log here?

💬 naumenkogs commented on pull request "[WIP] p2p: Add random txn's from mempool to GETBLOCKTXN":
(https://github.com/bitcoin/bitcoin/pull/27086#issuecomment-1427568262)
I agree with @sipa, with a stronger emphasis that I would probably NACK this change because the cost of this fix is too high, and the privacy gain is too low.

You may be interested in contributing to some SPV client implementation instead :) I'm curious how well they preserve privacy when they request transactions/blocks (that subset which is of interest to them specifically). E.g. whether they ask the same node to provide everything — then the node can correlate.

💬 hebasto commented on pull request "refactor, wallet: Avoid variable shadowing":
(https://github.com/bitcoin/bitcoin/pull/27087#issuecomment-1427632143)
> Could remove the top variable instead of renaming the internal one: [furszy@3fb01a9](https://github.com/furszy/bitcoin-core/commit/3fb01a94aa29dec9c546c0d0e9e54a513b3dee43).

Thanks! Done.

💬 naumenkogs commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1427705233)
Concept ACK.
I think the 43% saving is a big deal, and the code change amount seems fine, although worth prototyping those corner cases first :)

W.r.t. the security reduction pointed out by @petertodd, I agree with the @JoseSK999 [comment](https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1426014422) almost entirely.
It could be emphasized further — 43% saving by itself helps the node-running culture as a whole (more than it hurts it by dropping witness checks).

I agree with @sd
...

💬 MarcoFalke commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#discussion_r1104282294)
Allowing for a `void` failure type seems to make this incompatible to be switched out to `std::expected` https://en.cppreference.com/w/cpp/utility/expected ?

With multiple warning and error messages this may already be incompatible, though?

💬 MarcoFalke commented on pull request "[WIP] p2p: Add random txn's from mempool to GETBLOCKTXN":
(https://github.com/bitcoin/bitcoin/pull/27086#issuecomment-1427723661)
Wouldn't it be better to not add wallet transactions to the mempool if we don't want peers to query our mempool for wallet transactions?

See also https://github.com/bitcoin/bitcoin/issues/11887#issuecomment-1061946262 (and all in- and out- links in this issue)

✳️ MarcoFalke pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/b92d609fb256...a6316590d5bc)
Merge bitcoin/bitcoin#26970: test: fix immediate tx relay in wallet_groups.py

ab4efad51b9ba276ffeb6871931e13772493f7cc test: fix immediate tx relay in wallet_groups.py (Sebastian Falbesoner)

Pull request description:

In the functional test wallet_groups.py we whitelist peers on all nodes (`-whitelist=noban@127.0.0.1`) to enable immediate tx relay for fast mempool synchronization. However, considering that this setting only applies to inbound peers and the default test topology looks like this:
```
node0 <--- node1 <---- node2 <--- ... <-- nodeN
```

txs propagate fast only from lower- to higher-numbered nodes (i.e. "left to right" in the above diagram) and take long from higher- to lower-numbered nodes ("right to left") since in the latter direction we only have outbound peers, where the trickle relay is still active. As a consequence, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long.

This PR fixes this by simply adding another connection from node0 to the last node, leading to a ~2-3x speedup (5 runs measured via `time ./test/functional/wallet_groups.py` are shown):

```
master:
0m53.31s real 0m08.22s user 0m05.60s system
0m32.85s real 0m07.44s user 0m04.08s system
0m46.40s real 0m09.18s user 0m04.23s system
0m46.96s real 0m11.10s user 0m05.74s system
0m57.23s real 0m10.53s user 0m05.59s system

PR:
0m19.64s real 0m09.58s user 0m05.50s system
0m18.05s real 0m07.77s user 0m04.03s system
0m18.99s real 0m07.90s user 0m04.25s system
0m17.49s real 0m07.56s user 0m03.92s system
0m18.11s real 0m07.74s user 0m03.88s system
```
Note that in most tests this is not a problem since txs very often originate from node0.

ACKs for top commit:
brunoerg:
utACK ab4efad51b9ba276ffeb6871931e13772493f7cc

Tree-SHA512: 12675357e6eb5a18383f2bfe719a184c0790863b37a98749d8e757dd5dc3a36212e16a81f0a192340c11b793eda00db359c7011f46f7c27e3a093af4f5b62147

🚀 MarcoFalke merged a pull request: "test: fix immediate tx relay in wallet_groups.py"
(https://github.com/bitcoin/bitcoin/pull/26970)

✳️ MarcoFalke pushed commits to a branch: bitcoin/bitcoin:master
(https://github.com/bitcoin/bitcoin/compare/a6316590d5bc...141115a06040)
Merge bitcoin/bitcoin#27033: ci: Cache stuff in volumes, not host folders

fa8e92c022057adcb8b98647bde626ed9c054df2 doc: Update ci docs (721217.xyz)
5fffff54e9fcf154c722dc421025a567fa0c5c97 ci: Cache stuff in volumes, not host folders (MarcoFalke)

Pull request description:

Storing cached stuff in host system folders may lead to unexpected issues when the ci-built stuff is used for a non-ci build or a ci task leaks into another ci task.

ACKs for top commit:
john-moffett:
ACK fa8e92c022057adcb8b98647bde626ed9c054df2

Tree-SHA512: 8b0c9019452fbe507a272c1037c3dce3c178c21f85ab1096ed3372ad9d4b3c7aa27d89e5bf80c9a6260ea652e0268be0cbe61d6a4fcb3add569fa38076d32287

🚀 MarcoFalke merged a pull request: "ci: Cache stuff in volumes, not host folders"
(https://github.com/bitcoin/bitcoin/pull/27033)

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104354933)
Thanks, I adapted these changes with @pinheadmz name change suggested below

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104355153)
Agreed, and renamed.

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104355315)
agreed

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104356262)
Now returns a file path

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#issuecomment-1427800810)
Thanks @stickies-v & @pinheadmz , I addressed your comments in the new push.

💬 stickies-v commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1104364182)
nit
```suggestion
const auto path{GetDataDir(net_specific)};
```