💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#issuecomment-1956840825)
@epiccurious moved aborting the tests before the Unit Tests for Test Framework Modules.

💬 luke-jr commented on pull request "net: call `Select` with reachable networks in `ThreadOpenConnections`":
(https://github.com/bitcoin/bitcoin/pull/29436#discussion_r1497744510)
What is this supposed to do? :/

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1497755511)
```suggestion
if testdir_disk_usage_stats.free < MIN_FREE_SPACE:
```

nit: no need for `()` in python

💬 maflcko commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1956886011)
> @maflcko could you elaborate more on this

Sure, what is the question?

💬 hebasto commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1956889076)
> > In the libsecp repo, the `--with-asm=no` option is still required to avoid MSan warnings.
>
> Can you point to where this is an issue? It looks as though secp has proper `__msan_*` annotations in place these days.

Sure. For example, https://github.com/bitcoin-core/secp256k1/pull/1169#issuecomment-1370003449. It holds for the current libsecp master branch @ https://github.com/bitcoin-core/secp256k1/commit/0653a25d50f67c68bd2d196ecc7eddab067d95ef as well.

💬 BrandonOdiwuor commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1956893424)
was asking what you meant by this?

> Could also update the documentation of the setting to clarify that it refers to the relay of transactions which create bare outputs, and not to the relay of transactions spending them?

https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1895628355

💬 maflcko commented on pull request "test: fix RPC coverage check":
(https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1956896591)
Can you test what happens when the wallet is completely disabled in configure?

📝 ryanofsky opened a pull request: "ci: remove unnecessary git diff after applying patch"
(https://github.com/bitcoin/bitcoin/pull/29461)
Drop `git diff` command so it is easier to run CI locally if git checkout is a worktree. Currently this fails because the directory is not recognized as a git repository.

The `git diff` command was added recently in #28359 commit fa07ac48d804beac38a98d23be2167f78cadefae and isn't necessary. It is also probably not useful for debugging.

💬 maflcko commented on pull request "test: Add test case for spending bare multisig":
(https://github.com/bitcoin/bitcoin/pull/29120#issuecomment-1956903325)
I mean that the documentation in `init.cpp` can be clarified that it refers to outputs, not inputs.

💬 BrandonOdiwuor commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1497766575)
fixed

💬 starius commented on pull request "Extend signetchallenge to set target block spacing":
(https://github.com/bitcoin/bitcoin/pull/29365#discussion_r1497768481)
@Eunovo That's a good idea, thanks! I added the text of my comment above to the code and pushed in a separate temporal commit to track changes.

💬 maflcko commented on pull request "ci: remove unnecessary git diff after applying patch":
(https://github.com/bitcoin/bitcoin/pull/29461#issuecomment-1956954836)
It is there to clarify the CI is using different code than the one in the source dir.

lgtm ACK c835176774a4803132343d5e096cdc7fe902e8eb

👍 maflcko approved a pull request: "test: Handle functional test disk-full error"
(https://github.com/bitcoin/bitcoin/pull/29335#pullrequestreview-1893536162)
lgtm

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1497794666)
```suggestion
# Exit the test if there is not enough space on the testing dir

if shutil.disk_usage(tmpdir).free < MIN_FREE_SPACE:
```

💬 maflcko commented on pull request "test: Handle functional test disk-full error":
(https://github.com/bitcoin/bitcoin/pull/29335#discussion_r1497794980)
nit: can just inline this?

👍 theStack approved a pull request: "test: assert rpc error for addnode v2transport not enabled"
(https://github.com/bitcoin/bitcoin/pull/29460#pullrequestreview-1893543293)
Code-review ACK 345169a7523f00209985da88e0171e8687589c25

🚀 fanquake merged a pull request: "test: assert rpc error for addnode v2transport not enabled"
(https://github.com/bitcoin/bitcoin/pull/29460)

💬 fanquake commented on pull request "RFC: build: remove confusing and inconsistent disable-asm option":
(https://github.com/bitcoin/bitcoin/pull/29407#issuecomment-1956977378)
> @fanquake Do you think it's too late for this for v27?

I think it's still possible to do this for v27. It's not clear to me that the other change is strictly a requirement for doing this change?, but I also think that could go in.

💬 luke-jr commented on pull request "RPC/Wallet: Add "use_txids" to output of getaddressinfo":
(https://github.com/bitcoin/bitcoin/pull/22693#issuecomment-1956989002)
Rebased again

💬 glozow commented on pull request "policy: enable sibling eviction for v3 transactions":
(https://github.com/bitcoin/bitcoin/pull/29306#discussion_r1497818462)
You're right, it's not necessary. I think all we need is to gate on `ATMPArgs::m_allow_replacement` to not hit it in a multi-testmempoolaccept.