:lock: fanquake locked an issue: "Enable sanctions enforcement in Spam Filter Code"
(https://github.com/bitcoin/bitcoin/issues/29416)

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484797295)
> Would be good to explain that you are doing the mapWallet lookup because the wtx could have been updated.

I just realized that the wtx could not have been updated, because for it to be updated, `transactionAddedToMempool` would need to be called, which would be after this call, right? So I have reverted the change which compares witness hashes.

> Also, as you already know that the output belongs to one of the wallet's txs, could:

Yes, I have implemented this.

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1484800862)
> If we get the same txid but different wtxid, I'm not sure that the result is necessarily one that makes sense. Most things in the wallet are keyed and listed by txid, so with this change, we'd end up with a transaction that's listed as conflicted by it's own txid. Also, we won't ever update the wallet to store a transaction with a different wtxid.

Sorry I didn't see this earlier. After looking at this again I don't think that same-txid-different-witness will be a problem here. Please see: h
...

💬 sr-gi commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#issuecomment-1936624217)
ACK [13161ec](https://github.com/bitcoin/bitcoin/pull/27877/commits/13161ecf032b7a850686e5942c12222c8f3d0d52)

🚀 achow101 merged a pull request: "wallet: Add CoinGrinder coin selection algorithm"
(https://github.com/bitcoin/bitcoin/pull/27877)

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1936659009)
> looks like if I revert TX_MAX_STANDARD_VERSION to 3, all unit/functional tests pass?

I suppose my change at https://github.com/bitcoin/bitcoin/pull/28948/files#diff-cc0c6a9039a1c9fe38b8a21fe28391fffbac9b8531dfda0f658919a9f74b46baR793 is partially to blame. I made it `TX_MAX_STANDARD_VERSION + 1` instead of 3, thinking it might be easier to flip in the future.

I'm working on a followup to address the comments, and can incorporate if I need to retouch.

🤔 glozow reviewed a pull request: "p2p: Don't process mutated blocks"
(https://github.com/bitcoin/bitcoin/pull/29412#pullrequestreview-1873261879)
concept ACK, I agree with the approach of catching and dropping these earlier rather than later

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484847574)
Yes, seems this is because `SingleV3Checks` already covers it. I can add a comment and put the condition under an `Assume`?

💬 kevkevinpal commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1936684548)
> I'm seeing an issue during testing. The debug log incorrectly reports 0 MB. Are others able to reproduce my result?
>
> ```
> user1@comp1:~/Documents/GitHub/btc29402$ src/bitcoin-cli --rpcwait getmempoolinfo | jq -c '{ transactions: (.size), memory_usage_in_mb: (.usage/1000000) }' | jq; src/bitcoin-cli --rpcwait stop; sleep 10; grep -a Writing ~/.bitcoin/debug.log | tail -2
> {
> "transactions": 2529,
> "memory_usage_in_mb": 6.061648
> }
> Bitcoin Core stopping
> 2024-02-09T14:11
...

💬 epiccurious commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1936688373)
> it might make more sense to not have a MB amount on vinfo and print it out for the file size at the end of the creating the mempool file so users are not confused when they see xx amounts in MB and their file actually a different size

Approach ACK.

💬 kevkevinpal commented on pull request "mempool: Log added for dumping mempool transactions to disk":
(https://github.com/bitcoin/bitcoin/pull/29402#issuecomment-1936745081)
rebased and also updated code to get the size of the file after saving

logs now look like such and I tested to see that the `mempool.dat` file was the correct size by going to my data dir and doing `ls -alh` and they matched in sizes
```
2024-02-09T23:41:52Z DumpAnchors: Flush 2 outbound block-relay-only peer addresses to anchors.dat completed (0.02s)
2024-02-09T23:41:52Z scheduler thread exit
2024-02-09T23:41:52Z Writing 29 mempool transactions to disk...
2024-02-09T23:41:52Z Writing 0
...

💬 achow101 commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1936857327)
ACK 29029df5c700e6940c712028303761d91ae15847

🚀 achow101 merged a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948)

💬 murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1484980137)
Mh, it could also be turned on after max_weight was exceeded the first time. :thinking: Might follow-up on this.

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484992098)
nit: This is obvious I think, not really adding much, comment should be below explaining why we are assuming descendant count is 1 below
```suggestion
```

🤔 ismaelsadeeq reviewed a pull request: "v3 transaction policy for anti-pinning"
(https://github.com/bitcoin/bitcoin/pull/28948#pullrequestreview-1873486217)
Post merge ACK

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484992336)
nit: same here I think
```suggestion
```

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484994642)
The magic number can be declared with a descriptive variable name,
MAX_PUBKEYS_PER_MULTISIG = 20
bytespersigop = 5000

We can reuse them in the same function
```suggestion
expected_package_error = f"package-mempool-limits, package size {2*MAX_PUBKEYS_PER_MULTISIG*bytespersigop} exceeds ancestor size limit [limit: 101000]"
```

💬 ismaelsadeeq commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1484995474)
@instagibbs To test my understanding of the discussion here I created a test for this case https://github.com/ismaelsadeeq/bitcoin/commit/87fc7f0a8d8ad1fc2af60ee6cc678df5e6fb01dd

I agree we should not force siblings tx to conflict with reorged siblings, since we dont enforce v3 rules after reorg.

Maybe this should be documented in the comment.

💬 ismaelsadeeq commented on pull request "RPC: Add maxfeerate and maxburnamount args to submitpackage":
(https://github.com/bitcoin/bitcoin/pull/28950#issuecomment-1936910213)
Code review ACK 83f34b4783565336341948affa42e3ec71242c4b