Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Implement BAT into payments #10945

Closed
NejcZdovc opened this issue Sep 14, 2017 · 10 comments
Closed

Implement BAT into payments #10945

NejcZdovc opened this issue Sep 14, 2017 · 10 comments
Assignees
Labels
feature/rewards priority/P1 Blocks development or testing. Product cannot run. Must be fixed immediately, shipped next release. QA/no-qa-needed release-notes/include

Comments

@NejcZdovc
Copy link
Contributor

NejcZdovc commented Sep 14, 2017

Describe the issue you encountered:
Implement BAT into the existing ledger code.

Notes from @mrose17:

  1. you no longer need the ledger* packages
  2. instead, put this in package.json
"bat-balance": "^0.9.0",
"bat-client": "^0.9.0",
"bat-publisher": "^0.9.0",
"bignumber.js": "^4.0.4",
  1. wherever you see require('ledger-*') in app/ledger.js use the bat-* package instead
  2. get rid of anything relating to geoip, as this is no longer used
  3. when you instantiate ledgerClient = ... include version: 'v2' in the options parameter. the easiest way to do this is to add that value to clientOptions near the top of the file
  4. you can continue to call ledigerClient.getWalletAddress, which returns the BAT address, or you can call ledgerClient.getWalletAddresses which returns an array of all addresses
  5. you will probably make the most changes in the updateLedgerInfo and getPaymentInfo routines because this is where you get the current wallet state
    7a. here is an example of what getPaymentInfo gets when the callback to client.getWalletProperties invokes the callback:
{
 "paymentStamp": 0,
 "rates": {
   "USD": 0.19361672240887554
 },
 "addresses": {
   "BAT": "0x7c31560552170ce96c4a7b018e93cddc19dc61b6"
 },
 "altcurrency": "BAT",
 "probi": "32061750000000000000",
 "balance": "32.0618",
 "unconfirmed": "0.0000"
}

the balance and unconfirmed fields are now denominated in BAT rather than BTC. instead of satoshis, the probi field shows the BAT "wei", so if you want the exact number of BAT, you'll need to put that into a BigNum and then divide by 10^18. you can convert to USD by doing balance * rates.USD
you may notice that buyURLand buyURLExpires have gone away.
when we have a buy widget for BAT, then buyURL will come back.
7b. here is an example of what updateLedgerInfo sees in ledgerInfo._internal.paymentInfo:

{
 "balance": "32.0618",
 "altcurrency": "BAT",
 "probi": "32061750000000000000",
 "address": :0x7c31560552170ce96c4a7b018e93cddc19dc61b6",
 "addresses": {
   "BAT": "0x7c31560552170ce96c4a7b018e93cddc19dc61b6"
 },
 "amount": 5,
 "currency": "USD",
 "BAT": "25.7854"
}

again, no btc and satoshis fields, but BAT and probi fields.
8. finally, there is one bit of information hiding that i missed in the ledger-client package: in getStateInfo instead of doing
ledgerInfo.passphrase = state.properties.wallet.keychains.passphrase
do
ledgerInfo.passphrase = ledgerClient.prototype.getWalletPassphrase(state)

@NejcZdovc NejcZdovc added this to the 0.19.x (Beta Channel) milestone Sep 14, 2017
@NejcZdovc NejcZdovc self-assigned this Sep 14, 2017
@NejcZdovc NejcZdovc changed the title Implement BAT into the ledger Implement BAT into payments Sep 14, 2017
@NejcZdovc
Copy link
Contributor Author

NejcZdovc commented Sep 14, 2017

@mrose17 questions 😃

  1. I noticed that we renamed ledger into bat. Should we rename .json files as well?

  2. I notice that we don't actually use satoshis in the current code, should we implement probi?

  3. should we already see this amount when you create a new wallet?
    image

  4. What will happen with this section?
    image

@NejcZdovc NejcZdovc added the priority/P1 Blocks development or testing. Product cannot run. Must be fixed immediately, shipped next release. label Sep 14, 2017
NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Sep 14, 2017
@mrose17
Copy link
Member

mrose17 commented Sep 14, 2017

you have questions, we have answers!

  1. I noticed that we renamed ledger into bat. Should we rename .json files as well? you can if you wish, but i wouldn't. we went from "btc" to "bat", but it's still "the ledger"

  2. I notice that we don't actually use satoshis in the current code, should we implement probi? i wouldn't bother doing that until we need it. you can probably just do 1,$s/stoshis/probi/g in the code and forget about it.

  3. should we already see this amount when you create a new wallet? yes, because you are talking to a "mock" server that pre-loads some bat into your wallet. when we move it to the uphold server (probably today or tomorrow), it will start at zero.

  4. What will happen with this section? 1,$s/Bitcoin/Ethereum/g and 1,$s/BTC/ETH/g with one exception the final text should read Use any ETH wallet that can transfer BAT to your Brave wallet

ps: @NejcZdovc - thanks for finding and fixing the bugs in the 1ledger-client repo. i am pushing a new version in about 10m

@NejcZdovc
Copy link
Contributor Author

thank you for answers, I have few more

  1. what happens with this section
    image
  2. are we still limiting section mentioned above by countries?
  3. add found uphold widget is still in WIP phase right?

@mrose17
Copy link
Member

mrose17 commented Sep 14, 2017

and i have a few more answers

  1. what happens with this section: It becomes Buy BAT at our recommenced source and says "Uphold" and takes them to https://uphold.com

  2. re we still limiting section mentioned above by countries? no, there is no more "geoip" in use . ... it is possible in the future we may return "geoip" to find other places selling BAT to consumers, but not for the immediate future.

  3. add found uphold widget is still in WIP phase right? i'm not sure what "found uphjold widget" refers to!

@NejcZdovc
Copy link
Contributor Author

add found uphold widget is still in WIP phase right
I think that you answered it with 1 answer

@mrose17
Copy link
Member

mrose17 commented Sep 14, 2017

got it, thanks!

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Sep 14, 2017
@bradleyrichter
Copy link
Contributor

adding crypto svg icons for buttons

crypto_icons.zip

NejcZdovc added a commit to NejcZdovc/browser-laptop that referenced this issue Oct 3, 2017
NejcZdovc added a commit that referenced this issue Oct 5, 2017
NejcZdovc added a commit that referenced this issue Oct 5, 2017
NejcZdovc added a commit that referenced this issue Oct 6, 2017
NejcZdovc added a commit that referenced this issue Oct 6, 2017
NejcZdovc added a commit that referenced this issue Oct 6, 2017
Resolves #9740
Resolves #10945
Resolves #11251 
Resolves #11264
Resolves #11285
Resolves #11289
Resolves #11292
Resolves #11293
@luixxiul
Copy link
Contributor

luixxiul commented Oct 7, 2017

no-qa-needed (as it had been done already) ?

@luixxiul luixxiul added the needs-info Another team member needs information from the PR/issue opener. label Oct 7, 2017
@kjozwiak
Copy link
Member

kjozwiak commented Oct 11, 2017

@NejcZdovc, as per @luixxiul suggestion, can we mark this as no-qa-needed as the BAT implementation is being tested through the test plan that QA has created including several other issues that have already been created.

Is there anything that needs to be addressed by QA in this particular issue?

@NejcZdovc
Copy link
Contributor Author

no I don't think so. BAT flow become main stream now with ledger manual test plan

@luixxiul luixxiul removed QA/test-plan-required needs-info Another team member needs information from the PR/issue opener. labels Oct 11, 2017
syuan100 pushed a commit to syuan100/browser-laptop that referenced this issue Nov 9, 2017
Resolves brave#9740
Resolves brave#10945
Resolves brave#11251 
Resolves brave#11264
Resolves brave#11285
Resolves brave#11289
Resolves brave#11292
Resolves brave#11293
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature/rewards priority/P1 Blocks development or testing. Product cannot run. Must be fixed immediately, shipped next release. QA/no-qa-needed release-notes/include
Projects
None yet
Development

No branches or pull requests

5 participants