💬 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
> ``
...

💬 MarcoFalke commented on pull request "test: display abrupt shutdown errors in console output":
(https://github.com/bitcoin/bitcoin/pull/28253#discussion_r1294687210)
> In a first glance, that doesn't seems to print the nodes' stderr. That seems to be related to the job stderr (the test run in a subprocess). And those aren't connected.

Currently, you are reading the node's stderr and the printing it to stdout, so it *should* appear as `print(BOLD[1] + 'stdout:\n' + BOLD[0] + stdout + '\n')`.

> Ok. The result is pretty much the same. Just the stack trace is a bit longer.

The difference should be that the output won't be shown in the CI tail if the tes
...

👍 brunoerg approved a pull request: "net: Continuous ASMap health check"
(https://github.com/bitcoin/bitcoin/pull/27581#pullrequestreview-1578712710)
crACK b87045da22adec42cba28d1c9e9b910ab6cf5380

📝 MarcoFalke opened a pull request: "ci: Fix corrupt macOS-cross depends cache"
(https://github.com/bitcoin/bitcoin/pull/28273)
For some reason the macOS-cross depends cache is corrupt and leads to errors on all builds (master and pull requests): https://cirrus-ci.com/task/4858643863044096?logs=ci#L1291

Fix this by creating a new cache via a new fingerprint via the task name. See https://github.com/bitcoin/bitcoin/blob/cd43a8444ba44f86ddbb313a03a2782482beda89/.cirrus.yml#L69

💬 fjahr commented on pull request "net: Continuous ASMap health check":
(https://github.com/bitcoin/bitcoin/pull/27581#discussion_r1294802076)
Thanks, I will change this when I have to retouch.