Uplift of #3819 (squashed) to beta

brave · Nov 6, 2019 · f1fd360 · f1fd360
1 parent b46e615
commit f1fd360
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 30 deletions.
diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/contribution/phase_two.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/contribution/phase_two.cc
@@ -234,6 +234,7 @@ void PhaseTwo::PrepareBatch(
 
   auto callback = std::bind(&PhaseTwo::PrepareBatchCallback,
                             this,
+                            transaction.viewingId_,
                             _1,
                             _2,
                             _3);
@@ -245,7 +246,38 @@ void PhaseTwo::PrepareBatch(
       callback);
 }
 
+void PhaseTwo::AssignPrepareBallots(
+    const std::string& viewing_id,
+    const std::vector<std::string>& surveyors,
+    braveledger_bat_helper::Ballots* ballots) {
+  for (size_t j = 0; j < surveyors.size(); j++) {
+    std::string error;
+    braveledger_bat_helper::getJSONValue("error", surveyors[j], &error);
+    if (!error.empty()) {
+      // TODO(nejczdovc) what should we do here
+      continue;
+    }
+
+    std::string surveyor_id;
+    bool success = braveledger_bat_helper::getJSONValue("surveyorId",
+                                                        surveyors[j],
+                                                        &surveyor_id);
+    if (!success) {
+      // TODO(nejczdovc) what should we do here
+      continue;
+    }
+
+    for (auto& ballot : *ballots) {
+      if (ballot.surveyorId_ == surveyor_id &&
+          ballot.viewingId_ == viewing_id) {
+        ballot.prepareBallot_ = surveyors[j];
+      }
+    }
+  }
+}
+
 void PhaseTwo::PrepareBatchCallback(
+    const std::string& viewing_id,
     int response_status_code,
     const std::string& response,
     const std::map<std::string, std::string>& headers) {
@@ -264,38 +296,9 @@ void PhaseTwo::PrepareBatchCallback(
     return;
   }
 
-  braveledger_bat_helper::Transactions transactions =
-    ledger_->GetTransactions();
   braveledger_bat_helper::Ballots ballots = ledger_->GetBallots();
 
-  for (size_t j = 0; j < surveyors.size(); j++) {
-    std::string error;
-    braveledger_bat_helper::getJSONValue("error", surveyors[j], &error);
-    if (!error.empty()) {
-      // TODO(nejczdovc) what should we do here
-      continue;
-    }
-
-    std::string surveyor_id;
-    bool success = braveledger_bat_helper::getJSONValue("surveyorId",
-                                                        surveyors[j],
-                                                        &surveyor_id);
-    if (!success) {
-      // TODO(nejczdovc) what should we do here
-      continue;
-    }
-
-    for (int i = ballots.size() - 1; i >= 0; i--) {
-      if (ballots[i].surveyorId_ == surveyor_id) {
-        for (size_t k = 0; k < transactions.size(); k++) {
-          if (transactions[k].viewingId_ == ballots[i].viewingId_ &&
-              ballots[i].proofBallot_.empty()) {
-            ballots[i].prepareBallot_ = surveyors[j];
-          }
-        }
-      }
-    }
-  }
+  AssignPrepareBallots(viewing_id, surveyors, &ballots);
 
   ledger_->SetBallots(ballots);
   Proof();

diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/contribution/phase_two.h b/vendor/bat-native-ledger/src/bat/ledger/internal/contribution/phase_two.h
@@ -73,10 +73,16 @@ class PhaseTwo {
       const braveledger_bat_helper::TRANSACTION_ST& transaction);
 
   void PrepareBatchCallback(
+      const std::string& viewing_id,
       int response_status_code,
       const std::string& response,
       const std::map<std::string, std::string>& headers);
 
+  static void AssignPrepareBallots(
+      const std::string& viewing_id,
+      const std::vector<std::string>& surveyors,
+      braveledger_bat_helper::Ballots* ballots);
+
   std::vector<std::string> ProofBatch(
       const braveledger_bat_helper::BatchProofs& batch_proofs);
 
@@ -100,6 +106,7 @@ class PhaseTwo {
   // For testing purposes
   friend class PhaseTwoTest;
   FRIEND_TEST_ALL_PREFIXES(PhaseTwoTest, GetStatisticalVotingWinners);
+  FRIEND_TEST_ALL_PREFIXES(PhaseTwoTest, AssignPrepareBallotsRespectsViewingID);
 };
 
 }  // namespace braveledger_contribution

diff --git a/vendor/bat-native-ledger/src/bat/ledger/internal/contribution/phase_two_unittest.cc b/vendor/bat-native-ledger/src/bat/ledger/internal/contribution/phase_two_unittest.cc
@@ -77,4 +77,29 @@ TEST_F(PhaseTwoTest, GetStatisticalVotingWinners) {
   }
 }
 
+// Surveyor IDs are not unique and may be shared between different transactions.
+// Ensure that when assigning prepareBallot objects to ballots, we only assign
+// to ballots for the current viewing ID, even if they share a surveyor ID.
+TEST_F(PhaseTwoTest, AssignPrepareBallotsRespectsViewingID) {
+  const std::string shared_surveyor_id =
+      "Ad5pNzrwhWokTOR8/hC83LWJfEy8aY7mFwPQWe6CpRF";
+  const std::vector<std::string> surveyors = {
+      "{\"surveyorId\":\"" + shared_surveyor_id + "\"}"
+  };
+
+  // Create ballots with different viewing IDs but the same surveyor ID.
+  braveledger_bat_helper::Ballots ballots(2);
+  ballots[0].viewingId_ = "00000000-0000-0000-0000-000000000000";
+  ballots[0].surveyorId_ = shared_surveyor_id;
+  ballots[1].viewingId_ = "ffffffff-ffff-ffff-ffff-ffffffffffff";
+  ballots[1].surveyorId_ = shared_surveyor_id;
+
+  // Check that only ballot[0] with the matching viewing ID is updated. Ballot 1
+  // should remain unmodified.
+  PhaseTwo::AssignPrepareBallots("00000000-0000-0000-0000-000000000000",
+      surveyors, &ballots);
+  ASSERT_FALSE(ballots[0].prepareBallot_.empty());
+  ASSERT_TRUE(ballots[1].prepareBallot_.empty());
+}
+
 }  // namespace braveledger_contribution