Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes unable to get ad tokens if tokens are not ready #4334

Merged
merged 1 commit into from
Jan 9, 2020
Merged

Conversation

tmancey
Copy link
Collaborator

@tmancey tmancey commented Jan 8, 2020

Fixes brave/brave-browser#7666

Submitter Checklist:

Test Plan:

Confirm ad tokens are successfully fetched in the console log and ads are viewed

Reviewer Checklist:

  • New files have MPL-2.0 license header.
  • Request a security/privacy review as needed.
  • Adequate test coverage exists to prevent regressions
  • Verify test plan is specified in PR before merging to source

After-merge Checklist:

  • The associated issue milestone is set to the smallest version that the
    changes has landed on.
  • All relevant documentation has been updated.

Copy link
Contributor

@ryanml ryanml left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++

@tmancey
Copy link
Collaborator Author

tmancey commented Jan 9, 2020

Unrelated CI issues

@tmancey tmancey merged commit 4f181d7 into master Jan 9, 2020
@tmancey tmancey deleted the issues/7666 branch January 9, 2020 16:34
@kjozwiak
Copy link
Member

kjozwiak commented Jan 14, 2020

Verification PASSED on macOS 10.15.2 x64 using the following build:

Brave 1.5.32 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS macOS Version 10.15.2 (Build 19C57)

Ensured that 50 unblided tokens were added as per the following:

[45014:775:0114/111744.040602:INFO:confirmations_impl.cc(690)] Saving confirmations state
[45014:775:0114/111744.041034:INFO:refill_tokens.cc(300)] Added 50 unblinded tokens, you now have 50 unblinded tokens
[45014:775:0114/111744.041084:INFO:confirmations_impl.cc(690)] Saving confirmations state
[45014:775:0114/111744.041456:INFO:refill_tokens.cc(325)] Successfully refilled tokens
[45014:775:0114/111744.053264:INFO:confirmations_impl.cc(708)] Successfully saved confirmations state
[45014:775:0114/111744.065155:INFO:confirmations_impl.cc(708)] Successfully saved confirmations state

Ensured that Refill is correctly working as per:

[45014:775:0114/111903.092161:INFO:refill_tokens.cc(50)] Refill
[45014:775:0114/111903.092172:INFO:refill_tokens.cc(75)] RequestSignedTokens
[45014:775:0114/111903.092179:INFO:refill_tokens.cc(78)] No need to refill tokens as we already have 48 unblinded tokens which is above the minimum threshold of 20

Triggered an ad on brave.com without any issues:

[45015:775:0114/111859.314850:INFO:ads_impl.cc(1150)] Found 7 eligible ads
[45015:775:0114/111859.315227:INFO:ads_impl.cc(1398)] Ad notification shown:
  id: bd0d9805-7e07-427f-8e3f-80cf41559288
  campaign_id: dac978e2-05c3-4d2e-ab74-e7ee364f740b
  advertiser: Express VPN
  category: technology & computing
  text: Protect your online Identity! Get 3 months free now!
  url: https://motille.com/path/lp.php?trvid=10008&trvx=518abffe&var1=21_technology
  uuid: 208a3150-5b1b-49d9-be0e-e55d57d0ec39
[45015:775:0114/111859.315448:INFO:notifications.cc(262)] Saving notifications state

Once an ad was viewed, ensured that an unblinded payment token was created with a redemption value:

[45263:775:0114/114516.159577:INFO:confirmations_impl.cc(690)] Saving confirmations state
[45263:775:0114/114516.160086:INFO:redeem_token.cc(407)] Added 1 unblinded payment token with an estimated redemption value of 0.05 BAT, you now have 1 unblinded payment tokens
[45263:775:0114/114516.160119:INFO:confirmations_impl.cc(690)] Saving confirmations state

Verification PASSED on macOS 10.14.6 x64 using the following build:

Brave 1.5.29 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS macOS Version 10.14.6 (Build 18G103)
  • Verified "upgrade" case: reproduced issue using 1.3.86 (Beta), renamed Beta profile to Nightly and verified "upgrade" case works as expected - tokens were added, able to view an ad notification, and refill attempt triggered:

Tokens added:

[25335:775:0114/115112.140771:INFO:confirmations_impl.cc(690)] Saving confirmations state
[25335:775:0114/115112.141215:INFO:refill_tokens.cc(300)] Added 50 unblinded tokens, you now have 50 unblinded tokens
[25335:775:0114/115112.141263:INFO:confirmations_impl.cc(690)] Saving confirmations state
[25335:775:0114/115112.141693:INFO:refill_tokens.cc(325)] Successfully refilled tokens

Ad Notification viewed:

[25336:775:0114/115216.519689:INFO:ads_impl.cc(1150)] Found 7 eligible ads
[25336:775:0114/115216.520022:INFO:ads_impl.cc(1398)] Ad notification shown:
  id: 58d7fa44-d1d3-4664-8b74-c79df66df618
  campaign_id: eff08b98-29ae-4648-8a14-41fe50d7f96f
  advertiser: Nord VPN
  category: technology & computing
  text: Protect your online Identity! Get 70% off now!
  url: https://motille.com/path/lp.php?trvid=10017&trvx=1d35ebd8&var1=21_technology
  uuid: c12102b1-11a8-43a0-b645-2998ddf0ea25
[25336:775:0114/115216.520217:INFO:notifications.cc(262)] Saving notifications state

Refill attempted (not needed):

[25335:775:0114/115216.522749:INFO:refill_tokens.cc(50)] Refill
[25335:775:0114/115216.522763:INFO:refill_tokens.cc(75)] RequestSignedTokens
[25335:775:0114/115216.522772:INFO:refill_tokens.cc(78)] No need to refill tokens as we already have 49 unblinded tokens which is above the minimum threshold of 20
[25336:775:0114/115216.526273:INFO:client.cc(598)] Successfully saved client state
[25335:775:0114/115216.547341:INFO:confirmations_impl.cc(708)] Successfully saved confirmations state
  • Verified with a clean profile as well, tokens were added, able to view an ad notification, and refill attempt triggered:

Tokens added:

[25401:775:0114/120550.203021:INFO:confirmations_impl.cc(690)] Saving confirmations state
[25401:775:0114/120550.203330:INFO:refill_tokens.cc(300)] Added 50 unblinded tokens, you now have 50 unblinded tokens
[25401:775:0114/120550.203380:INFO:confirmations_impl.cc(690)] Saving confirmations state
[25401:775:0114/120550.203675:INFO:refill_tokens.cc(325)] Successfully refilled tokens

Ad notification viewed:

[25402:775:0114/121401.256400:INFO:ads_impl.cc(1150)] Found 7 eligible ads
[25402:775:0114/121401.256798:INFO:ads_impl.cc(1398)] Ad notification shown:
  id: b4836544-03b2-4697-9e85-211297b75ccc
  campaign_id: eff08b98-29ae-4648-8a14-41fe50d7f96f
  advertiser: Nord VPN
  category: technology & computing
  text: You’re Not Safe Without a VPN. Protect all Your Devices
  url: https://motille.com/path/lp.php?trvid=10017&trvx=1d35ebd8&var1=24_technology
  uuid: 78e18e82-cca1-46b1-b41e-787c3e884df6
[25402:775:0114/121401.257056:INFO:notifications.cc(262)] Saving notifications state

Refill attempted (not needed):

[25401:775:0114/121401.259886:INFO:refill_tokens.cc(50)] Refill
[25401:775:0114/121401.259898:INFO:refill_tokens.cc(75)] RequestSignedTokens
[25401:775:0114/121401.259907:INFO:refill_tokens.cc(78)] No need to refill tokens as we already have 49 unblinded tokens which is above the minimum threshold of 20
[25402:775:0114/121401.263152:INFO:client.cc(598)] Successfully saved client state

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to get ad tokens if tokens are not ready with 1.3.x, 1.4.x and 1.5.x
4 participants