💬 hebasto commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644428684)
@sipa
> Ok so what seems to be happening is that in your build, `%16` gets mapped to `(%rsp)` (aka the memory address stored in the stack pointer), which in a few places gets used as e.g. `8+%16`, which results in `8+(%rsp)`, and that appears to be invalid asm code.
>
> On my system (and presumably ~every build we have since this code was introduced), `%16` is mapped to `16(%rsp)` (16 bytes further than the memory address stored in the stack pointer), and thus `8+%16` becomes `8+16(%rsp)`, w
...

💬 sipa commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644430606)
Yes, indirectly. Having that flag or not affects the stack layout. The issue is triggered by the "xfer" variable being allocated at the top of the stack.

📝 jonatack opened a pull request: "test: update tool_wallet coverage for unexpected writes to wallet"
(https://github.com/bitcoin/bitcoin/pull/28116)
PR #15687 added test coverage in `test/functional/tool_wallet.py` to reproduce unexpected writes to the wallet file, as described in issue https://github.com/bitcoin/bitcoin/issues/15608:

- wallet tool info unexpectedly writes to the wallet file if the wallet file permissions are read/write
- wallet tool info raises if the wallet file permissions are read-only

Update these tests/docs for two changes since #15687:

- the CI migration away from Appveyor
- the addition of descriptor walle
...

💬 MarcoFalke commented on pull request "test: update tool_wallet coverage for unexpected writes to wallet":
(https://github.com/bitcoin/bitcoin/pull/28116#discussion_r1269857579)
Instead of putting the error message in a static comment, it may be better to check that the error message is thrown at runtime, and only keep the comment that it shouldn't throw.

💬 MarcoFalke commented on pull request "refactor: Open file in FileCommit":
(https://github.com/bitcoin/bitcoin/pull/28006#issuecomment-1644469037)
Closing for now.

✅ MarcoFalke closed a pull request: "refactor: Open file in FileCommit"
(https://github.com/bitcoin/bitcoin/pull/28006)

🤔 jonatack reviewed a pull request: "test: Add unit & functional test coverage for blockstore"
(https://github.com/bitcoin/bitcoin/pull/27850#pullrequestreview-1539942275)
ACK 068b523ee7720e6a392efc294ff89bf936a3e2ac reviewed the tests only, didn't review the CI and `is_unix_root` commits

💬 jonatack commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1269874331)
It looks like the PR description section about `skip_if_root` is now out of date.

💬 jonatack commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1269876108)
If you retouch, I think the logging for the assertions inside this `is_unix_root` conditional should be logged as `info` (maybe all the debug logging in this file currently could be info).

💬 sipa commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644484394)
FWIW, I assume this issue has existed since this code was introduced, but isn't harmful apart from preventing compilation, as far as I can tell.

💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1269884246)
thank you., fixed

💬 fanquake commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644486605)
I don't actually think it even prevents compilation, and only warns? Unclear from the issue.

💬 hebasto commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644487671)
> I don't actually think it even prevents compilation, and only warns? Unclear from the issue.

Only warnings. Compilation succeeds.

💬 sipa commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644488214)
Errr, if it doesn't prevent compilation it's possible that the result is wrong. Do tests pass?

💬 hebasto commented on issue ""missing operand" assembler warnings on Windows":
(https://github.com/bitcoin/bitcoin/issues/28111#issuecomment-1644488972)
> Errr, if it doesn't prevent compilation it's possible that the result is wrong (when the warning is emitted). Do tests pass?

They do.

💬 MarcoFalke commented on pull request "ci: Set DEBUG=1 for valgrind fuzz task":
(https://github.com/bitcoin/bitcoin/pull/28106#issuecomment-1644495948)
Looks like this will be too slow for the functional tests, and there are other tasks setting it, so I've only added it to the fuzz valgrind task, which had it not previously set.

An alternative might be to add a new fuzz task with just DEBUG=1 set?

💬 MarcoFalke commented on pull request "ci: Set DEBUG=1 for valgrind fuzz task":
(https://github.com/bitcoin/bitcoin/pull/28106#issuecomment-1644514902)
Maybe I'll just do it in another repo, instead of here. Closing for now.

✅ MarcoFalke closed a pull request: "ci: Set DEBUG=1 for valgrind fuzz task"
(https://github.com/bitcoin/bitcoin/pull/28106)

💬 Crypt-iQ commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#issuecomment-1644517170)
crACK fa633aa

💬 brunoerg commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1269917734)
After running it for some time, the maximum exec/s I got was 21 exec/s which is extremely low. It's not all the targets that reach more than 1000 exec/s here but 21 is too slow.