💬 jamesob commented on pull request "script: utxo_snapshot.sh error handling":
(https://github.com/bitcoin/bitcoin/pull/27845#issuecomment-1613314386)
ACK https://github.com/bitcoin/bitcoin/pull/27845/commits/39aa958b2049bc50819d645052c0b90cf19f306e

But I think you can just squash these two commits down into one.

💬 MarcoFalke commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1613343824)
> By my measurement, this data structure with ~125,000 transactions requires ~14Mb of RAM.

If memory usage is the only concern, I am sure you could toss the transactions into (lossy) buckets and then subscribe via zmq to avoid duplicates, without having to remember txids, or individual size/fee details.

> ... applicable to higher time preference fee estimations?

If you only care about the top N blocks worth of transactions, the memory usage likely could also be reduced.

💬 MarcoFalke commented on issue "failure in wallet_basic.py --descriptors":
(https://github.com/bitcoin/bitcoin/issues/27249#issuecomment-1613349621)
I think adding it to the mempool should always be instantaneous. However, the background thread will still handle the AddedToMempool event, which may fire in the wallet when the transaction has already been removed from the mempool, and thus cause a wrong transaction state.

Not sure if this is what happened here, though.

🤔 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)