-
Notifications
You must be signed in to change notification settings - Fork 79
macOS arm64 and Windows: build HW VDF client with FT4222H drivers (CI + docs) #299
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
hoffmang@MacBook-Air-2025-07 src % ./hw_vdf_client --list hoffmang@MacBook-Air-2025-07 src % ./hw_test |
8a9e40b to
9a5117e
Compare
| res_string.resize(mpz_sizeinbase(impl, 10)); | ||
| mpz_get_str(&(res_string[0]), 10, impl); | ||
| return res_string.c_str(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Buffer overflow in to_string_dec for negative integers
Medium Severity
to_string_dec() allocates mpz_sizeinbase(impl, 10) bytes, which per GMP docs returns the digit count ignoring the sign. For negative numbers (like discriminants), mpz_get_str writes a leading - sign plus digits plus a null terminator, overflowing the buffer by at least one byte. The to_string() method in the same file correctly adds + 2 extra bytes for this reason.
| { | ||
| init_gmp(); | ||
| fesetround(FE_TOWARDZERO); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newly created vdf_base_hw.cpp is unreferenced dead code
Medium Severity
vdf_base_hw.cpp is a newly created lightweight file providing VdfBaseInit without pulling in heavy VDF headers. However, it's never referenced by the CMake or Makefile build systems. The CMake HW build at line 412 uses vdf_base.cpp (which pulls in verifier.h, prover_slow.h, etc.) instead. This file appears intended to replace vdf_base.cpp for HW targets but was never wired up, leaving it as dead code and forcing an unnecessarily heavy include chain.
Additional Locations (1)
| reduce_form(qf2.a, qf2.b, qf2.c); | ||
| #else | ||
| reducer.reduce(qf2); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant form reduction in Windows emulator squaring loop
Low Severity
On Windows, the emulator calls square(st->qf) which internally calls form::reduce() before returning the result. Then reduce_form(qf2.a, qf2.b, qf2.c) is called again on the already-reduced form. The second reduction is a no-op that adds unnecessary work on every iteration of the hot emulation loop. Only one of the two reduction steps is needed.
| qf2 = square(st->qf); | ||
| #else | ||
| nudupl_form(qf2, st->qf, st->d, st->l); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows emulator uses different squaring algorithm than production
Medium Severity
On Windows, the emulator substitutes square() for nudupl_form() in the VDF squaring loop. While mathematically equivalent, using a fundamentally different algorithm in the emulator than what the real HW verification path expects undermines the emulator's purpose of faithfully simulating hardware behavior. The extensive check_valid diagnostics and bad-form logging added around this call confirm the substitution is producing unexpected results.
Potentially revert the 2weso bug
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| reduce_form(qf2.a, qf2.b, qf2.c); | ||
| #else | ||
| reducer.reduce(qf2); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows emulator double-reduces forms after squaring
Low Severity
On Windows, the emulator calls square(st->qf) which internally calls res.reduce() before returning, then immediately calls reduce_form(qf2.a, qf2.b, qf2.c) again on the already-reduced result. The second reduction is redundant since reducing an already-reduced form is a no-op. The non-Windows path correctly calls nudupl_form (which does not reduce) followed by a single reducer.reduce(qf2).


Summary
hw_vdf_client,hw_test, plus emu variants).macos-13-arm64, downloading FTDI’s LibFT4222 package and wiring headers + dylibs intosrc/hw/libft4222.@executable_pathrpath.README_ASIC.md.Notes
LibFT4222-mac-v1.4.4.190because it ships universal dylibs including arm64.hw_vdf_client(socket address initialization + portableuint64_tformatting).Test plan
src/hw/libft4222, then:./scripts/get-libft4222.sh install cd src make -f Makefile.vdf-client hw_test hw_vdf_client emu_hw_test emu_hw_vdf_client ./hw_vdf_client --listNote
Medium Risk
Moderate risk due to broad build-system and low-level portability changes (CMake/Makefile, asm generation, allocator and Windows ABI/socket handling) that could affect cross-platform builds and runtime behavior even though core algorithms are largely unchanged.
Overview
Adds cross-platform CI builds for the hardware VDF client: new GitHub Actions jobs build and artifact
hw_vdf_client/hw_test(and emu variants) on macOS arm64 and Windows, plus smoke tests; the main test workflow now also runs on Windows (with sanitizer exclusions) using a CMake/Ninja + clang-cl build.Introduces helper scripts (
get-libft4222.shandget-libft4222.ps1) to download/stage FT4222H driver headers/libs and updates docs/ignore rules accordingly. Build system changes expandsrc/CMakeLists.txtwith toggles for building client/bench/tests/HW tools, add Windows/macOS compatibility (Boost/MPIR handling, rpaths, calling convention/asm tweaks, aligned alloc, logging/format fixes), and switch shared parsing streams tothread_localplus AVX flag detection to atomics/env-controlled behavior.Written by Cursor Bugbot for commit 41b1369. This will update automatically on new commits. Configure here.