💬 vasild commented on pull request "Handle CJDNS from LookupSubNet()":
(https://github.com/bitcoin/bitcoin/pull/27071#issuecomment-1678816243)
@mzumsande, what's your take on this now that it does not touch the GUI?

💬 fanquake commented on pull request "[WIP] guix: use `-muse-unaligned-vector-move` for Windows builds":
(https://github.com/bitcoin/bitcoin/pull/28151#issuecomment-1678870156)
> Edit: Ok, right, this is avx-only. Ignore most of the above.

Yea. It seems hard to (generally) produce working Windows binaries outside of Guix, because I assume most Windows toolchains are broken with the same bug (unless they are all being patched similar to Debian/Ubuntu).

So the best we can do is retain our GCC patching, so the entire toolchain/libs/bins are built correctly, and then in configure, we could pass the assembler flags as a best-effort, to _try_ and produce working binari
...

⚠️ Crypt-iQ opened an issue: "compact block fingerprinting"
(https://github.com/bitcoin/bitcoin/issues/28272)
I haven't written a test for this, but it seems that a peer can fingerprint a chunk of our mempool by announcing a new compact block with a valid header, but inserting many `shorttxids` that don't belong to the block but are suspected to be in our mempool. Assuming no short id collisions, we'll then request each tx that we didn't find in either `vExtraTxnForCompact` or the mempool.

I'm not sure how to fix this since compact block relay relies on this behavior. One possible way of fixing this
...

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1294434738)
Forgot `CI_FAILFAST_TEST_LEAVE_DANGLING` ? Otherwise, it would be good to see a functional test failure log

💬 MarcoFalke commented on pull request "ci: Run "macOS native x86_64" job on GitHub Actions":
(https://github.com/bitcoin/bitcoin/pull/28187#discussion_r1294603992)
Also, `CCACHE_NOHASHDIR`? Or maybe that'd be better moved to `./ci/test/00_setup_env.sh`

💬 achow101 commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1294605712)
Requesting the already confirmed transaction is surprising to me. Is this expected to change in future work (if so a comment mentioning that would be useful) or is it intended behavior?

🤔 achow101 reviewed a pull request: "test: tx orphan handling"
(https://github.com/bitcoin/bitcoin/pull/28199#pullrequestreview-1578574354)
ACK 9eac5a0529f869075f0331e40d322c34fc8fc2af

💬 achow101 commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1294606522)
Could check that `parent_low_fee_nonsegwit` is not in the mempool as done in the other test cases.

💬 MarcoFalke commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-1678954526)
I guess this relates to the TODO as well: https://github.com/bitcoin/bitcoin/blob/80d70cb6b04b5a3c13e661c0718a4b5108d55869/src/blockencodings.cpp#L24

💬 furszy commented on pull request "test: display abrupt shutdown errors in console output":
(https://github.com/bitcoin/bitcoin/pull/28253#discussion_r1294613398)
The idea was to separate the stderr output from the exception being thrown here. Mainly to get a more readable print when a crash happens. The `FailedToStartError` exception message appears in-between two long stack traces.

But, either way is ok for me.

💬 glozow commented on pull request "test: tx orphan handling":
(https://github.com/bitcoin/bitcoin/pull/28199#discussion_r1294614675)
> Requesting the already confirmed transaction is surprising to me.

Yep, was strange to me too, but `AlreadyHaveTx` is peerman's best metric for determining what the missing parents are (validation doesn't say which one(s) are missing). Also afaiu it's considered uncommon to (1) have an orphan (2) have a transaction that spends a very old coin, so I'm guessing this is quite rare.

> Is this expected to change in future work

I'm not aware of any plans to change the legacy way of requestin
...

💬 MarcoFalke commented on pull request "test: display abrupt shutdown errors in console output":
(https://github.com/bitcoin/bitcoin/pull/28253#discussion_r1294617879)
If you want to keep it, it would also be good to show an example output (CI and non-CI) when this is printed in a "normal" test (not create_cache)

💬 Crypt-iQ commented on issue "compact block fingerprinting":
(https://github.com/bitcoin/bitcoin/issues/28272#issuecomment-1678970122)
I think implementing the TODO might add round-trips and maybe leak what we had in our own mempool prior to block acceptance?

💬 furszy commented on pull request "test: display abrupt shutdown errors in console output":
(https://github.com/bitcoin/bitcoin/pull/28253#discussion_r1294638606)
Why might the output differ between CI and non-CI runs? The test framework redirect stderr to a file, and this file remains untouched regardless of the execution environment.

Also, the `"normal" test (not create_cache)` tense isn't clear for me. Does it refer to running an individual test locally?

💬 fjahr commented on pull request "rpc: Add dumpcoinstats":
(https://github.com/bitcoin/bitcoin/pull/19792#issuecomment-1678988325)
There doesn't seem to be much interest in this feature currently. If someone needs it, feel free to ping me here and I will reorg/rebase for you.

✅ fjahr closed a pull request: "rpc: Add dumpcoinstats"
(https://github.com/bitcoin/bitcoin/pull/19792)

💬 achow101 commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#issuecomment-1678988739)
ACK fa776e61cd64a5ffd9a4be589ab8efeb5421861a

💬 MarcoFalke commented on pull request "test: display abrupt shutdown errors in console output":
(https://github.com/bitcoin/bitcoin/pull/28253#discussion_r1294654358)
> Why might the output differ between CI and non-CI runs?

I thought that the test_runner.py may be doing something with stderr, but it looks like it will get printed:

```py
print(BOLD[1] + 'stdout:\n' + BOLD[0] + stdout + '\n')
print(BOLD[1] + 'stderr:\n' + BOLD[0] + stderr + '\n')
if combined_logs_len and os.path.isdir(testdir):
# Print the final `combinedlogslen` lines of the combined logs
```



> Also, the `"n
...

🚀 achow101 merged a pull request: "rpc: Add importmempool RPC"
(https://github.com/bitcoin/bitcoin/pull/27460)

💬 furszy commented on pull request "test: display abrupt shutdown errors in console output":
(https://github.com/bitcoin/bitcoin/pull/28253#discussion_r1294678025)
> > Why might the output differ between CI and non-CI runs?
>
> I thought that the test_runner.py may be doing something with stderr, but it looks like it will get printed:
>
> ```python
> print(BOLD[1] + 'stdout:\n' + BOLD[0] + stdout + '\n')
> print(BOLD[1] + 'stderr:\n' + BOLD[0] + stderr + '\n')
> if combined_logs_len and os.path.isdir(testdir):
> # Print the final `combinedlogslen` lines of the combined logs
> ``
...