HAL: refactor salt_root functions #1717
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
cedwies
changed the title
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
benma
left a comment
benma
left a comment
There was a problem hiding this comment.
utACK, nice! (Left a small comment, please take a look at that before merging).
| lock(); | ||
|
|
||
| let mut hal = TestingHal::new(); | ||
| let _ = hal.memory.reset_hww(); |
There was a problem hiding this comment.
Why is this call needed?
If needed, prefer reset_hww().unwrap() instead of ignoring the output.
edit: tested, tests also pass without this line. TestingHal::new() inits the fields to the same values after all.
cedwies
force-pushed
the
cedwies/salt-root
branch
3 times, most recently
from
8b8e98b to
aa315e3
Compare
Centralizes salt root access behind the HAL abstraction, enabling full test isolation without C memory dependencies. TestingHal initializes with a default salt root to preserve existing test behavior without manual setup in each test. The 2 rust_salt_hash_data tests keep using mock_memory() + C-backed salt root because the extern "C" shim internally constructs BitBox02Hal—these tests verify the FFI path works.
cedwies
force-pushed
the
cedwies/salt-root
branch
from
aa315e3 to
d0db51a
Compare
2 participants
Centralizes salt root access behind the HAL abstraction, enabling full test isolation without C memory dependencies.
TestingHal initializes with a default salt root to preserve existing test behavior without manual setup in each test.
The 2 rust_salt_hash_data tests keep using mock_memory() + C-backed salt root because the extern "C" shim internally constructs BitBox02Hal—these tests verify the FFI path works.