🤔 sipa reviewed a pull request: "test: add python implementation of Elligator swift"
(https://github.com/bitcoin/bitcoin/pull/24005#pullrequestreview-1505500030)
Code review ACK afd762afa39569d8932dd0cf62f8d134e25fc651

💬 sipa commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1246749262)
Perhaps it makes sense to assert here that X is actually on the curve.

`assert GE.is_valid_x(x)`

💬 sipa commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1246752385)
Would it make sense to include the BIP324 test vectors here? (https://github.com/bitcoin/bips/blob/master/bip-0324/ellswift_decode_test_vectors.csv and https://github.com/bitcoin/bips/blob/master/bip-0324/xswiftec_inv_test_vectors.csv). See `test_schnorr_testvectors` in test/functional/test_framework/key.py for an example that involves CSV files.

💬 Empact commented on pull request "Enable -Wstring-concatenation and -Wstring-conversion on clang builds":
(https://github.com/bitcoin/bitcoin/pull/26288#discussion_r1246795871)
Done

💬 Empact commented on pull request "Enable -Wstring-concatenation and -Wstring-conversion on clang builds":
(https://github.com/bitcoin/bitcoin/pull/26288#discussion_r1246796464)
Done

💬 Empact commented on pull request "Enable -Wstring-concatenation and -Wstring-conversion on clang builds":
(https://github.com/bitcoin/bitcoin/pull/26288#discussion_r1246797170)
Done

💬 Empact commented on pull request "Enable -Wstring-concatenation and -Wstring-conversion on clang builds":
(https://github.com/bitcoin/bitcoin/pull/26288#issuecomment-1613406421)
Rebased. 👍

💬 fanquake commented on pull request "build: LLVM 15 & LLD based macOS toolchain":
(https://github.com/bitcoin/bitcoin/pull/21778#issuecomment-1613444184)
Rebased on master, and put on top of #27999. Also swapped out the time-machine bumping commit to be the same one as from #27897.

👍 theStack approved a pull request: "test: add python implementation of Elligator swift"
(https://github.com/bitcoin/bitcoin/pull/24005#pullrequestreview-1505614994)
Code-review ACK afd762afa39569d8932dd0cf62f8d134e25fc651

Regarding https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1246752385, I've prepared at least adding the BIP324 decoding test vectors today (https://github.com/theStack/bitcoin/commit/06c942493af9df766f85dc1387daaa0441e7d10e). Feel free to pick it up either in this PR or a follow-up.

💬 theStack commented on pull request "test: add python implementation of Elligator swift":
(https://github.com/bitcoin/bitcoin/pull/24005#discussion_r1246822528)
non-blocking nit:
```suggestion
self.assertEqual(shared_secret1, shared_secret2)
```
(slightly preferred, as in the case of a mismatch the values would be shown)

💬 sipa commented on pull request "refactor: Drop unsafe AsBytePtr function":
(https://github.com/bitcoin/bitcoin/pull/27978#issuecomment-1613459921)
utACK 7c853619ee9ea17a79f1234b6c7871a45ceadcb9

🤔 vasild reviewed a pull request: "net: support unix domain sockets for -proxy and -onion"
(https://github.com/bitcoin/bitcoin/pull/27375#pullrequestreview-1505515197)
64d0f234e9 looks good, except there is some messup in the contents of these two commits:

87064712899dacfac7f66f60c8ed1c0f4e65c24f `netbase: refactor CreateSock() to accept sa_family_t`
bbff39d3619428df3745d884ea36d7febff337b1 `netbase: extend Proxy class to wrap UNIX socket as well as TCP`

The first one introduces one more `IsValid()` method, in addition to the existent one. The new method uses a variable that does not exist: `is_unix_socket`, thus it will not compile.
Then the other com
...

💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1246758964)
nit: it does not have to be `const` when passing by value - there is no way to for the function to modify the variable that is being passed (and have effects visible outside of the function).

```suggestion
std::unique_ptr<Sock> CreateSockOS(sa_family_t address_family);
```

See f3() in http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-in

💬 vasild commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1246856004)
The new member `unix_socket_path` is still without `m_` prefix. Only `is_unix_socket` was renamed to `m_is_unix_socket`.

📝 fanquake opened a pull request: "doc: cleanup release process doc"
(https://github.com/bitcoin/bitcoin/pull/28003)
To-be-updated collection of changes to to simplify / correct the release-process documentation.
I think we could still simplify this further. For example, we could remove the guix building docs, and defer to `contrib/guix`.

📝 Qoutip opened a pull request: "See text - *"
(https://github.com/bitcoin/bitcoin/pull/28004)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "See text - *"
(https://github.com/bitcoin/bitcoin/pull/28004)

📝 fanquake locked a pull request: "See text - *"
(https://github.com/bitcoin/bitcoin/pull/28004)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

⚠️ john-moffett opened an issue: "Command line options after any non-dash arguments are silently ignored"
(https://github.com/bitcoin-core/gui/issues/741)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

For example, invoking `bitcoin-qt -server=1 foo -regtest` on a fresh install will run `mainnet` instead of the expected `regtest`.



### Expected behaviour

I would expect it to do one of the following:

1. Run `regtest`
2. Warn me that my arguments contained a "command" and any subsequent options were ignored
3. Exit with an error message

The problem is that `ArgsManager::ParseP
...

📝 john-moffett opened a pull request: "Exit and show error if unrecognized command line args are present"
(https://github.com/bitcoin-core/gui/pull/742)
Fixes #741

Starting bitcoin-qt with non-dash ("-") arguments causes it to silently ignore any later valid options. For instance, invoking `bitcoin-qt -server=1 foo -regtest` on a fresh install will run `mainnet` instead of `regtest`.

This change makes the client exit with an error message if any such "loose" arguments are encountered. This mirrors how `bitcoind` handles it:

https://github.com/bitcoin/bitcoin/blob/c6287faae4c0e705a9258a340dfcf548906f12af/src/bitcoind.cpp#L127-L132