💬 pinheadmz commented on pull request "system: use %LOCALAPPDATA% as default datadir on windows":
(https://github.com/bitcoin/bitcoin/pull/27064#issuecomment-2085584460)
@hebasto thanks for the review, I addressed your comments

💬 ajtowns commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2085586513)
> > > I think most of the criticism of this PR from AJ is saying that the changes here are unnecessary, not really that they are harmful,

> > I said "As I've already said, I think `m_log.SetCategory(NET); m_log.Debug("connected to foo")` is much worse than `LogDebug(NET, "connected to foo")`." Does that really not come across as me saying they're harmful? I really don't think I'm being **that** unclear. Anyway, I'll bow out, I'm already sorry for touching this.

> I think it's possible you
...

👍 hebasto approved a pull request: "system: use %LOCALAPPDATA% as default datadir on windows"
(https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2031717243)
re-ACK 84900ac34f6888b7a851d0a6a5885192155f865c, only addressed feedback since my recent [review](https://github.com/bitcoin/bitcoin/pull/27064#pullrequestreview-2028718273).

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#issuecomment-2085668407)
Will fix the linter when I rebase

💬 Lukasz8181 commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2085763047)
> Using `2020-10-01` as the fake timestamp will cause many test failures with `/gnu/store/bfirgq65ndhf63nn4q6vlkbha9zd931q-openssl-1.1.1l.drv`. I didn't investigate why, but I guess because it's _before_ the test certificates were created. They expired in June 2022. I tried a month before that, which worked.
>
> Also fixes layout of instructions.

// Całkowita ilość tokenów
uint256 public totalSupply;

// Mapowanie sald dla każdego adresu
mapping(address => uint256) public balanceOf;

...

💬 flack commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2085779187)
FWIW "migrate" has a definition as a computer (or information system) term in common dictionaries, e.g.:

https://www.merriam-webster.com/dictionary/migrate

https://www.dictionary.com/browse/migrate

It's also not a Bitcoin-specific invention, but used in many projects (although probably not so often in end user communication). Could it be that those translators simply used some machine translation that was missing the context?

That being said, "upgrade wallet" would definitely be easi
...

💬 achow101 commented on pull request "Disable util::Result copying and assignment":
(https://github.com/bitcoin/bitcoin/pull/29906#issuecomment-2085790704)
ACK 6a8b2befeab25e4e92d8e947a23e78014695e06c

💬 achow101 commented on issue ""Migrate Wallet" is unclear to translators":
(https://github.com/bitcoin/bitcoin/issues/29979#issuecomment-2085813449)
Migrate is a term that is used in other computer related contexts, e.g. "database migration". It was chosen because of the process's similarities to a database migration, and particularly in a way similar to how Django [uses it](https://docs.djangoproject.com/en/5.0/topics/migrations/). Since Django has similar usage of the term with translations, perhaps we can point translators to there?

While "upgrade" was considered, we're already using that to mean something else, and I don't think overl
...

💬 theStack commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585154706)
Ugh indeed, apparently I've used a word for several years without noticing that it doesn't exist (mostly in "ephermal port") :D fixed.

💬 theStack commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585154931)
> [901ffa9](https://github.com/bitcoin/bitcoin/commit/901ffa9e6e57f7c0e0db967fba9c715139bbcf0b) shouldn't this be sent from the ephemeral miniwallet?

If the to-be-evicted tx and the filling txs originate from the same miniwallet instance, the problem fixed by e9dc511a7e9a562f953ff93f358102f555f583e6 is unfortunately re-introduced, as it could happen again that a filler tx spends from the to-be-evicted tx, increasing its' effective fee-rate and thus avoiding the expected eviction (making the a
...

🚀 achow101 merged a pull request: "Disable util::Result copying and assignment"
(https://github.com/bitcoin/bitcoin/pull/29906)

💬 laanwj commented on pull request "JSON-RPC request Content-Type is application/json":
(https://github.com/bitcoin/bitcoin/pull/29946#issuecomment-2085872620)
Code review ACK f90a84d61505443fd3bb83253c091590b3dc7f45

💬 ryanofsky commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-2085898761)
Thanks, I added a link directly to this comment in the summary so those who are interested can follow the chain of reasoning.

To reiterate my own point of view, `-logsourcelocation` does not solve the same problem this PR solves because it does not (1) let libitcoinkernel applications call validation functions and control which `Logger` object they write to, and (2) allow code like wallet code and index code to easily include additional metadata in log messages (wallet names and index names)
...

👍 MarnixCroes approved a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2031988964)
lgtm 83def1c5a3741878aa63e6f28cc3def99f76c358
useful clarification

💬 brunoerg commented on pull request "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`":
(https://github.com/bitcoin/bitcoin/pull/29939#discussion_r1585186363)
Not 100% related to these change, but`fill_mempool` requires -datacarriersize=100000 and -maxmempool=5 and in `rpc_packages` test, this is mentioned:
```py
# Make chain of two transactions where parent doesn't make minfee threshold
# but child is too high fee
# Lower mempool limit to make it easier to fill_mempool
self.restart_node(0, extra_args=[
"-datacarriersize=100000",
"-maxmempool=5",
"-persistmempool=0",
])
```

Perhaps, it would worth to also mention it in `mempoo
...

💬 sipa commented on pull request "msvc: Compile `test\fuzz\bitdeque.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29983#issuecomment-2085961219)
utACK 774359b4a96d2724dc70f900cb71e084a77164da

💬 sr-gi commented on pull request "fuzz: txorphan tests fixups":
(https://github.com/bitcoin/bitcoin/pull/29974#discussion_r1585200234)
Covered in [ec29dc4](https://github.com/bitcoin/bitcoin/pull/29974/commits/ec29dc4266e1fe2eb967a7ce164a59e6c162c592)

💬 theStack commented on pull request "Reintroduce external signer support for Windows":
(https://github.com/bitcoin/bitcoin/pull/29868#issuecomment-2086022405)
Concept ACK

💬 dongcarl commented on pull request "guix: fix suggested fake date for openssl-1.1.1l":
(https://github.com/bitcoin/bitcoin/pull/29999#issuecomment-2086032180)
If I remember correctly, I think that you can likely just patch OpenSSL here since it's not a package that is "core" to Guix like GnuTLS was?

👍 pinheadmz approved a pull request: "refactor: Avoid unused-variable warning in init.cpp"
(https://github.com/bitcoin/bitcoin/pull/29968#pullrequestreview-2032047471)
ACK fa2353d9c3ff68995e7478b9b80dcdb1fc638a53

Confirmed the warning on master by setting `-Wall` and then forcing `HAVE_SOCKADDR_UN = 0` in `configure.ac`. This patch fixes the warning in a clean way. I dunno if `(void)unix;` needs an explanatory comment or if thats a recognizable pattern for this kind of thing.

Thanks again @maflcko for cleaning up my mess 😬

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK fa2353d9c3ff68995e747
...

👍 BrandonOdiwuor approved a pull request: "gui: fix misleading signmessage error with segwit"
(https://github.com/bitcoin-core/gui/pull/819#pullrequestreview-2032085316)
Code Review ACK 83def1c5a3741878aa63e6f28cc3def99f76c358