💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428105177)
would be nice to explain if the key a v3 tx or the ancestor set is, for name reading purposes

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428172941)
txns can not be empty

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428129715)
```suggestion
const int64_t vsize = GetVirtualTransactionSize(*tx, /*nSigOpCost=*/0, /*bytes_per_sigop=*/0);
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428162185)
Anything that's not-already `PreCheck`'ed will necessarily have empty `Workspace::m_ancestors` sets, so there will be a mismatch in the over-estimates.

(not sure it matters, just noting)

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428144170)
```suggestion
// Map from txid of a V3 transaction to its in-package ancestor set. Not MemPoolAccept-wide
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428181463)
```suggestion
unsigned int num_in_pkg_ancestors,
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428181640)
```suggestion
const CTxMemPool::setEntries& mempoool_ancestors,
```

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428198917)
this function explicitly doesn't check sigops adjusted vsize

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428212191)
I think sometimes this can over-shoot, meaning we might hide the actual check we're shooting for
```suggestion
```
we don't get a great error here either so it's hard to check precisely; should we pass the whole reason back in results?

📝 djschnei21 opened a pull request: "Update doc/policy/README.md"
(https://github.com/bitcoin/bitcoin/pull/29095)
Include `-datacarriersize` in the examples because people are extremely confused about this...

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428321924)
Updated the docstring

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428320528)
I've changed this to just be preceding transactions instead of all of them, which also matches with this behavior. Added a comment

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428319820)
Edited comment

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428322312)
Removed. Now using `package_state.ToString()` for "package_msg" result

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428320034)
paranoia deleted

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428319924)
taken

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1428327034)
Tbh since the limit is 1 ancestor, we could just make this a `std::map<Txid, Txid>` and save some space.

💬 murchandamus commented on issue "Flaky `wallet_transactiontime_rescan.py --legacy-wallet` functional test":
(https://github.com/bitcoin/bitcoin/issues/28221#issuecomment-1858309126)
The `wallet_transactiontime_rescan.py` test (and only it) frequently fails on my local machine due to timeout, whenever I run the tests with multiple threads, e.g.

`test/functional/test_runner.py -j "$(($(nproc)-1))"`

leads to

```
286/286 - wallet_transactiontime_rescan.py --legacy-wallet failed, Duration: 187 s

stdout:
2023-12-15T16:00:06.770000Z TestFramework (INFO): PRNG seed is: 5568797213703148493
2023-12-15T16:00:06.771000Z TestFramework (INFO): Initializing test directory
...

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1428378451)
too many zeroes on this, it's a 1/10 of a sat

💬 instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1428379004)
too many zeroes on this, it's a 1/10 of a sat