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 contribution with unblinded tokens #4317

Merged
merged 1 commit into from
Jan 8, 2020
Merged

Fixes contribution with unblinded tokens #4317

merged 1 commit into from
Jan 8, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Jan 7, 2020

Resolves brave/brave-browser#7608

Submitter Checklist:

Test Plan:

test plan defined in brave/brave-browser#7608

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.

@NejcZdovc NejcZdovc added this to the 1.5.x - Nightly milestone Jan 7, 2020
@NejcZdovc NejcZdovc self-assigned this Jan 7, 2020
@@ -157,7 +157,7 @@ void Unblinded::OnUnblindedTokens(
continue;
}

if (item->value + current_amount > reconcile.fee) {
if (current_amount >= reconcile.fee) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the fix as we decided to go over if needed

@@ -139,7 +148,8 @@ ledger::UnblindedTokenList DatabaseUnblindedToken::GetAllRecords(
info->public_key = statement.ColumnString(2);
info->value = statement.ColumnDouble(3);
info->promotion_id = statement.ColumnString(4);
info->expires_at = statement.ColumnInt64(5);
info->expires_at =
Copy link
Contributor Author

@NejcZdovc NejcZdovc Jan 7, 2020

Choose a reason for hiding this comment

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

this fixes ads grants as some may expired, but we don't want to exclude them

@NejcZdovc NejcZdovc force-pushed the tip-vg branch 2 times, most recently from 53a8b73 to f81c80d Compare January 8, 2020 16:00
return false;
}

const std::string query = base::StringPrintf(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fixes users in bad state

@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Jan 8, 2020

macos failed on audit-network and dist, re-running it now. Everything else passed

@NejcZdovc NejcZdovc added CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-linux labels Jan 8, 2020
@ryanml ryanml self-requested a review January 8, 2020 20:36
@gdregalo gdregalo self-requested a review January 8, 2020 20:38
Copy link
Contributor

@gdregalo gdregalo left a comment

Choose a reason for hiding this comment

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

looks good from C++ POV

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.

lgtm

@kjozwiak
Copy link
Member

kjozwiak commented Jan 9, 2020

Reproduced the original issue using the following build:

Brave 1.5.10 Chromium: 79.0.3945.88 (Official Build) nightly (64-bit)
Revision c2a58a36b9411c80829b4b154bfcab97e581f1f3-refs/branch-heads/3945@{#954}
OS macOS Version 10.15.2 (Build 19C57)

Case #1:

  • launch Brave using --enable-logging=stderr --rewards=staging=true
  • restore a wallet that currently has an ad grant (document in the shared GDrive)
  • claim the UGP grant
  • send 5 BAT to https://duckduckgo.com/ and ensure it goes through without any issues
  • send 5 BAT to https://laurenwags.github.io/ and ensure it goes through without any issues
  • check brave://rewards and ensure that everything is being displayed as expected
  • ensured that the correct amount of BAT has been deducted from Brave

Case #2:

  • install 1.5.10 CR: 79.0.3945.88 and launch Brave using --enable-logging=stderr --rewards=staging=true
  • restore a wallet that currently has an ad grant (document in the shared GDrive)
  • claim the UGP grant
  • send 5 BAT to https://duckduckgo.com/ (nothing should go through)
  • send 5 BAT to https://laurenwags.github.io/ (nothing should go through)
  • install 1.5.22 CR: 79.0.3945.117 or the latest Nightly (use the same profile as above)
  • send 5 BAT to https://duckduckgo.com/ and ensure it goes through without any issues
  • send 5 BAT to https://laurenwags.github.io/ and ensure it goes through without any issues
  • ensured that the correct amount of BAT has been deducted from Brave

@kjozwiak
Copy link
Member

kjozwiak commented Jan 9, 2020

@LaurenWags noticed that there was a discrepancy in the amount of BAT that was being deducted from the account when tipping. @NejcZdovc mentioned that the server side fix hasn't been pushed. Waiting on @evq to fix the above server side. Once the fix has been pushed, @LaurenWags and myself will go through the cases outlined via #4317 (comment).

If things are looking good, we'll continue with the uplift process for both #4341 & #4342.

@kjozwiak
Copy link
Member

As per earlier meetings, the a-c cases will be checked via brave/brave-browser#7709.

@kjozwiak
Copy link
Member

kjozwiak commented Jan 13, 2020

Verification PASSED on macOS 10.15.2 x64 using the following build:

Brave 1.5.27 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)

Screen Shot 2020-01-12 at 11 07 04 PM

Screen Shot 2020-01-13 at 1 31 33 PM

Verification PASSED on macOS 10.14.6 x64 using the following build:

Brave 1.5.27 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 Case 1 from Fixes contribution with unblinded tokens #4317 (comment)
    • launch Brave using --enable-logging=stderr --rewards=staging=true
    • restore a wallet that currently has an ad grant (document in the shared GDrive) and claim ad grant
    • claim the UGP grant
    • send 1 BAT to https://duckduckgo.com/ and ensure it goes through without any issues
    • send 5 BAT to https://laurenwags.github.io/ and ensure it goes through without any issues
    • check brave://rewards and ensure that everything is being displayed as expected
    • ensured that the correct amount of BAT has been deducted from Brave

Case 1

  • Verified Case 2 from Fixes contribution with unblinded tokens #4317 (comment)
    • install 1.5.10 CR: 79.0.3945.88 and launch Brave using --enable-logging=stderr --rewards=staging=true
    • restore a wallet that currently has an ad grant (document in the shared GDrive) and claim Ad grant
    • claim the UGP grant
    • send 1 BAT to https://duckduckgo.com/ and verify it goes thru/displays in UI as expected
    • install 1.5.27 CR: 79.0.3945.117 or the latest Nightly (use the same profile as above)
    • send 1 BAT to https://duckduckgo.com/ and ensure it goes through without any issues
    • send 5 BAT to https://laurenwags.github.io/ and ensure it goes through without any issues
    • ensured that the correct amount of BAT has been deducted from Brave

Screen Shot 2020-01-13 at 2 01 34 PM

Other checks performed:

  • Verified able to tip using 1.5.27 and an Ad grant only (no UGP/VG BAT)
  • Verified able to tip using 1.5.27 and a KYC'd user wallet to a KYC'd publisher (no UGP/VG BAT)

Verification PASSED on Win 10 x64 using the following build:

Brave 1.5.30 Chromium: 79.0.3945.117 (Official Build) nightly (64-bit)
Revision 04f0a055010adab4484f7497fbfdbf312c307f1d-refs/branch-heads/3945@{#1019}
OS Windows 10 OS Version 1909 (Build 18363.535)

Annotation 2020-01-13 191915

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS feature/rewards
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tips not going thru when tokens are not standardized to 0.25 value
4 participants