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.

Sign up for GitHub

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

Jump to bottom

Adds unique constraint on unblinded_tokens table #5683

Merged
merged 1 commit into from
May 29, 2020
Merged

Adds unique constraint on unblinded_tokens table #5683

merged 1 commit into from
May 29, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented May 28, 2020 •
edited
Loading

Resolves brave/brave-browser#9889

Submitter Checklist:

Test Plan:

  • start from master
  • enable rewards
  • claim grant
  • close the browser
  • open db and duplicate some unblinded tokens
  • start the browser and make sure that balance went up based on duplication
  • close the browser
  • switch to this PR
  • start the browser
  • make sure that balance is now back to the original grant value

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.11.x - Nightly milestone May 28, 2020
@NejcZdovc NejcZdovc self-assigned this May 28, 2020
@NejcZdovc NejcZdovc force-pushed the unique-token branch 2 times, most recently from ff82bae to 9adc0ec Compare May 28, 2020 08:08
@NejcZdovc NejcZdovc changed the title Adds unique constrain on unblinded_tokens table Adds unique constraint on unblinded_tokens table May 28, 2020
@NejcZdovc NejcZdovc marked this pull request as ready for review May 29, 2020 05:06
@NejcZdovc NejcZdovc requested a review from a team May 29, 2020 05:06
NejcZdovc
NejcZdovc commented May 29, 2020
View reviewed changes
test/data/rewards-data/creds_tokens_prod_resp.json
"sIqey9z6yP2Zysp8jPzuABYo9cXEYB05m4WkwppzZkc=",
"usjICudNQU9OUDM47IcEmgW753Kyaa7yEo9z30FsaFM=",
"etDK97tchgvLzSFmzcGhGT34KaVa6DVUU2iNB9Urxgk="
"dgLbHfLuF+mam7rtR5KFURW3dFuUDv2Ej7vMJ3ymCjI=",
Copy link
Contributor Author

@NejcZdovc NejcZdovc May 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this file was changed so that all tokens are unique

NejcZdovc
NejcZdovc commented May 29, 2020
View reviewed changes
test/data/rewards-data/creds_tokens_resp.json
"sIqey9z6yP2Zysp8jPzuABYo9cXEYB05m4WkwppzZkc=",
"usjICudNQU9OUDM47IcEmgW753Kyaa7yEo9z30FsaFM=",
"etDK97tchgvLzSFmzcGhGT34KaVa6DVUU2iNB9Urxgk="
"dgLbHfLuF+mam7rtR5KFURW3dFuUDv2Ej7vMJ3ymCjI=",
Copy link
Contributor Author

@NejcZdovc NejcZdovc May 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

this file was changed so that all tokens are unique

@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 CI/skip-macos-x64 Do not run CI builds for macOS x64 labels May 29, 2020
emerick
emerick approved these changes May 29, 2020
View reviewed changes
Copy link
Contributor

@emerick emerick left a comment

Choose a reason for hiding this comment

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

LGTM

@NejcZdovc NejcZdovc requested a review from zenparsing May 29, 2020 16:18
zenparsing
zenparsing reviewed May 29, 2020
View reviewed changes
Copy link
Collaborator

@zenparsing zenparsing left a comment

Choose a reason for hiding this comment

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

There are db files in here for v13 and v25, but we're updating to v26 in this PR. Was that intentional?

vendor/bat-native-ledger/include/bat/ledger/public/interfaces/ledger.mojom
@@ -393,6 +393,9 @@ struct UnblindedToken {
double value;
string creds_id;
uint64 expires_at;
uint64 redeemed_at;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we set these values when reading from the database, or only during testing?

@NejcZdovc
Copy link
Contributor Author

@zenparsing yes v13 is intentional as tokens in that db were duplicated, so they were not correct. What I just did is set unique token values for them

@NejcZdovc NejcZdovc merged commit dc4219c into master May 29, 2020
@NejcZdovc NejcZdovc deleted the unique-token branch May 29, 2020 21:03
@Miyayes
Copy link

Miyayes commented Jun 6, 2020

This is part of the resolution for the "BAT not transferring to/syncing up with Uphold account" issue.

@LaurenWags LaurenWags mentioned this pull request Jun 26, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@emerick emerick emerick approved these changes

@zenparsing zenparsing zenparsing approved these changes

Assignees

@NejcZdovc NejcZdovc

Labels
CI/skip-android Do not run CI builds for Android CI/skip-ios Do not run CI builds for iOS CI/skip-macos-x64 Do not run CI builds for macOS x64 feature/rewards
Projects
None yet
Milestone
1.11.x - Release #1
Development

Successfully merging this pull request may close these issues.

Add unique to token_value in unblinded_tokens table
4 participants
@NejcZdovc @Miyayes @emerick @zenparsing