💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464367456)
good catch! done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464367790)
done. liked the simpler language.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464368898)
good find! replaced it with an `else` pathway in `data_received()`. this was kept to not perform parsing/deserialising of P2P messages inside `_on_data()` in case `v2_handshake()` wasn't over.

done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464371637)
added more comments. meant it as the [ASCII message type](https://github.com/bitcoin/bips/blob/master/bip-0324.mediawiki#v2-bitcoin-p2p-message-structure) like in v1.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464372562)
true! removed.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464372887)
nice! done.

💬 stratospher commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464375410)
right, not much of a difference - leaving it as a follow up if desired.

💬 maflcko commented on pull request "validation: improve checkblockindex comments":
(https://github.com/bitcoin/bitcoin/pull/29299#discussion_r1464467510)
```suggestion
// Transaction may be completely unset - happens if only the header was accepted but the block or it's ancestors haven't been downloaded.
```

nit: The block itself may have been processed (ProcessNewBlock is called). Maybe clarify that you meant that this block and all previous ones have been processed or downloaded to storage?

💬 maflcko commented on pull request "validation: improve checkblockindex comments":
(https://github.com/bitcoin/bitcoin/pull/29299#discussion_r1464471710)
```suggestion
// nChainTx may be unset, but nTx set (if a block has been accepted, but one of its predecessors hasn't been processed yet)
|| (pindex->nChainTx == 0 && pindex->nTx >=1 && prev_chain_tx == 0 && pindex->pprev)
```

nit: Missing nTx check, according to comment?

💬 maflcko commented on issue "Build error on macOS, tag v26.0 builds fine":
(https://github.com/bitcoin/bitcoin/issues/29289#issuecomment-1907608162)
> I think the conclusion is that I'm on clang 13 and need to upgrade. I'll close this issue (I don't have the will power to upgrade Xcode/clang on that machine right now and verify, but if it doesn't resolve the issue when I get to this I'll report back)

I think you are on clang 12 (for some reason apple calls clang 12 clang1300), according to https://en.wikipedia.org/wiki/Xcode#Xcode_11.0_-_14.x_%28since_SwiftUI_framework%29

But yeah, let us know if this is still an issue after upgrading
...

💬 naumenkogs commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464497339)
I think it deserves a comment :)

💬 maflcko commented on pull request "ci: Rename tasks (previous releases, macOS cross)":
(https://github.com/bitcoin/bitcoin/pull/29218#issuecomment-1907627949)
> > a new label can be added, if a dedicated "category" is needed
>
> I think this would be great. Ideally there would just be some link you could go to quickly see recent known CI failures and what the expected error messages look like.

Ok, I've recycled the "CI failed" label for this. See https://github.com/bitcoin/bitcoin/issues?q=is%3Aissue+is%3Aopen+label%3A%22CI+failed%22

There are 13 issues right now. (14 if someone creates an issue for the one mentioned in this thread)

If the
...

💬 naumenkogs commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#discussion_r1464515415)
The goal of this class is mostly to test the tests, right? Commenting that more clearly could help.

💬 S3RK commented on pull request "Add max_tx_weight to transaction funding options":
(https://github.com/bitcoin/bitcoin/pull/29264#discussion_r1464525381)
suggestion, add

```cpp
if (group.m_weight <= max_input_weight) continue;
```
on top of the for loop instead of checking within each condition.

Currently, you add groups that are above weight to `applicable_groups`

💬 naumenkogs commented on pull request "test/BIP324: functional tests for v2 P2P encryption":
(https://github.com/bitcoin/bitcoin/pull/24748#issuecomment-1907670816)
ACK fa4d9d5b39d5652de1d83564f19b1a0749863b16

There are many more tests possible, right? E.g. sending more than 4095 bytes of garbage.
I assume the goal here wasn't to cover everything?

✅ maflcko closed an issue: "Win64 CI failure in feature_versionbits_warning.py"
(https://github.com/bitcoin/bitcoin/issues/28115)

💬 maflcko commented on issue "Win64 CI failure in feature_versionbits_warning.py":
(https://github.com/bitcoin/bitcoin/issues/28115#issuecomment-1907723845)
Is this still an issue with a recent version of Bitcoin Core? Closing for now, but feel free to reopen if this happens again.

✅ maflcko closed an issue: "test: failure in feature_notifications.py (PermissionError: [WinError 32] The process cannot access the file because it is being used by another process:)"
(https://github.com/bitcoin/bitcoin/issues/27352)

💬 maflcko commented on issue "test: failure in feature_notifications.py (PermissionError: [WinError 32] The process cannot access the file because it is being used by another process:)":
(https://github.com/bitcoin/bitcoin/issues/27352#issuecomment-1907727054)
Is this still an issue with a recent version of Bitcoin Core? Closing for now, but feel free to reopen if this happens again.

💬 maflcko commented on issue "ci: feature_proxy failing in MSVC job":
(https://github.com/bitcoin/bitcoin/issues/29090#issuecomment-1907735929)
> Similar failure, different test: https://github.com/bitcoin/bitcoin/actions/runs/7249043694/job/19746211856#step:27:521.

Not sure. This one has

```
Error: 2023-12-18T14:15:31.391534Z [D:\a\bitcoin\bitcoin\src\logging.h:264] [error] ERROR: Commit: Failed to commit latest txindex state
```

in the log.