From 0252d5c7625bf50c46a37a07b82a3dd8f4250572 Mon Sep 17 00:00:00 2001
From: Daniel Lubarov <daniel@lubarov.com>
Date: Tue, 27 Apr 2021 09:06:07 -0700
Subject: [PATCH] Fix Challenger's duplicate challenge bug

`absorb_buffered_inputs` is called even if the input buffer is empty. In that case it should no-op, but it was instead replenishing the output buffer because of this line:

    self.output_buffer = self.sponge_state[0..SPONGE_RATE].to_vec();

Easiest fix is to skip that code if the input buffer is empty.
---
 src/plonk_challenger.rs | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/src/plonk_challenger.rs b/src/plonk_challenger.rs
index a8f5e605..05eaea16 100644
--- a/src/plonk_challenger.rs
+++ b/src/plonk_challenger.rs
@@ -89,6 +89,10 @@ impl<F: Field> Challenger<F> {
 
     /// Absorb any buffered inputs. After calling this, the input buffer will be empty.
     fn absorb_buffered_inputs(&mut self) {
+        if self.input_buffer.is_empty() {
+            return;
+        }
+
         for input_chunk in self.input_buffer.chunks(SPONGE_RATE) {
             // Overwrite the first r elements with the inputs. This differs from a standard sponge,
             // where we would xor or add in the inputs. This is a well-known variant, though,
@@ -189,6 +193,10 @@ impl RecursiveChallenger {
 
     /// Absorb any buffered inputs. After calling this, the input buffer will be empty.
     fn absorb_buffered_inputs<F: Field>(&mut self, builder: &mut CircuitBuilder<F>) {
+        if self.input_buffer.is_empty() {
+            return;
+        }
+
         for input_chunk in self.input_buffer.chunks(SPONGE_RATE) {
             // Overwrite the first r elements with the inputs. This differs from a standard sponge,
             // where we would xor or add in the inputs. This is a well-known variant, though,
@@ -218,6 +226,25 @@ mod tests {
     use crate::target::Target;
     use crate::witness::PartialWitness;
 
+    #[test]
+    fn no_duplicate_challenges() {
+        type F = CrandallField;
+        let mut challenger = Challenger::new();
+        let mut challenges = Vec::new();
+
+        for i in 1..10 {
+            challenges.extend(challenger.get_n_challenges(i));
+            challenger.observe_element(F::rand());
+        }
+
+        let dedup_challenges = {
+            let mut dedup = challenges.clone();
+            dedup.dedup();
+            dedup
+        };
+        assert_eq!(dedup_challenges, challenges);
+    }
+
     /// Tests for consistency between `Challenger` and `RecursiveChallenger`.
     #[test]
     fn test_consistency() {