feat: statement for the Binary Basefold protocol - #100
Conversation
|
/review External: Internal: Comments: |
🤖 AI Review (with external context)\n\nGenerating AI review...Your pull request has been reviewed. Here is the summary and detailed feedback: High-level SummaryThis pull request introduces a new file to formalize a single round of the Binary Basefold protocol as an interactive oracle reduction. This is a foundational step towards formalizing the complete Binius polynomial commitment scheme. The PR defines the necessary data structures for statements, witnesses, and relations for one round, and outlines the prover and verifier logic within the existing The overall structure aligns well with the reference paper, and the use of the repository's existing framework is appropriate. However, the implementation is still a work in progress with many key definitions and proofs left as General Feedback
Specific Feedback & Actionable SuggestionsHere are some specific points that need attention: 1. Missing Characteristic 2 ConstraintIssue: The formalization is for "Binary Basefold", which, as the name and reference paper (Section 4) imply, operates over binary fields (characteristic 2). The current code defines the field Suggestion: Add a -- In ArkLib/ProofSystem/Binius/Basic.lean
-- ...
namespace Binius
namespace BinaryBasefold
noncomputable section Esstentials
universe u
variable {r : ℕ} [NeZero r]
-- Add [CharP L 2] constraint
variable {L : Type u} [Field L] [CharP L 2] [Fintype L] [DecidableEq L]
-- ...
end Esstentials
-- ...
end BinaryBasefold
end Binius2. Incomplete Key DefinitionsIssue: Several critical definitions are incomplete and rely on
Suggestion:
3. Undefined Variable in Soundness ProofIssue: In -- Problematic code in ArkLib/ProofSystem/Binius/Basic.lean
theorem verifier_rbrKnowledgeSoundness [Fintype L] (i : Fin ℓ) :
...
(rbrKnowledgeError:=fun _ => (deg : ℝ≥0) / (Fintype.card L)) := by
sorrySuggestion: This variable likely refers to the degree of the sumcheck polynomial -- Corrected code
theorem verifier_rbrKnowledgeSoundness [Fintype L] (i : Fin ℓ) :
let deg := 1 -- The degree of the sumcheck polynomial hᵢ(X)
(verifier ...).rbrKnowledgeSoundness init impl
...
(rbrKnowledgeError:=fun _ => (deg : ℝ≥0) / (Fintype.card L)) := by
sorry4. Ambiguity of
|
f867ab5 to
89a3c86
Compare
66cb4e8 to
99ac7dd
Compare
311f6cd to
4025260
Compare
c358648 to
b6469e8
Compare
|
@chung-thai-nguyen : can you split this PR into multiple parts? Seems like there are three main protocols: FRI-Binius, Binary-Basefold, and Ring switching (in the binary case) Each one should be its own PR? Scope the dependencies so there is an ordering to how each PR should be merged. And what is the difference between FRI-Binius and Binary-Basefold? |
Yep I will split it into 3 PRs in this order: Binary-Basefold -> Ring switching -> FRI-Binius. |
|
/review External: Internal: Comments: |
🤖 AI Review (with external context)\n\nGenerating AI review...Excellent, this is a substantial pull request that introduces a formalization of the Binius proof system. As an expert code reviewer, I've analyzed the changes in detail, keeping the reference paper, repository context, and best practices in mind. Here is my review: High-Level SummaryThis pull request introduces a large-scale implementation of the Binius proof system, which, according to the reference paper, combines a "ring-switching" technique with a characteristic-2 adaptation of the BaseFold polynomial commitment scheme. The changes are well-structured, breaking down the protocol into three main components: The PR also adds foundational utilities to existing files, notably for:
Architecturally, this is a strong foundation for formalizing a complex, state-of-the-art proof system in Lean. Overall FeedbackThis pull request represents a significant and well-thought-out architectural effort. The modular decomposition of the Binius protocol into phases and the use of the However, the most critical issue is that the implementation is largely a skeleton. A vast majority of the logic and almost all proofs are currently placeholders ( Recommendation: I cannot approve this PR for merging in its current state. I recommend breaking this down into a series of smaller, self-contained, and fully proven pull requests. A good approach would be:
This will make the review process more manageable and ensure each component is correct before being built upon. Specific Issues and SuggestionsBelow are specific points of feedback on the code changes. 1.
|
65bd98c to
a0470e1
Compare
a0470e1 to
29b6687
Compare
e668a31 to
0feade4
Compare
32d2cb2 to
fb35e3a
Compare
e84a5e9 to
6e8f675
Compare
6e8f675 to
bcf91a0
Compare
bcf91a0 to
df675fb
Compare
7f984d4 to
226f636
Compare
|
/review External: Internal: Comments: |
🤖 AI Review (with external context)\n\nGenerating AI review...Thank you for the opportunity to review this pull request. Here is my detailed analysis: High-Level SummaryThis pull request introduces a substantial and well-structured implementation of the Binary Basefold polynomial commitment scheme, as described in the reference paper "Polylogarithmic Proofs for Multilinears over Binary Towers". The changes span new protocol specifications, interactive oracle reductions for each protocol step, and their composition into a full protocol. The PR also includes necessary modifications to supporting libraries, particularly for Additive NTT, bitwise operations, and polynomial properties. The modular design, breaking the complex protocol into smaller, composable However, a major concern is that all security proofs ( General FeedbackCorrectness and Completeness
Code Style and Readability
File-Specific Feedback
|
226f636 to
fb32824
Compare
🤖 Gemini PR SummaryThis diff introduces a comprehensive implementation of the Binary Basefold protocol, a core component of the Binius proof system. The key changes are:
Analysis of Changes
Last updated: 2025-10-31 02:10 UTC. See the main CI run for build status. |
[x] Full statement for Binary Basefold using Oracle Reduction
Note
Introduces a full Binary Basefold protocol framework (specs, verifiers, reductions, core interaction and query phases) and augments AdditiveNTT with domain/fiber, quotient-map, folding, and linearity utilities to support it.
BinaryBasefold.{Prelude,Basic,Steps,CoreInteractionPhase,General,QueryPhase,Spec}.pSpec*), oracle statements/interfaces, and per-round verifiers/reductions for fold, relay, commit, and final sumcheck.fold,iterated_fold), and matrix/tensor forms.W,normalizedW, composition, and linear maps.sDomainbasis/size, bijectionssDomainFinEquiv, and conversions between indices/points.intermediateEvaluationPoly, refinements) and sumcheck identities.Binius.Basicimport with new Binary Basefold modules.Written by Cursor Bugbot for commit aaa6016. This will update automatically on new commits. Configure here.