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

feat(wallet): add Ramp as an fiat onramp provider #12861

Merged
merged 1 commit into from
Apr 11, 2022
Merged

feat(wallet): add Ramp as an fiat onramp provider #12861

merged 1 commit into from
Apr 11, 2022

Conversation

onyb
Copy link
Member

@onyb onyb commented Apr 1, 2022 •
edited
Loading

Extracted out some of the TS logic in #12830 into the cpp backend. This PR only includes backend changes; Ramp is not yet added to the frontend.

cc: @kylehickinson regarding the ios changes - this PR changes the buyUrl and buyTokens API.

Resolves brave/brave-browser#21639.

Submitter Checklist:

  • I confirm that no security/privacy review is needed, or that I have requested one
  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Squashed any review feedback or "fixup" commits before merge, so that history is a record of what happened in the repo, not your PR
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)

Reviewer Checklist:

  • A security review is not needed, or a link to one is included in the PR description
  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Demo

N/A

@onyb onyb requested a review from a team as a code owner April 1, 2022 15:54
@onyb onyb self-assigned this Apr 1, 2022
@github-actions github-actions bot added the CI/storybook-url Deploy storybook and provide a unique URL for each build label Apr 1, 2022
@onyb onyb requested a review from muliswilliam April 1, 2022 15:56
@onyb onyb requested a review from kylehickinson April 1, 2022 15:57
@yrliou yrliou requested a review from supermassive April 1, 2022 16:43
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@onyb onyb force-pushed the f/wallet/multi-onramp-provider branch 2 times, most recently from 00515c1 to 912bed0 Compare April 8, 2022 02:58
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@onyb onyb force-pushed the f/wallet/multi-onramp-provider branch 3 times, most recently from df99223 to b7ac21e Compare April 8, 2022 11:12
@onyb onyb force-pushed the f/wallet/multi-onramp-provider branch from b7ac21e to bd4da02 Compare April 11, 2022 11:46
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

SergeyZhukovsky
SergeyZhukovsky approved these changes Apr 11, 2022
View reviewed changes
Copy link
Member

@SergeyZhukovsky SergeyZhukovsky left a comment

Choose a reason for hiding this comment

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

Android changes look good!

@onyb onyb force-pushed the f/wallet/multi-onramp-provider branch from bd4da02 to 5d8634a Compare April 11, 2022 14:11
@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@brave-builds
Copy link
Collaborator

A Storybook has been deployed to preview UI for the latest push

@onyb onyb added this to the 1.39.x - Nightly milestone Apr 11, 2022
@onyb
Copy link
Member Author

onyb commented Apr 11, 2022

Update since last review: added Ramp Network production host key to the query parameters for us to receive commissions.

@onyb onyb merged commit 5ea519b into master Apr 11, 2022
@onyb onyb deleted the f/wallet/multi-onramp-provider branch April 11, 2022 16:28
@onyb onyb mentioned this pull request Apr 11, 2022
Merged
25 tasks
yrliou
yrliou reviewed Apr 11, 2022
View reviewed changes
components/brave_wallet/browser/brave_wallet_constants.h
Comment on lines -412 to -413
{"braveWalletConfirmTransactionTransactionFee",
IDS_BRAVE_WALLET_CONFIRM_TRANSACTION_TRANSACTION_FEE},
Copy link
Member

Choose a reason for hiding this comment

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

@onyb This shouldn't be removed by your PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't recall why I removed it. Probably because it was not used at the time. Should've checked with a git blame. I see Douglas took care of it.

yrliou
yrliou reviewed Apr 11, 2022
View reviewed changes
components/brave_wallet/browser/brave_wallet_constants.h
Comment on lines -418 to -419
{"braveWalletConfirmTransactionAmountFee",
IDS_BRAVE_WALLET_CONFIRM_TRANSACTION_AMOUNT_FEE},
Copy link
Member

Choose a reason for hiding this comment

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

This shouldn't be removed either.

@Douglashdaniel Douglashdaniel mentioned this pull request Apr 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yrliou yrliou yrliou left review comments

@SergeyZhukovsky SergeyZhukovsky SergeyZhukovsky approved these changes

@supermassive supermassive supermassive approved these changes

@bbondy bbondy Awaiting requested review from bbondy

@muliswilliam muliswilliam Awaiting requested review from muliswilliam

@kylehickinson kylehickinson Awaiting requested review from kylehickinson

Assignees

@onyb onyb

Labels
CI/storybook-url Deploy storybook and provide a unique URL for each build
Projects
None yet
Milestone
1.39.x - Release
Development

Successfully merging this pull request may close these issues.

Buy crypto assets UI v2
6 participants
@onyb @brave-builds @supermassive @yrliou @SergeyZhukovsky @muliswilliam