💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1337024969)
Question: What's the expected state of a transaction if its parent is replaced? My (shallow) understanding suggests it becomes inactive? And if the parent is re-added to mempool, it stays inactive?

🤔 glozow reviewed a pull request: "wallet: track mempool conflicts with wallet transactions"
(https://github.com/bitcoin/bitcoin/pull/27307#pullrequestreview-1643959917)
Approach ACK

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471392482)
in a6ae5b23b1497ab6f4899db49348db623700a2d8

same, `auto` or `Txid` would be better

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471531989)
in 067189eddee1a2209c6c42741a91601c5514e170

Got a little nervous seeing the txid comparison and "same tx." Is it possible to run into a same-txid-different-witness situation here?

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471533005)
c71736d372c65b0978671501029e202574c6bbef

Also good to document - is it just directly conflicting, or descendants as well?

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471530256)
fwiw, it seems better to keep it the way it was if it's not doing anything. What was the bug?

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471567921)
71059cf4267d80d2934b85a7046fc7b64900378b

There's not anything preventing us from checking the exact contents of `listunspent` right? Here and in the rest of the test.

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471581755)
Not sure if in scope for this PR, but it would be nice to have a test for how the wallet sees descendants of conflicting transactions (e.g. if Bob creates a child off of tx1 or tx2).

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471391179)
in a6ae5b23b1497ab6f4899db49348db623700a2d8:

It's best to use `Txid` or `Wtxid` for this. And maybe add a doc stating whether these are direct conflicts only, or if they might be descendants of a conflicting transaction?

💬 glozow commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471575174)
71059cf4267d80d2934b85a7046fc7b64900378b

We don't expect to see tx1 in mempool, right? It's not being rebroadcast, as it's inactive?

```suggestion
assert tx1_txid not in self.nodes[0].getrawmempool()
```
Might be good to have a check for this behavior.

👍 pablomartin4btc approved a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347#pullrequestreview-1851832717)
re ACK 0bef1042ce6c459acb1de965cbccd98867a417f1

💬 usuarrio-id81214293 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471596662)
> Fixed

Need enter wallet

💬 ajtowns commented on pull request "RFC: Deprecate libconsensus":
(https://github.com/bitcoin/bitcoin/pull/29189#issuecomment-1917479122)
> Rather than spending time arguing about removing some silly tests, I think there's a compromise here: Let's do the deprecation now (the readme changes as well as a release note as suggested by @achow101 above) and plan to just do the lib removal first thing after branch-off. That way we have our assurances that we can simply skip the port on the CMake side so it won't slow anything down there.
>
> Sound good?

Yeah, that's exactly what I was expecting fwiw.

💬 hebasto commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1917493926)
The same error when using Ubuntu 24.04:
```
$ x86_64-w64-mingw32-g++-posix --version
x86_64-w64-mingw32-g++-posix (GCC) 13-posix
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

```

💬 hernanmarino commented on pull request "assumeutxo, rpc: Add 'start' parameter to loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28659#issuecomment-1917507004)
> Are you still working on this?

Yes ! I was witing for more reviews, but I'll go ahead with @Sjors suggestion asap.

📝 maflcko opened a pull request: " refactor: Remove nChainTx from consensus "
(https://github.com/bitcoin/bitcoin/pull/29349)
`nChainTx` is returned by RPCs and is used to estimate (sync and rescan) progress.

However, it is also used for consensus and P2P logic to denote that a block and all its parents have `BLOCK_VALID_TRANSACTIONS` set.

This is confusing, because:

* It would be clearer to use the `BlockStatus` enum for this as well.
* It requires AssumeUtxo to fake the `nChainTx` values.
* It makes it harder to remove `nChainTx` completely.

Fix it by introducing a new `BLOCK_VALID_TRANSACTIONS_TREE` le
...

💬 maflcko commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1917512287)
Ok, as a next step it could make sense to create a standalone reproducer, so that the bug can be reported upstream.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1471649978)
With this PR, if a transaction's parent is replaced, then it should become `TxStateMempoolConflicted`. If the conflict is removed, then it becomes inactive.

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1471652116)
Ah, didn't realize we had that. Done

💬 achow101 commented on pull request "consensus: Store transaction nVersion as uint32_t":
(https://github.com/bitcoin/bitcoin/pull/29325#discussion_r1471652269)
Done