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 transaction amount #6757

Merged
merged 1 commit into from
Oct 2, 2020
Merged

Fixes transaction amount #6757

merged 1 commit into from
Oct 2, 2020

Conversation

NejcZdovc
Copy link
Contributor

@NejcZdovc NejcZdovc commented Oct 2, 2020

Resolves brave/brave-browser#11818

Submitter Checklist:

Test Plan:

defined in the issue

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.17.x - Nightly milestone Oct 2, 2020
@NejcZdovc NejcZdovc requested a review from a team October 2, 2020 07:59
@NejcZdovc NejcZdovc self-assigned this Oct 2, 2020
@NejcZdovc NejcZdovc requested review from emerick and zenparsing and removed request for a team October 2, 2020 07:59
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 merged commit 2be6c79 into master Oct 2, 2020
@NejcZdovc NejcZdovc deleted the uphold-transaction-amount branch October 2, 2020 16:07
denomination.SetDoubleKey("amount", transaction.amount);
denomination.SetStringKey(
"amount",
base::StringPrintf("%f", transaction.amount));
Copy link
Member

Choose a reason for hiding this comment

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

Just checking if any old clients are sending this in the wrong format? Maybe need to make sure iOS gets this one if they recently updated to 1.15.x? cc @kylehickinson just in case.

Copy link
Member

Choose a reason for hiding this comment

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

Snippet from the issue report:

Important: This is regression in 1.15.x. Not reproducible on 1.14.x

@btlechowski
Copy link

LGTM

Verification passed on

Brave 1.17.13 Chromium: 86.0.4240.68 (Official Build) nightly (64-bit)
Revision ad72ee9aa8e15ed300df1238e76c7a8f4d686f97-refs/branch-heads/4240@{#1097}
OS Ubuntu 18.04 LTS

No more errors in the logs.
Verified both transactions are successful:

For 0.05 BAT to Brave

[10381:10381:1005/184824.147968:VERBOSE5:ledger_impl.cc(133)] 
[ REQUEST ]
> URL: https://api-sandbox.uphold.com/v0/me/cards/fbd1546c-6706-4267-9e6c-3eda44945283/transactions
> Method: UrlMethod::POST
> Content: {"denomination":{"amount":"0.050000","currency":"BAT"},"destination":"1b2b466f-5c15-49bf-995e-c91777d3da93","message":"5% transaction fee collected by Brave Software International"}

[10381:10381:1005/184824.974332:VERBOSE6:logging_util.cc(136)] 
[ RESPONSE - OnRequest ]
> Url: https://api-sandbox.uphold.com/v0/me/cards/fbd1546c-6706-4267-9e6c-3eda44945283/transactions/
> Result: Success
> HTTP Code: 200
> Body: {"application":{"clientId":"4c","name":"Brave Browser"},"createdAt":"2020-10-05T16:48:23.135Z","denomination":{"pair":"BATBAT","rate":"1.00","amount":"0.05"

0.95 Bat to the creator:

[10381:10381:1005/184715.262668:VERBOSE5:ledger_impl.cc(133)] 
[ REQUEST ]
> URL: https://api-sandbox.uphold.com/v0/me/cards/fbd1546c-6706-4267-9e6c-3eda44945283/transactions
> Method: UrlMethod::POST
> Content: {"denomination":{"amount":"0.950000","currency":"BAT"},"destination":"0fd15856-ada1-45a3-9ec7-74ac4db76eec","message":""}

[10381:10381:1005/184716.146313:VERBOSE6:logging_util.cc(136)] 
[ RESPONSE - OnRequest ]
> Url: https://api-sandbox.uphold.com/v0/me/cards/fbd1546c-6706-4267-9e6c-3eda44945283/transactions/
> Result: Success
> HTTP Code: 200
> Body: {"application":{"clientId":"4c2b665ca060d912fec5c735c734859a06118cc8","name":"Brave Browser"},"createdAt":"2020-10-05T16:47:14.262Z","denomination":{"pair":"BATBAT","rate":"1.00","amount":"0.95","currency":"BAT"}

Verified the transactions are listed on uphold
image

Also verified by tipping 10 BAT to a youtube creator

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.

Validation error in logs for Brave transaction fee (5%)
5 participants