Skip to content

Commit

Permalink
Merge pull request #4479 from randombit/jack/better-mod-sub
Browse files Browse the repository at this point in the history
Use a better approach for modular subtraction
  • Loading branch information
randombit authored Dec 17, 2024
2 parents eadd6b1 + 33412b7 commit c8ffa1f
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 40 deletions.
8 changes: 1 addition & 7 deletions src/lib/math/bigint/big_ops2.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -105,13 +105,7 @@ BigInt& BigInt::mod_sub(const BigInt& s, const BigInt& mod, secure_vector<word>&
ws.resize(mod_sw);
}

if(mod_sw == 4) {
bigint_mod_sub_n<4>(mutable_data(), s._data(), mod._data(), ws.data());
} else if(mod_sw == 6) {
bigint_mod_sub_n<6>(mutable_data(), s._data(), mod._data(), ws.data());
} else {
bigint_mod_sub(mutable_data(), s._data(), mod._data(), mod_sw, ws.data());
}
bigint_mod_sub(mutable_data(), s._data(), mod._data(), mod_sw, ws.data());

return (*this);
}
Expand Down
35 changes: 5 additions & 30 deletions src/lib/math/mp/mp_core.h
Original file line number Diff line number Diff line change
Expand Up @@ -737,38 +737,13 @@ inline constexpr int32_t bigint_sub_abs(W z[], const W x[], size_t x_size, const
*/
template <WordType W>
inline constexpr void bigint_mod_sub(W t[], const W s[], const W mod[], size_t mod_sw, W ws[]) {
// is t < s or not?
const auto is_lt = bigint_ct_is_lt(t, mod_sw, s, mod_sw);
// ws = t - s
const W borrow = bigint_sub3(ws, t, mod_sw, s, mod_sw);

// ws = p - s
const W borrow = bigint_sub3(ws, mod, mod_sw, s, mod_sw);
// Conditionally add back the modulus
bigint_cnd_add(borrow, ws, mod, mod_sw);

// Compute either (t - s) or (t + (p - s)) depending on mask
const W carry = bigint_cnd_addsub(is_lt, t, ws, s, mod_sw);

if(!std::is_constant_evaluated()) {
BOTAN_DEBUG_ASSERT(borrow == 0 && carry == 0);
}

BOTAN_UNUSED(carry, borrow);
}

template <size_t N, WordType W>
inline constexpr void bigint_mod_sub_n(W t[], const W s[], const W mod[], W ws[]) {
// is t < s or not?
const auto is_lt = bigint_ct_is_lt(t, N, s, N);

// ws = p - s
const W borrow = bigint_sub3(ws, mod, N, s, N);

// Compute either (t - s) or (t + (p - s)) depending on mask
const W carry = bigint_cnd_addsub(is_lt, t, ws, s, N);

if(!std::is_constant_evaluated()) {
BOTAN_DEBUG_ASSERT(borrow == 0 && carry == 0);
}

BOTAN_UNUSED(carry, borrow);
copy_mem(t, ws, mod_sw);
}

/**
Expand Down
7 changes: 6 additions & 1 deletion src/lib/math/pcurves/pcurves_impl/pcurves_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,12 @@ class IntMod final {
return Self(r);
}

friend constexpr Self operator-(const Self& a, const Self& b) { return a + b.negate(); }
friend constexpr Self operator-(const Self& a, const Self& b) {
std::array<W, N> r;
word carry = bigint_sub3(r.data(), a.data(), N, b.data(), N);
bigint_cnd_add(carry, r.data(), N, P.data(), N);
return Self(r);
}

/// Return (*this) divided by 2
Self div2() const {
Expand Down
18 changes: 16 additions & 2 deletions src/tests/test_pcurves.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -156,8 +156,22 @@ class Pcurve_Arithmetic_Tests final : public Test {
const auto g_plus_g = g_one + g_one;
result.test_eq("2*g == g+g", g_two.to_affine().serialize(), g_plus_g.to_affine().serialize());

result.confirm("Scalar::zero is zero", curve->scalar_zero().is_zero());
result.confirm("Scalar::one is not zero", !curve->scalar_one().is_zero());
result.confirm("Scalar::zero is zero", zero.is_zero());
result.confirm("(zero+zero) is zero", (zero + zero).is_zero());
result.confirm("(zero*zero) is zero", (zero * zero).is_zero());
result.confirm("(zero-zero) is zero", (zero - zero).is_zero());

const auto neg_zero = zero.negate();
result.confirm("zero.negate() is zero", neg_zero.is_zero());

result.confirm("(zero+nz) is zero", (zero + neg_zero).is_zero());
result.confirm("(nz+nz) is zero", (neg_zero + neg_zero).is_zero());
result.confirm("(nz+zero) is zero", (neg_zero + zero).is_zero());

result.confirm("Scalar::one is not zero", !one.is_zero());
result.confirm("(one-one) is zero", (one - one).is_zero());
result.confirm("(one+one.negate()) is zero", (one + one.negate()).is_zero());
result.confirm("(one.negate()+one) is zero", (one.negate() + one).is_zero());

for(size_t i = 0; i != 16; ++i) {
const auto pt = curve->mul_by_g(curve->random_scalar(rng), rng).to_affine();
Expand Down

0 comments on commit c8ffa1f

Please sign in to comment.