Skip to content

Commit

Permalink
Issue with Ads Payments (August 5)
Browse files Browse the repository at this point in the history
Fixes brave/brave-browser#5548

* Fix race condition where same token was reused due to network latency etc.
* Fix issue where only 1 failed confirmation was retried per session
* Migrate failed confirmation tokens which return HTTP_NOT_FOUND due to race condition
* Fix HTTP_BAD_REQUEST errors when retrying failed confirmations if the confirmation was already created
  • Loading branch information
tmancey committed Aug 7, 2019
1 parent 019c1f1 commit 042bb27
Show file tree
Hide file tree
Showing 6 changed files with 216 additions and 140 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ ConfirmationInfo::ConfirmationInfo() :
token_info(TokenInfo()),
payment_token(nullptr),
blinded_payment_token(nullptr),
credential("") {}
credential(""),
timestamp_in_seconds(0),
created(false) {}

ConfirmationInfo::ConfirmationInfo(const ConfirmationInfo& info) :
id(info.id),
Expand All @@ -23,7 +25,9 @@ ConfirmationInfo::ConfirmationInfo(const ConfirmationInfo& info) :
token_info(info.token_info),
payment_token(info.payment_token),
blinded_payment_token(info.blinded_payment_token),
credential(info.credential) {}
credential(info.credential),
timestamp_in_seconds(info.timestamp_in_seconds),
created(info.created) {}

ConfirmationInfo::~ConfirmationInfo() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#ifndef BAT_CONFIRMATIONS_INTERNAL_CONFIRMATION_INFO_H_
#define BAT_CONFIRMATIONS_INTERNAL_CONFIRMATION_INFO_H_

#include <stdint.h>
#include <string>

#include "bat/confirmations/confirmation_type.h"
Expand All @@ -30,6 +31,8 @@ struct ConfirmationInfo {
Token payment_token;
BlindedToken blinded_payment_token;
std::string credential;
uint64_t timestamp_in_seconds;
bool created;
};

} // namespace confirmations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,9 @@ void ConfirmationsImpl::MaybeStart() {
auto start_timer_in = CalculateTokenRedemptionTimeInSeconds();
StartPayingOutRedeemedTokens(start_timer_in);

RetryFailedConfirmations();

RefillTokensIfNecessary();

StartRetryingFailedConfirmations();
}

void ConfirmationsImpl::NotifyAdsIfConfirmationsIsReady() {
Expand Down Expand Up @@ -194,6 +194,12 @@ base::Value ConfirmationsImpl::GetConfirmationsAsDictionary(
confirmation_dictionary.SetKey("credential",
base::Value(confirmation.credential));

confirmation_dictionary.SetKey("timestamp_in_seconds",
base::Value(std::to_string(confirmation.timestamp_in_seconds)));

confirmation_dictionary.SetKey("created",
base::Value(confirmation.created));

list.GetList().push_back(std::move(confirmation_dictionary));
}

Expand Down Expand Up @@ -529,6 +535,24 @@ bool ConfirmationsImpl::GetConfirmationsFromDictionary(
continue;
}

// Timestamp
auto* timestamp_in_seconds_value =
confirmation_dictionary->FindKey("timestamp_in_seconds");
if (timestamp_in_seconds_value) {
auto timestamp_in_seconds =
std::stoull(timestamp_in_seconds_value->GetString());

confirmation_info.timestamp_in_seconds = timestamp_in_seconds;
}

// Created
auto* created_value = confirmation_dictionary->FindKey("created");
if (created_value) {
confirmation_info.created = created_value->GetBool();
} else {
confirmation_info.created = true;
}

confirmations->push_back(confirmation_info);
}

Expand Down Expand Up @@ -803,24 +827,37 @@ void ConfirmationsImpl::AppendConfirmationToQueue(
confirmations_.push_back(confirmation_info);

SaveState();

BLOG(INFO) << "Added " << confirmation_info.id
<< " confirmation id with " << confirmation_info.creative_instance_id
<< " creative instance id for " << std::string(confirmation_info.type)
<< " to the confirmations queue";

if (!IsRetryingFailedConfirmations()) {
StartRetryingFailedConfirmations();
}
}

void ConfirmationsImpl::RemoveConfirmationFromQueue(
const ConfirmationInfo& confirmation_info) {
auto id = confirmation_info.id;

auto it = std::find_if(confirmations_.begin(), confirmations_.end(),
[=](const ConfirmationInfo& info) {
return (info.id == id);
return (info.id == confirmation_info.id);
});

if (it == confirmations_.end()) {
BLOG(WARNING) << "Failed to remove " << confirmation_info.id
<< " confirmation id with " << confirmation_info.creative_instance_id
<< " creative instance id for " << std::string(confirmation_info.type)
<< " from the confirmations queue";

return;
}

BLOG(INFO) << "Removed " << confirmation_info.creative_instance_id
BLOG(INFO) << "Removed " << confirmation_info.id
<< " confirmation id with " << confirmation_info.creative_instance_id
<< " creative instance id for " << std::string(confirmation_info.type)
<< " from the confirmation queue";
<< " from the confirmations queue";

confirmations_.erase(it);

Expand Down Expand Up @@ -1076,13 +1113,15 @@ void ConfirmationsImpl::UpdateNextTokenRedemptionDate() {
SaveState();
}

void ConfirmationsImpl::StartRetryingFailedConfirmations() {
auto start_timer_in =
brave_base::random::Geometric(kRetryFailedConfirmationsAfterSeconds);

StartRetryingFailedConfirmations(start_timer_in);
}

void ConfirmationsImpl::StartRetryingFailedConfirmations(
const uint64_t start_timer_in) {
if (confirmations_.size() == 0) {
BLOG(INFO) << "No failed confirmations to retry";
return;
}

StopRetryingFailedConfirmations();

confirmations_client_->SetTimer(start_timer_in,
Expand All @@ -1098,14 +1137,20 @@ void ConfirmationsImpl::StartRetryingFailedConfirmations(
<< " seconds";
}

void ConfirmationsImpl::RetryFailedConfirmations() const {
void ConfirmationsImpl::RetryFailedConfirmations() {
StopRetryingFailedConfirmations();

if (confirmations_.size() == 0) {
BLOG(INFO) << "No failed confirmations to retry";
return;
}

ConfirmationInfo confirmation_info(confirmations_.front());
RemoveConfirmationFromQueue(confirmation_info);

redeem_token_->Redeem(confirmation_info);

StartRetryingFailedConfirmations();
}

void ConfirmationsImpl::StopRetryingFailedConfirmations() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ class ConfirmationsImpl : public Confirmations {

// Confirmations
void AppendConfirmationToQueue(const ConfirmationInfo& confirmation_info);
void RemoveConfirmationFromQueue(const ConfirmationInfo& confirmation_info);
void StartRetryingFailedConfirmations(const uint64_t start_timer_in);
bool IsRetryingFailedConfirmations() const;
void StartRetryingFailedConfirmations();

// Ads rewards
void UpdateAdsRewards(const bool should_refresh) override;
Expand Down Expand Up @@ -80,8 +78,8 @@ class ConfirmationsImpl : public Confirmations {
bool OnTimer(const uint32_t timer_id) override;

// Refill tokens
void RefillTokensIfNecessary() const;
void StartRetryingToGetRefillSignedTokens(const uint64_t start_timer_in);
void RefillTokensIfNecessary() const;

// Redeem unblinded tokens
void ConfirmAd(std::unique_ptr<NotificationInfo> info) override;
Expand All @@ -108,7 +106,10 @@ class ConfirmationsImpl : public Confirmations {

// Confirmations
uint32_t retry_failed_confirmations_timer_id_;
void RetryFailedConfirmations() const;
void RemoveConfirmationFromQueue(const ConfirmationInfo& confirmation_info);
void StartRetryingFailedConfirmations(const uint64_t start_timer_in);
bool IsRetryingFailedConfirmations() const;
void RetryFailedConfirmations();
void StopRetryingFailedConfirmations();
std::vector<ConfirmationInfo> confirmations_;

Expand Down
Loading

0 comments on commit 042bb27

Please sign in to comment.