💬 achow101 commented on pull request "guix: switch to some `minimal` versions of packages in our manifest":
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1450352576)
ACK 2c9eb4afe1f583aafa552b2711b149f17ef8320f

```
220003bd9c9cb840444494232b01b3d9e17ddda007abfd1b3a1001662b5f24c6 guix-build-2c9eb4afe1f5/output/aarch64-linux-gnu/SHA256SUMS.part
cc7f6e969a37d66164aad138635ea4ca1bb30eff2ed59a16c6b4af716824e4f1 guix-build-2c9eb4afe1f5/output/aarch64-linux-gnu/bitcoin-2c9eb4afe1f5-aarch64-linux-gnu-debug.tar.gz
6f1afad24db86220a29f8e3ee9170201b5ece045e00ce94d1000f0541a111a4d guix-build-2c9eb4afe1f5/output/aarch64-linux-gnu/bitcoin-2c9eb4afe1f5-aarch64-lin
...

💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1121931215)
Some suggestions after our chat:

* let's move this into (a) helper function(s):
* `FindGlobalHDKeyAmongstDescriptors()`
* `SetGlobalHDKey()`
* document this function, e.g. explain that it's trying to find the pattern of 3 or 4 descriptor pairs that use the same master key. Note how some might no longer be active because the user imported another descriptor and made it active. There might also be more than one set matching this pattern due to toggling encryption, so we pick the most rec
...

💬 TheCharlatan commented on pull request "guix: pass `--enable-initfini-array` to release GCC":
(https://github.com/bitcoin/bitcoin/pull/27153#issuecomment-1450406787)
Guix builds

```
b3e7fb01fb3cac7748f4d2800386dfcf65ac07b4b92e9be9f0169cc62db25798 guix-build-fc733e98e3f5/output/aarch64-linux-gnu/SHA256SUMS.part
c00eb6419247169400b8d91d7b6b514975ce3fc4104eba9f2432a628e5c46191 guix-build-fc733e98e3f5/output/aarch64-linux-gnu/bitcoin-fc733e98e3f5-aarch64-linux-gnu-debug.tar.gz
d2b279dcfe98ef2495164e369b4362e0719a2a87cd6d935ec364875884e60862 guix-build-fc733e98e3f5/output/aarch64-linux-gnu/bitcoin-fc733e98e3f5-aarch64-linux-gnu.tar.gz
b6bace7cb75d0f45c6
...

🚀 achow101 merged a pull request: "guix: switch to some `minimal` versions of packages in our manifest"
(https://github.com/bitcoin/bitcoin/pull/27172)

💬 pinheadmz commented on pull request "blockstorage: do not flush block to disk if it is already there":
(https://github.com/bitcoin/bitcoin/pull/27039#issuecomment-1450428114)
force-push to e600eb442802ca7b23189c8ceee448730e22e75a:
- rebased on master and fixed conflict
- added 2-second sleep before calls to `SaveBlockToDisk()`. This is to ensure that if the file is modified, its modification timestamp will definitely be updated. This test was intermittent on Windows I think because FAT has a 2-second resolution for `mtime` values. 🤷‍♂️

📝 hebasto opened a pull request: "Adjust plural forms for translations"
(https://github.com/bitcoin-core/gui/pull/716)
These plural forms were re-added in https://github.com/bitcoin-core/gui/pull/557.

The following `make -C src translate` also caught a new last-minute [added](https://github.com/bitcoin/bitcoin/pull/27157) string.

💬 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.