💬 murchandamus commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384005669)
Perhaps:
```suggestion
const bool valid_or_reconsiderable{atmp_result.m_result_type == MempoolAcceptResult::ResultType::VALID ||
```

💬 brunoerg commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1384023104)
In fde0193e687ad50a01a191e14fb6c052b3534bc1: Is there any case of `state` not being `None` for any `load_file` usage?

💬 ismaelsadeeq commented on pull request "validation: return more helpful results for reconsiderable fee failures and skipped transactions":
(https://github.com/bitcoin/bitcoin/pull/28785#discussion_r1384025601)
PR description also needs to be updated to `TX_RECONSIDERABLE`

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1383974021)
In 003efbbe45079c4416810a025b2bc372559dff15 "test: Add tests for wallet mempool conflicts"

nit: When doing separate test cases like this, we should also use different wallets to prevent state from one test cases from effecting another. So this should be making its own wallets once again.

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1384017154)
In 28a7e883c2f9249213fddbe24948ec5bb90b0fad "wallet: track mempool conflicts"

It's not clear to me that this change has any effect.

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1384032166)
In 003efbbe45079c4416810a025b2bc372559dff15 "test: Add tests for wallet mempool conflicts"

It would be useful to have a comment that explains that 11 blocks are mined so that when they are invalidated, `tx2_conflict` does not get put back into the mempool.

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1383975379)
In 003efbbe45079c4416810a025b2bc372559dff15 "test: Add tests for wallet mempool conflicts"

nit: No need to make another wallet for the receiver, just use the default wallet.

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1384035046)
In 003efbbe45079c4416810a025b2bc372559dff15 "test: Add tests for wallet mempool conflicts"

This isn't actually necessary since `listunspent` has these fields already and the extra ones are just ignored.

💬 achow101 commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1384024252)
In 003efbbe45079c4416810a025b2bc372559dff15 "test: Add tests for wallet mempool conflicts"

This can be compressed into one line using the `send` RPC.

```suggestion
raw_tx1 = alice.send(outputs=[{bob.getnewaddress(): 24.9999}], inputs=[unspents[0]], add_to_wallet=False)["hex"]
```

💬 achow101 commented on pull request "Test: followups to #27823":
(https://github.com/bitcoin/bitcoin/pull/28612#issuecomment-1796622219)
ACK 5ab6419f380cc0a8cde78b125f3eeee5fcba43ae

💬 achow101 commented on pull request "mempool: Persist with XOR":
(https://github.com/bitcoin/bitcoin/pull/28207#discussion_r1384073170)
In eeee4a5779ea20d859cff2e411ad46dd52384f1d "mempool: persist with XOR"

Closing and reopening the file once we have the xor seems kinda clunky. Why not have a function on `CAutoFile` that lets us set the xor key after the fact?

✅ achow101 closed an issue: "Extend feature_init.py file perturbations"
(https://github.com/bitcoin/bitcoin/issues/28603)

🚀 achow101 merged a pull request: "Test: followups to #27823"
(https://github.com/bitcoin/bitcoin/pull/28612)

📝 mzumsande opened a pull request: "test: Make existing functional tests compatible with --v2transport"
(https://github.com/bitcoin/bitcoin/pull/28805)
This makes the functional test suite compatible with BIP324, so that
`python3 test_runner.py --v2transport`
should succeed (currently, 12 tests fail for me on master).
Includes two commits by TheStack I found in an old discussion https://github.com/bitcoin/bitcoin/pull/28331#discussion_r1326714164

Note that even thought all tests should pass, the python `p2p.py` module will only do v2 connections only after the merge of #24748.
Some of the fixed tests were added with `--v2transport` to t
...

💬 mzumsande commented on pull request "test: Make existing functional tests compatible with --v2transport":
(https://github.com/bitcoin/bitcoin/pull/28805#issuecomment-1796828203)
FYI @stratospher @TheStack

💬 fanquake commented on pull request "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`":
(https://github.com/bitcoin/bitcoin/pull/28791#issuecomment-1796851218)
cc @pablomartin4btc

💬 TheBlueMatt commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1796963490)
Nothing has ever been imported into this wallet, its pretty much bog-standard, except that transactions are built with the raw transaction interface (eg the linked transaction, which spends my money back to myself and adds another input, in this case a lightning anchor). Nothing touching this wallet uses `lockunspent`.

💬 TheBlueMatt commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1796972545)
There doesn't appear to be anything in-wallet that is spending that output:

`bitcoin-cli listtransactions "*" 10000000 | jq '.[] | select(.confirmations <= 0) | .txid' | xargs -L1 bitcoin-cli gettransaction | grep bc1qag6gtevhvuhafph4d73ajlj5vyhh7k4xff0p4f` shows nothing.

💬 ajtowns commented on pull request "ArgsManager: support subcommand-specific options":
(https://github.com/bitcoin/bitcoin/pull/28802#discussion_r1384153284)
Renamed to `COMMAND_OPTIONS` and changed "subcommand" to "command" more generally.

💬 TheBlueMatt commented on issue "Wallet Missing Balances/Unspent":
(https://github.com/bitcoin/bitcoin/issues/28797#issuecomment-1797031592)
Ah, okay, so what happened is two bugs and one need-package-relay issue:

The above snippet didn't work because of the bug you can see in the above `listtransactions` output: if a transaction spends from an address of ours to an address of ours its (correctly) listed twice, but both times with the output address, neither time with the input address, even though one listing is "send". That's a bug.

This did turn up a spending tx, but that tx was in the broader mempool because its parent didn
...