👍 stickies-v approved a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1545751164)
ACK d87edf309ffe60ae044a57ce9a0d9cc711edc365

Removes dependency on UniValue in kernel, and imo improves the interface of dealing with sighashtype user input along the way.

No strong view on keeping/reverting the separate `script/sighashtype.h` header, happy to go either way. Unnecessary code churn is always better to be avoided, but I'd also rather not bike-shed over it. Will quickly re-ACK if consensus is to revert it, though.

💬 stickies-v commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1273714341)
nit: `signrawtransactionwithwallet` and `walletprocesspsbt` also affected

💬 stickies-v commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1273709151)
nit: missing `#include <script/sighashtype.h>`?

💬 MarcoFalke commented on issue "error: in "validationinterface_tests/unregister_all_during_call": check destroyed has failed":
(https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1650064645)
> UpdatedBlockTip runs in the scheduler thread

Thanks. That sounds like a bug. The unit test should be single threaded, or alternatively drop all async notifications before starting.

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1273736467)
Ugh, how did I miss those, will re-push.

📝 MarcoFalke opened a pull request: "test: Avoid intermittent issues due to async events in validationinterface_tests"
(https://github.com/bitcoin/bitcoin/pull/28150)
Currently the tests have many issues:

* They setup the genesis block, even though it is not needed
* They queue an async `UpdatedBlockTip` even, which causes intermittent issues: https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1650064645

Fix all issues by trimming down the setup to just `ChainTestingSetup`.

💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1273739178)
> I wonder what this failure looks like in practice, though. Could we write a test for it?

I could not come up with a way to test this with our existing code. If anybody has any pointers for how to achieve this, I'd happy to give it a shot at implementing the test.

> Can you describe what they should be doing, and do you know if they actually are doing it?

Callers of `FlushStateToDisk` already need to handle failures of the block index writing, so I don't think they require extra logic
...

💬 MarcoFalke commented on issue "error: in "validationinterface_tests/unregister_all_during_call": check destroyed has failed":
(https://github.com/bitcoin/bitcoin/issues/28146#issuecomment-1650073657)
Let's continue discussion in https://github.com/bitcoin/bitcoin/pull/28150

✅ MarcoFalke closed an issue: "error: in "validationinterface_tests/unregister_all_during_call": check destroyed has failed"
(https://github.com/bitcoin/bitcoin/issues/28146)

💬 MarcoFalke commented on pull request "test: Avoid intermittent issues due to async events in validationinterface_tests":
(https://github.com/bitcoin/bitcoin/pull/28150#issuecomment-1650075726)
An alternative might be to call `SyncWithValidationInterfaceQueue` at the start of the test.

💬 MarcoFalke commented on pull request "ci: Use qemu-user through container engine":
(https://github.com/bitcoin/bitcoin/pull/28087#issuecomment-1650081711)
Looks like CI is now green :)

Also the test failure from https://github.com/bitcoin/bitcoin/pull/27529#issuecomment-1521387810 can now be tested on this pull with just:

```
MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_s390x.sh" ./ci/test_run_all.sh
```

Output:
```
test 2023-07-25T15:37:42.430000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
F
...

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1650091313)
Updated d87edf309ffe60ae044a57ce9a0d9cc711edc365 -> 6960c81cbfa6208d4098353e53b313e13a21cb49 ([kernelRmUnivalue_8](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_8) -> [kernelRmUnivalue_9](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_9), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_8..kernelRmUnivalue_9))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1273714341), fixing release note.
...

💬 MarcoFalke commented on pull request "test: Drop 22.x node from TxindexCompatibilityTest":
(https://github.com/bitcoin/bitcoin/pull/28070#issuecomment-1650131175)
Dropped that change for now, to save it for a later pull.

💬 Crypt-iQ commented on pull request "test: Avoid intermittent issues due to async events in validationinterface_tests":
(https://github.com/bitcoin/bitcoin/pull/28150#issuecomment-1650137575)
> An alternative might be to call `SyncWithValidationInterfaceQueue` at the start of the test.

Tested it and both approaches work, not sure which is better. Using `ChainTestingSetup` should be faster?

🤔 jamesob reviewed a pull request: "BIP324 ciphersuite"
(https://github.com/bitcoin/bitcoin/pull/28008#pullrequestreview-1541600581)
Review check-in; up to (but not including) 9e65744c4f44ce89f3176870d3cc25ac2b6bdc08

💬 jamesob commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1270948739)
cacc8fe2b59e45cde248a031c5ebbf9ec39c5b8f

"thag." Sounds Nordic.

💬 jamesob commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1271064136)
df4303b996f8b4f095de18edd7d3dbe281b5f124

If you have to retouch, maybe worth keeping the `BOOST_CHECK_EQUAL` (which compiles for me okay) since it'll give a printout of the mismatch.

💬 jamesob commented on pull request "BIP324 ciphersuite":
(https://github.com/bitcoin/bitcoin/pull/28008#discussion_r1270986511)
5e0ca4ba371570d1a5275c5a79bc0c1253d97ead

Odd to me that `KEYLEN` doesn't live higher up in the class hierarchy, since the other `ChaCha20*` only deal in 32-byte keys, but definitely not a big deal.

📝 fanquake opened a pull request: "[WIP] guix: use `-muse-unaligned-vector-move` for Windows builds "
(https://github.com/bitcoin/bitcoin/pull/28151)
We currently work around a longstanding GCC issue with aligned vector instructions, by patching the behaviour we want into GCC (see discussion in #24736). Possibly in response to the GCC thread (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=54412#c40), a new option was [introduced into the binutils assembler](https://sourceware.org/git/?p=binutils-gdb.git;a=commit;h=c8480b58e1968f209b6365af7422678f348222c2) with the 2.38 release:
```
x86: Add -muse-unaligned-vector-move to assembler

Unaligne
...

💬 fanquake commented on pull request "[WIP] guix: use `-muse-unaligned-vector-move` for Windows builds":
(https://github.com/bitcoin/bitcoin/pull/28151#issuecomment-1650157656)
cc @theuni