💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1449025828)
Disabling the Transaction tab when the user sets the mask value option:


Also, if you are already in the Transaction tab, switch/ change focus to the overview page as shown here:


💬 theStack commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1120968022)
```suggestion
// Parent + child with v3 in the mempool. Child is allowed as long as it is under V3_CHILD_MAX_WEIGHT.
```

💬 theStack commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1120967545)
```suggestion
* 2. The tx must be within V3_CHILD_MAX_WEIGHT
```

💬 theStack commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1120974200)
Seems like this is not used anywhere:
```suggestion
```

💬 theStack commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1120982825)
Missing assertions for the replaced-tx checks:
```suggestion
assert submitres2["replaced-transactions"] == [tx.rehash() for tx in package_txns1]
self.assert_mempool_contents(expected=package_txns2, unexpected=package_txns1)
submitres3 = node.submitpackage(package_hex3)
assert submitres3["replaced-transactions"] == [tx.rehash() for tx in package_txns2]
```

💬 theStack commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1120972484)
```suggestion
coin = self.coins.pop(0)
```

💬 theStack commented on pull request "policy: nVersion=3 and Package RBF":
(https://github.com/bitcoin/bitcoin/pull/25038#discussion_r1120985992)
```suggestion
// For example, the mempool contains a large, low-feerate transaction A (99,000vB, 1sat/vB feerate).
// Transaction A has a small, high-feerate child B (1,000vB, 101sat/vB). The user wants to
```

or maybe

```suggestion
// For example, the mempool contains a large, low-feerate transaction A (99,000vB, 1sat/vB feerate)
// which has a small, high-feerate child B (1,000vB, 101sat/vB). The user wants to
```

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