💬 dergoegge commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1448555096)
Concept ACK

💬 dergoegge commented on pull request "build: produce a .zip for macOS distribution":
(https://github.com/bitcoin/bitcoin/pull/27099#issuecomment-1448575726)
Concept ACK

💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1120508531)
it makes sense, I didn't notice it was discarding the previous one, gonna change it. However, I realized we can't always apply this coinbase filter because some tests need to use unconfirmed coinbase utxo (because it's the only available ones).

💬 achow101 commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#issuecomment-1448601870)
> Maintainers: what else would you like to see for a merge here?

Waiting for a couple more reviews.

📝 fanquake opened a pull request: "guix: use osslsigncode 2.5"
(https://github.com/bitcoin/bitcoin/pull/27179)
Switches to using a newer version of [osslsigncode](https://github.com/mtrojnar/osslsigncode) in our Guix environment.

@achow101 can you test this with some sort of WIndows code-signing dry-run (no-rush).

💬 pablomartin4btc commented on pull request "Mask values on Transactions View":
(https://github.com/bitcoin-core/gui/pull/708#issuecomment-1448682269)
> Thinking further about this, if the "privacy" option is enabled, the transactions screen has no usage, thus shouldn't be accessible for the user.

> We could either block the view when the privacy option is enabled, or could create an empty screen image to replace the transactions list when privacy is enabled.

Yeah, I realised what you are saying as soon as I fixed the negative amount issue looking at the type transactions, date-time, addresses columns, the red foreground colour due to th
...

💬 ryanofsky commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1120648056)
re: https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1120039071

Thanks, applied change

💬 ryanofsky commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1120648916)
re: https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1119622390

> nit: s/to whether/whether to/

Thanks, fixed comment

💬 ryanofsky commented on pull request "Deduplicate bitcoind and bitcoin-qt init code":
(https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1120647673)
re: https://github.com/bitcoin/bitcoin/pull/27150#discussion_r1120047778

Thanks, applied change

💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1120662435)
What's the purpose of this annotation `Any` here?

💬 pinheadmz commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#discussion_r1120688318)
Maybe I'm reading this wrong, but it looks like the `reversed()` on L288 can just be factored in to this first sort? That might make the actual filtering logic cleaner.

💬 brunoerg commented on pull request "test: fix intermittent issue in `feature_bip68_sequence`":
(https://github.com/bitcoin/bitcoin/pull/27177#issuecomment-1448877559)
> I assume you meant immature coinbases, rather than unconfirmed ones. Which tests currently rely on those? Can't really imagine a case where an unspendable UTXO would be of any use (only if mining of new blocks happen between fetching the UTXO and actually spending them, but then we can probably change those tests to fetch the UTXO later?).

One of the examples I found:

In `p2p_segwit.py`:
https://github.com/bitcoin/bitcoin/blob/cb40639bdf04bab31bcdceb3bf941e9bade8317a/test/functional/p2
...

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#issuecomment-1448970703)
Thanks for the reviews!

> Seeing all that effort to pass around the hacky fastprune arg (which doesn't make any sense outside of functional tests) seems a bit sad, so I wonder if it's really inevitable.
> Did you consider whether it's possible to also move BlockFileSeq into blockmanager? I guess it doesn't work just because of one remaining call site for node::ReadBlockFromDisk from zmqpublishnotifier?

Yes, totally echo this sentiment. Indeed BlockFileSeq and its dependants are not class
...

📝 brunoerg converted_to_draft a pull request: "test: fix intermittent issue in `feature_bip68_sequence`"
(https://github.com/bitcoin/bitcoin/pull/27177)
Fixes #27129

To avoid `bad-txns-premature-spend-of-coinbase` error,
`get_utxo` doesn't return by default immature coinbase.

💬 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)
```