💬 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

💬 LarryRuane commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1428387286)
I hadn't seen those PRs, thanks. I tried running `test_bitcoin` with the `TMPDIR` environment variable set (on Linux), and it works, so that means you don't have to set up a symlink.

> or.. don't delete anything and move such responsibility to the user.

Yes, I like that suggestion. I had forgotten that vscode (which is where I run the debugger from) allows one to set up an arbitrary "task" to run, `preLaunchTask`, before starting the executable, and this can remove the (fixed-named) data d
...

💬 pablomartin4btc commented on issue "The `streams_tests/xor_file` test fails on Windows":
(https://github.com/bitcoin/bitcoin/issues/29014#issuecomment-1858387980)
I've also tested the fix above provided by @hebasto and it works as expected, thanks!

Tested it running all tests successfully on Windows 11 Pro after a cross-build on Ubuntu 22.04 with the fix above.

```
.\test_bitcoin.exe --show_progress=true
Running 588 test cases...

0% 10 20 30 40 50 60 70 80 90 100%
|----|----|----|----|----|----|----|----|----|----|
***************************************************

*** No errors detected
```

💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1428446668)
Sounds good to me.
Probably the only reason we may want to abort running a test on a specific datadir is when the directory contain mainnet data. But it shouldn't be a problem as we should be having something to detect this scenario inside `init.cpp` already.

💬 instagibbs commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1858463698)
Here's a small patch to add sigops-adjusted coverage for v3 children, with an intentional bug added to ensure it could be hit.

https://gist.github.com/instagibbs/c5cb0796ceec81f0374ae614f8cdab7f