polytune: use Block type consistently

[robinhundt]

closes #210

This also fixes some cases where there were potential timing side-channel issues by introducing a Block::const_mul method that multiplies a Block with a bool in constant time by using the conditional select implementation.

This does not really change performance, as Block was already used in the performance critical parts.

Summary by CodeRabbit

• Refactor
  • Updated public API to return Block values instead of u128 primitives for oblivious transfer and cryptographic operations.
  • Hash function signatures changed to operate with Block types instead of numeric primitives.
  • Data structure fields refactored throughout to use Block-based cryptographic representations for improved type consistency.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[Copilot]

Pull request overview

This PR refactors the codebase to consistently use the Block type instead of u128 throughout the cryptographic protocol implementation, addressing issue #210. The changes also introduce a constant-time multiplication method (Block::const_mul) to mitigate potential timing side-channel vulnerabilities.

• Replaced u128 with Block type across OT functions, data types, and protocol implementations
• Added Block::const_mul method for constant-time multiplication with boolean values
• Updated hash functions and serialization to work with Block instead of u128

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
src/ot.rs	Removed block_to_u128 conversion function and updated OT sender/receiver to return Vec<Block>
src/mpc/protocol.rs	Updated label initialization to use Block::ZERO instead of 0
src/mpc/garble.rs	Changed key/nonce generation to use as_bytes() and corrected endianness from to_be_bytes() to to_le_bytes()
src/mpc/fpre.rs	Updated Mac/Key initialization to use default() and comparison with Mac::default()
src/mpc/faand.rs	Extensive changes replacing u128 operations with Block, introducing const_mul for timing-safe operations
src/mpc/data_types.rs	Changed Delta, Mac, Key, and Label to wrap Block instead of u128
src/block.rs	Added const_mul method for constant-time multiplication
src/bench_reexports.rs	Updated return types from Vec<u128> to Vec<Block>

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

📝 Walkthrough

Walkthrough

The PR systematically replaces u128 with Block types across the codebase to leverage SIMD operations for improved performance. Core data structures (Delta, Mac, Key, Label) now wrap Block instead of u128, with corresponding updates to arithmetic operations, serialization, hashing, and default initializers.

Changes

Cohort / File(s)	Summary
Core data type refactoring src/mpc/data_types.rs	Delta, Mac, Key, Label wrapper types changed from u128 to Block. Updated BitAnd/BitXor operations to use Block-based arithmetic and const_mul. Reworked internal state initializers and aggregations to use Block XOR operations.
Hash and OT operations src/mpc/faand.rs, src/ot.rs	hash_vec and hash128 signatures changed to accept and return Block instead of u128. Removed block_to_u128 helper. kos_ot_sender and kos_ot_receiver now return Vec<Block> instead of Vec<u128>. Updated all hash and bitwise combination logic to operate on Block values.
Default initializers src/mpc/fpre.rs, src/mpc/protocol.rs	Replaced zero-value initializers (Key(0), Mac(0), Label(0)) with Key::default(), Mac::default(), and Label(Block::ZERO). Updated comparisons to use default values.
Serialization and label operations src/mpc/garble.rs	key_and_nonce now uses as_bytes() on Block-based labels instead of to_be_bytes(). Test triple updated to use Mac::default().
Public API re-exports and utilities src/bench_reexports.rs, src/block.rs	Return type updated for kos_ot_sender and kos_ot_receiver to Result<Vec<Block>, Error>. Added new const_mul method to Block for constant-time boolean multiplication.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

---

🐰 Blocks replace numbers, Swift and clean,
SIMD dances where u128 has been,
Garbled circuits now sparkle bright,
Cryptographic hops take flight! 🚀✨

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately and concisely describes the main objective of the PR: replacing u128 with Block type consistently across the codebase.
Linked Issues check	✅ Passed	All changes align with #210's requirement to replace u128 with Block for SIMD performance benefits. Block type is used consistently across data structures, hashing, OT functions, and arithmetic operations.
Out of Scope Changes check	✅ Passed	All changes are scoped to replacing u128 with Block type as per #210. No unrelated modifications to other functionality or logic are present.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch polytune/use-block-consistently

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6a4b730 and b7d50cc.

📒 Files selected for processing (8)

• src/bench_reexports.rs
• src/block.rs
• src/mpc/data_types.rs
• src/mpc/faand.rs
• src/mpc/fpre.rs
• src/mpc/garble.rs
• src/mpc/protocol.rs
• src/ot.rs

🧰 Additional context used

🧬 Code graph analysis (3)

src/ot.rs (3)

src/block.rs (1)

• new (52-54)

src/ot_core/kos.rs (2)

• init (89-97)
• init (192-200)

src/ot_core/mod.rs (2)

• init (49-54)
• init (93-98)

src/mpc/faand.rs (1)

src/block.rs (9)

• from (287-289)
• from (294-296)
• from (301-303)
• from (307-309)
• from (314-316)
• from (321-323)
• from (328-330)
• conditional_select (429-434)
• new (52-54)

src/mpc/data_types.rs (1)

src/block.rs (1)

• bitand (176-178)

🔇 Additional comments (33)

src/block.rs (1)

163-168: LGTM! Well-designed constant-time boolean multiplication.

The implementation correctly leverages subtle's Choice and Block::conditional_select to ensure constant-time execution, addressing potential timing side-channel vulnerabilities. The Choice::from(u8::from(b)) conversion chain maintains the constant-time property.

src/mpc/protocol.rs (4)

49-49: LGTM! Import added for Block-based zero initialization.

---

657-657: Correct zero-label initialization with Block::ZERO.

The change from Label(0) to Label(Block::ZERO) aligns with the updated Label type that now wraps Block instead of u128.

---

917-917: Consistent Block-based zero initialization.

---

997-997: Consistent Block-based zero initialization in evaluate path.

src/mpc/garble.rs (2)

73-74: LGTM! Key construction updated for Block-based Labels.

The change correctly uses Block::as_bytes() to extract the raw bytes from Label's inner Block for key construction.

---

85-88: Test updated for Block-based Mac type.

The change from Mac(0) to Mac::default() is consistent with Mac now wrapping Block (where Block::default() is Block::ZERO).

Also applies to: 102-102

src/mpc/fpre.rs (5)

94-94: Consistent use of Default for Block-based Key initialization.

---

102-102: Consistent use of Default for Block-based Mac and Key initialization.

---

155-155: Comparison updated for Block-based Mac type.

The comparison *mac_i != Mac::default() correctly checks for non-zero MAC values, equivalent to the previous Mac(0) comparison.

---

196-196: AND shares generation updated for Block-based types.

Also applies to: 204-204

---

365-368: Test code updated consistently with production changes.

Also applies to: 390-393

src/bench_reexports.rs (1)

21-22: LGTM! Benchmark API updated to return Block instead of u128.

The return type changes align with the underlying OT implementation changes. Since this is a benchmark-only API (requires __bench feature), the breaking change is acceptable.

Also applies to: 30-31

src/ot.rs (2)

13-13: LGTM! OT sender updated to return Block directly.

The change eliminates the unnecessary conversion to u128, keeping the output as Block for SIMD-optimized operations downstream.

Also applies to: 21-23

---

32-32: LGTM! OT receiver updated to return Block directly.

Also applies to: 36-39

src/mpc/data_types.rs (4)

7-8: LGTM! Delta now wraps Block for SIMD operations.

Also applies to: 11-11

---

13-19: Excellent use of const_mul for constant-time Delta masking.

The BitAnd<Delta> for bool implementation correctly uses Block::const_mul to ensure constant-time execution when computing bool & Delta. This is security-critical as it prevents timing side-channels when masking with the global key.

---

22-23: Core types updated to wrap Block.

Mac, Key, and Label now consistently wrap Block instead of u128, enabling SIMD-optimized operations throughout the codebase.

Also applies to: 42-43, 138-139

---

115-121: Clean refactor of xor_keys aggregation.

The change from in-place mutation (k.0 ^= key.0) to immutable XOR assignment (k = k ^ *key) is idiomatic Rust and works correctly with the Key type's BitXor implementation.

src/mpc/faand.rs (14)

108-132: LGTM! hash_vec updated to return Block.

The truncation to 128 bits with documented security implications (64-bit collision resistance for RHO <= 64) is appropriate for the protocol's requirements.

---

150-150: Correct Block::ZERO initialization for hash vectors.

---

399-399: Delta vector now uses Block type directly.

---

478-481: Constant-time mask approach retained for performance.

The manual mask construction Block::from((-(rbit as i128)) as u128) is equivalent to const_mul in terms of constant-time properties but faster, as noted by the author. Both approaches prevent timing side-channels.

Also applies to: 493-497

---

516-516: Default initialization for Mac/Key pairs.

---

619-625: Endianness consideration documented appropriately.

The TODO comment raises a valid point about cross-platform compatibility between little-endian and big-endian systems. For protocol interoperability across architectures, consider using explicit endianness (e.g., to_le_bytes()).

Is cross-architecture protocol interoperability (LE ↔ BE) a requirement for Polytune? If so, explicit endianness should be enforced here.

---

650-650: Correct use of Block::conditional_select for bi-based selection.

This maintains constant-time properties when selecting between d0 and d1 based on bi[r].

---

776-794: LGTM! hash128 updated to return Block.

Security implications of 128-bit truncation are documented, and the RHO <= 64 check ensures sufficient security margin.

---

826-826: Correct default initialization for zshares.

---

844-844: Proper use of const_mul throughout flaand.

The const_mul method is correctly used for boolean multiplication with Block values, maintaining constant-time execution for security-sensitive operations.

Also applies to: 865-866, 870-871, 883-886

---

908-908: Correct Block::ZERO comparison for verification.

---

1028-1028: Beaver triple computation uses const_mul correctly.

Also applies to: 1053-1054

---

1137-1137: Constant-time MAC verification in check_dvalue.

---

1180-1180: combine_two_leaky_ands updated with const_mul for constant-time security.

The multiplication of MAC and Key by boolean d uses const_mul to prevent timing side-channels during leaky AND combination.

Also applies to: 1189-1189, 1195-1196

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[zeitgeist]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.