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

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423981481)
Added mention of no topological restrictions to the comment

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

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423982889)
Added a check for in-package + mempool ancestors, and both of your tests.

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

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#discussion_r1423983892)
I've changed the `RemovalReasonToString` to be "sizelimit or <=0 fee". Alternatively, we can add another reason?

💬 fanquake commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1852024695)
> C++20 fixed that, see https://en.cppreference.com/w/cpp/utility/source_location

Hopefully we'll be able to use this at some point, however Apple Clang does not support this at all yet, and it looks like it will also come with a LLVM 15, if not 16+ requirement as well.

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1852028059)
> As a side effect, I think this will allow users to remove select entries from their non-full mempool by prioritizing to large negative values (so basically a poor man's https://github.com/bitcoin/bitcoin/pull/15873).

This was discussed as part of #27018, also see the discussion on irc that day: https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2023-02-01. I think the general idea is "this is good, and will help avoid unspent ephemeral anchors."

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

💬 glozow commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1852030780)
Last push fixed issues and addressed most comments, I'm also working on adding more tests.

💬 maflcko commented on pull request "build: Enable -Wunreachable-code":
(https://github.com/bitcoin/bitcoin/pull/28999#issuecomment-1852031885)
Jup, it is GCC libstdc++11, or Clang libc++16, or later

💬 murchandamus commented on pull request "wallet: skip BnB when SFFO is enabled":
(https://github.com/bitcoin/bitcoin/pull/28994#issuecomment-1852040443)
Yes, ACK https://github.com/bitcoin/bitcoin/commit/576bee88fd36e207b7288077626947a1fce0fc33

🚀 fanquake merged a pull request: "Revert "ci: Only run functional tests on windows in master""
(https://github.com/bitcoin/bitcoin/pull/29059)

✅ maflcko closed an issue: "Software wont launch"
(https://github.com/bitcoin/bitcoin/issues/29033)

💬 maflcko commented on issue "Software wont launch":
(https://github.com/bitcoin/bitcoin/issues/29033#issuecomment-1852075275)
Closing for now, due to lack of more details. Once you find the debug log with more details, feel free to reply below.

💬 maflcko commented on issue "bumpfee doc about incrementalfee is incorrect":
(https://github.com/bitcoin/bitcoin/issues/29053#issuecomment-1852076174)
Pull requests with improvements are welcome :)

💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1424022422)
I was talking about what we do before the test. The test cleanup after execution is fine for the default case.

I think we shouldn't block the test creation on different paths. Be forced to place the test only inside the temp directory doesn't seems so useful to me.
What if we make the `-testdatadir` arg the base path for all test cases, and create a subdirectory for each run test inside it?. (with this, you would not need to delete the datadir at the beginning, because it will not exist).
E
...

💬 furszy commented on pull request "test: test_bitcoin: allow -testdatadir=<datadir>":
(https://github.com/bitcoin/bitcoin/pull/26564#discussion_r1424023270)
this should use std::cerr

💬 fanquake commented on pull request "Replace non-standard CLZ builtins with c++20's bit_width":
(https://github.com/bitcoin/bitcoin/pull/29057#issuecomment-1852088669)
Concept ACK. We can also check the benchmarks in https://corecheck.dev/bitcoin/bitcoin/pulls/29057 when they become available.

> I can either add a commit on top of this PR to do the switch after the c-i has run or do it as a follow-up PR, I have no preference.

I think the changes here should be straightforward enough, that you could push another commit doing the cleanup.

💬 fanquake commented on pull request "depends: fix libmultiprocess build on aarch64":
(https://github.com/bitcoin/bitcoin/pull/28846#discussion_r1424034141)
Have dropped this change.