From 8268f8da47b76d16a0c7fd3179341a904bc89c35 Mon Sep 17 00:00:00 2001 From: Ryan Kim Date: Sat, 13 Apr 2024 17:39:56 +0900 Subject: [PATCH 1/2] chore(math): add helpful error message when failing to parse `BigInt` --- tachyon/math/base/big_int.cc | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tachyon/math/base/big_int.cc b/tachyon/math/base/big_int.cc index 63927a189..a5c10e320 100644 --- a/tachyon/math/base/big_int.cc +++ b/tachyon/math/base/big_int.cc @@ -13,8 +13,17 @@ namespace { template bool DoStringToLimbs(std::string_view str, uint64_t* limbs, size_t limb_nums) { mpz_class out; - if (!gmp::ParseIntoMpz(str, Base, &out)) return false; - if (gmp::GetLimbSize(out) > limb_nums) return false; + if (!gmp::ParseIntoMpz(str, Base, &out)) { + LOG(ERROR) << "failed to parse string(" << str << ") with base(" << Base + << ")"; + return false; + } + size_t actual_limb_nums = gmp::GetLimbSize(out); + if (actual_limb_nums > limb_nums) { + LOG(ERROR) << "actual limb nums(" << actual_limb_nums + << ") is greater than expected limb nums(" << limb_nums << ")"; + return false; + } gmp::CopyLimbs(out, limbs); return true; } From 54e801e69e3cb63826d051efedcac63a17de8b03 Mon Sep 17 00:00:00 2001 From: Ryan Kim Date: Sat, 13 Apr 2024 17:44:51 +0900 Subject: [PATCH 2/2] feat(math): add range check to `From(Dec|Hex)String` While this may appear redundant in debug mode due to the existing range check in the constructor of PrimeField, we find it acceptable since `From(Dec|Hex)String` is not called frequently and the performance impact in release mode is minimal. --- .../generator/prime_field_generator/prime_field_x86.h.tpl | 8 ++++++++ .../goldilocks/goldilocks_prime_field_x86_special.cc | 8 ++++++++ tachyon/math/finite_fields/prime_field_generic.h | 8 ++++++++ tachyon/math/finite_fields/prime_field_gpu.h | 8 ++++++++ tachyon/math/finite_fields/prime_field_gpu_debug.h | 8 ++++++++ 5 files changed, 40 insertions(+) diff --git a/tachyon/math/finite_fields/generator/prime_field_generator/prime_field_x86.h.tpl b/tachyon/math/finite_fields/generator/prime_field_generator/prime_field_x86.h.tpl index 6165b37d4..b8564db49 100644 --- a/tachyon/math/finite_fields/generator/prime_field_generator/prime_field_x86.h.tpl +++ b/tachyon/math/finite_fields/generator/prime_field_generator/prime_field_x86.h.tpl @@ -69,11 +69,19 @@ class PrimeField<_Config, std::enable_if_t<_Config::%{flag}>> final constexpr static std::optional FromDecString(std::string_view str) { std::optional> value = BigInt::FromDecString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeField(std::move(value).value()); } constexpr static std::optional FromHexString(std::string_view str) { std::optional> value = BigInt::FromHexString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeField(std::move(value).value()); } diff --git a/tachyon/math/finite_fields/goldilocks/goldilocks_prime_field_x86_special.cc b/tachyon/math/finite_fields/goldilocks/goldilocks_prime_field_x86_special.cc index 88d584473..23003fb1b 100644 --- a/tachyon/math/finite_fields/goldilocks/goldilocks_prime_field_x86_special.cc +++ b/tachyon/math/finite_fields/goldilocks/goldilocks_prime_field_x86_special.cc @@ -34,6 +34,10 @@ template std::optional CLASS::FromDecString(std::string_view str) { uint64_t value = 0; if (!base::StringToUint64(str, &value)) return std::nullopt; + if (value >= Config::kModulus[0]) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeField(::Goldilocks::fromU64(value).fe); } @@ -42,6 +46,10 @@ template std::optional CLASS::FromHexString(std::string_view str) { uint64_t value = 0; if (!base::HexStringToUint64(str, &value)) return std::nullopt; + if (value >= Config::kModulus[0]) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeField(::Goldilocks::fromU64(value).fe); } diff --git a/tachyon/math/finite_fields/prime_field_generic.h b/tachyon/math/finite_fields/prime_field_generic.h index 117bfe7f4..a9df92176 100644 --- a/tachyon/math/finite_fields/prime_field_generic.h +++ b/tachyon/math/finite_fields/prime_field_generic.h @@ -75,12 +75,20 @@ class PrimeField<_Config, std::enable_if_t> final std::string_view str) { std::optional> value = BigInt::FromDecString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeField(std::move(value).value()); } constexpr static std::optional FromHexString( std::string_view str) { std::optional> value = BigInt::FromHexString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeField(std::move(value).value()); } diff --git a/tachyon/math/finite_fields/prime_field_gpu.h b/tachyon/math/finite_fields/prime_field_gpu.h index 346d6801a..788b253e0 100644 --- a/tachyon/math/finite_fields/prime_field_gpu.h +++ b/tachyon/math/finite_fields/prime_field_gpu.h @@ -73,11 +73,19 @@ class PrimeFieldGpu final : public PrimeFieldBase> { constexpr static PrimeFieldGpu FromDecString(std::string_view str) { std::optional> value = BigInt::FromDecString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeFieldGpu(std::move(value).value()); } constexpr static PrimeFieldGpu FromHexString(std::string_view str) { std::optional> value = BigInt::FromHexString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeFieldGpu(std::move(value).value()); } diff --git a/tachyon/math/finite_fields/prime_field_gpu_debug.h b/tachyon/math/finite_fields/prime_field_gpu_debug.h index 359697f7e..433570d95 100644 --- a/tachyon/math/finite_fields/prime_field_gpu_debug.h +++ b/tachyon/math/finite_fields/prime_field_gpu_debug.h @@ -72,11 +72,19 @@ class PrimeFieldGpuDebug final constexpr static PrimeFieldGpuDebug FromDecString(std::string_view str) { std::optional> value = BigInt::FromDecString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeFieldGpuDebug(std::move(value).value()); } constexpr static PrimeFieldGpuDebug FromHexString(std::string_view str) { std::optional> value = BigInt::FromHexString(str); if (!value.has_value()) return std::nullopt; + if (value >= Config::kModulus) { + LOG(ERROR) << "value(" << str << ") is greater than or equal to modulus"; + return std::nullopt; + } return PrimeFieldGpuDebug(std::move(value).value()); }