👍 jarolrod approved a pull request: "Update translations for 25.0 soft translation string freeze"
(https://github.com/bitcoin/bitcoin/pull/27169)
ACK 9172cc672ea99eac6d0210e8b793ca030c20e179

Confirming that we are following the steps laid out in [release-process](https://github.com/bitcoin/bitcoin/blob/master/doc/release-process.md) for translations. After replicating the steps I have a zero-diff with this branch.

💬 jarolrod commented on pull request "guix: switch to some `minimal` versions of packages in our manifest":
(https://github.com/bitcoin/bitcoin/pull/27172#issuecomment-1449439152)
GUIX hashes:
```
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-linux-gnu.tar.gz
776ae93e37ddddb9c3
...

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1120543538)
```suggestion
// Copyright (c) 2023 The Bitcoin Core developers
```

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1120992192)
Not a bug, but I noticed that the only reference to `m_to_be_replaced` outside the constructor is an `Assume` in `SanityCheck()`, I wonder if it's worth `m_to_be_replaced` being a class member. If we give up the `Assume`, which doesn't seem to be doing that much heavy lifting, then `m_to_be_replaced` could be a local variable in the constructor.

Another nice thing about making `m_to_be_replaced` a local variable is that its entries can be pointers to transactions, instead of entire txid hashe
...

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1120948639)
Nit, more efficient to not make a copy
```suggestion
std::vector<COutPoint> outpoints_of_tx({outpoint});
m_requested_outpoints_by_txid.emplace(outpoint.hash, std::move(outpoints_of_tx));
```
or (I didn't test this, but I'm pretty sure that since the vector argument is an rvalue, the compiler will do a move)
```suggestion
m_requested_outpoints_by_txid.emplace(outpoint.hash, std::vector<COutPoint>{outpoint});
```

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1120964573)
Nit, simpler. I don't know if the comment is necessary (the reader is expected to know this about maps), but it may help to document that we're taking advantage of that behavior.

The developer notes do [say](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#c-data-structures) not to use the `std::map []` syntax, but only for reading.
```suggestion
// This creates the map entry if it doesn't already exist.
m_requested_outpoints_by_txid[outpoint.ha
...

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121191113)
Maybe this 500 can be a `constexpr` in the `MiniMiner` class? I probably wouldn't suggest this if this is the only place it occurs, but the tests specify this value too. It might be nice to be able to change this limit by changing a single line of code.

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121184614)
Can we remove this code? (I prefer avoiding special cases.) The only effect difference I can see by eliminating this is that, with it, `m_ready_to_calculate` will remain `true`, whereas without it, it will be `false`. Does that matter? I'm unsure of the purpose of `m_ready_to_calculate` (in this PR it's only used by test code, although I understand it may be used in the follow-on PR). Seems like once the constructor runs, it should be ready to calculate, no matter what. There's nothing else that
...

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121220463)
```suggestion
if (!m_to_be_replaced.count(txiter->GetTx().GetHash())) {
```

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121223393)
```suggestion
const bool remove_desc{to_be_replaced.count(&tx_desc) > 0};
```

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121215437)
This isn't a very efficient data structure, because each entry's key, the txid, is repeated in all of its vector's entries (because a `COutPoint` includes the txid). All you need is the index (`COutPoint::n`) So this could be:
```
std::map<uint256, std::vector<uint32_t>> m_requested_outpoints_by_txid;
```
although the name would be wrong, it's no longer "requested outpoints". Elsewhere in the code where you need a `COutPoint` (I think there are only two places), you can construct it from
...

💬 LarryRuane commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#discussion_r1121222211)
```suggestion
const bool remove{m_to_be_replaced.count(txid) > 0};
```

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1449443895)
Updated 88e88a5 -> f87c39399823e22c553b797cc66fa4063462a32b ([removeBlockstorageArgs_5](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_5) -> [removeBlockstorageArgs_6](https://github.com/TheCharlatan/bitcoin/tree/removeBlockstorageArgs_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/removeBlockstorageArgs_5..removeBlockstorageArgs_6)) with linter fix.

💬 wtogami commented on pull request "p2p, validation: Don't download witnesses for assumed-valid blocks when running in prune mode":
(https://github.com/bitcoin/bitcoin/pull/27050#issuecomment-1449474465)
Could we please pick a different name for old witnessless syncing than "pruned"? It's different from the old pruned node type that entirely discards old blocks. Old witnessless nodes could reindex and rescan as well as provide old witnessless blocks to other nodes doing IBD of this type. We need a new name for this.

👍 w0xlt approved a pull request: "doc: Expand scantxoutset help text to cover tr() and miniscript"
(https://github.com/bitcoin/bitcoin/pull/27155)
ACK https://github.com/bitcoin/bitcoin/pull/27155/commits/ca605f015dc8fbabbc6c0640d87429d6bf8f553f

💬 w0xlt commented on pull request "doc: Expand scantxoutset help text to cover tr() and miniscript":
(https://github.com/bitcoin/bitcoin/pull/27155#discussion_r1121339332)
Could also include `rawtr(<pubkey>) P2TR output with the specified key as output key`.

📝 Sjors opened a pull request: "26032 followups"
(https://github.com/bitcoin/bitcoin/pull/27180)
Followups for #26032. So far nothing major.

💬 Sjors commented on pull request "wallet: skip R-value signature grinding for external signers":
(https://github.com/bitcoin/bitcoin/pull/26032#discussion_r1121378441)
Added to a followup PR #27180.

💬 Sjors commented on pull request "contrib: Improve verify-commits.py to work with maintainers leaving":
(https://github.com/bitcoin/bitcoin/pull/27058#issuecomment-1449640124)
I think it's fine to have a temporary hoop here, as long as it's documented.

💬 hebasto commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1449655488)
Well, speaking of the removed questionable [commit](https://github.com/bitcoin/bitcoin/commit/a25daf76b5c6659e9002d6ae08a7907933fc120b), I've finally managed to provide steps to reproduce unexpected behavior _without_ that commit:
1. Complete all tasks.
2. Re-run, for example, the "32-bit + dash [gui] [CentOS 8]" task. As expected, ccache achieves 100% hit.
3. Add a commit which does _not_ change any code and run CI job. It is natural to expect that ccache will 100% hit this time as well. But
...