💬 hebasto commented on pull request "Adjust plural forms for translations":
(https://github.com/bitcoin-core/gui/pull/716#issuecomment-1450430776)
cc @jarolrod @luke-jr

💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1121971951)
1d093bbf97b8d4ec5413ca560ac79e56f23d7f1d `{`

Could also use the `IsNull()` helper.

💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1121979628)
1d093bbf97b8d4ec5413ca560ac79e56f23d7f1d: maybe log the exact error code.

💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1122017022)
1224f45a92972199799d1ea33700eee567dc66fe: maybe call this file `bdb_ro_wallet_db.h`?

💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1121975697)
1d093bbf97b8d4ec5413ca560ac79e56f23d7f1d: Note for other reviewers: yes

https://en.cppreference.com/w/c/io/fseek

💬 Sjors commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1122022825)
d6c5aa0f6783ef9f26f93a583d2fad5a943e23ec: commit description also mentions `StartCursor`, `ReadAtCursor`, `CloseCursor`.

💬 achow101 commented on pull request "Switch hardened derivation marker to h (in normalized descriptors and new wallets)":
(https://github.com/bitcoin/bitcoin/pull/26076#discussion_r1122037209)
Could this default to `false` so that we prefer `h`'s everywhere? In my own testing, this only caused a few functional tests to fail, the unit tests seem fine.

💬 jonatack commented on pull request "rpc, test: remove newline escape sequence from wallet warning fields":
(https://github.com/bitcoin/bitcoin/pull/27138#issuecomment-1450468867)
@ryanofsky Adding newlines is a client-side concern. If it is important for the RPC client to know how many warnings have been returned in order to display them on separate lines, it seems best to replace the `warning` string field with a `warnings` JSON array of strings. That would be an orthogonal change, however: first the `warnings` field would be added and the `warning` field deprecated, then after a release the latter would be removed.

In the meantime, this is a simple and clear fix.

💬 ryanofsky commented on pull request "wallet: when a block is disconnected, update transactions that are no longer conflicted":
(https://github.com/bitcoin/bitcoin/pull/27145#issuecomment-1450530407)
Concept ACK on refreshing state of conflicted transactions in the wallet when a block is disconnected. I think there is a potential problem with the implementation though because it isn't recursively marking conflicted transactions as unconfirmed, only marking the top level transactions without recursing.

I think a good approach could be to steal from the previous implementation of this idea in commits b8b4592d985fa36dd0dddfd0e2bc6daf8df6e79c and 3c8bd6814ca88715d3d56994b10dc2c5f3e38e2d from
...

💬 TheCharlatan commented on pull request "guix: pass `--enable-initfini-array` to release GCC":
(https://github.com/bitcoin/bitcoin/pull/27153#issuecomment-1450562166)
I can confirm that this adds relevant `.fini_array`sections and gets rid of the `.ctors` and `.dtors` sections in the bitcoin x86 _64 linux binaries by comparing the binaries of this PR's `HEAD` and its `HEAD~1`.

`objdump -h bitcoind` on fc733e9 (HEAD) :
```
bitcoind: file format elf64-x86-64
Sections:
Idx Name Size VMA LMA File off Algn
0 .interp 0000001c 0000000000000350 0000000000000350 00000350 2**0
CONTENT
...

💬 TheCharlatan commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#issuecomment-1450575944)
ACK 802cc1ef536e11944608fe9ab782d3e962037703

💬 jonatack commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1450599651)
@pinheadmz Thanks for working on unit test coverage -- will circle back soon.

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122161205)
I ' ll actually remove the variable `executableCommand` since it's only used once, I prefer avoiding memory overhead and better speed execution (althoug minimal). Okey with the other 2 variables

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122164062)
Indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.

💬 hernanmarino commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1122165135)
Removed extra parenthesis, and I agree with extra long lines, but as I said before, indentation was introduced after running the script suggested by @ jarolrod and are apparently ok according to clang format standard.

💬 furszy commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122170162)
If the view is not accessible anymore, this changes aren't needed.

(same for the transaction table model ones)

💬 furszy commented on pull request "rpc: make importaddress compatible with descriptors wallet":
(https://github.com/bitcoin/bitcoin/pull/27034#discussion_r1122178962)
> > upgraded the legacy wallet into a descriptors wallet
>
> I think it's best to have only one way to go from a legacy wallet to a descriptor wallet (the migration RPC), so in the long run it's easier to keep testing this stuff.

yeah, agree.

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1450711073)
@willcl-ark thanks, I added comments and release notes.

I also wrote a tiny testing package using [libjson-rpc-cpp](https://github.com/cinemast/libjson-rpc-cpp) to check against an "outside" JSON-RPC 2.0 implementation. The package is https://github.com/pinheadmz/jsonrpc-bitcoin and [seems to like the 2.0 implementation so far.](https://github.com/pinheadmz/jsonrpc-bitcoin#examples)

💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#discussion_r1122210010)
yeah, I'll do.

💬 mzumsande commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#discussion_r1122020763)
if this is only necessary for windows, would it be possible to hide it behind `#ifdef WIN32` so that the unit tests don't run 3*2=6 seconds longer for everyone else (which is quite a lot)?