add more alt currency support #215
Conversation
bat-utils/lib/runtime-wallet.js
Outdated
|
|
||
| return this.uphold.createCardAddress(cardId, network) | ||
| }, | ||
| addAddress: async function (info, altcoin) { |
There was a problem hiding this comment.
can we consolidate addAddress and createAddress?
it seems like the only difference is addAddress sets info.addresses[altcoin] - maybe we could remove from create and just use the consolidated function
There was a problem hiding this comment.
short answer: no, because addAddress is called both internally and from the upper-layer...
There was a problem hiding this comment.
sorry I don't understand. if we put the functionality from createAddress in addAddress couldn't we continue to call it from internally and in the upper layer?
There was a problem hiding this comment.
the upper layer calls both addAddress and create. both of these call createAddress, which wants a cardId as a parameter. create could pass the cardId; however addAddress being called by the upper layer is given an info object, in which info.providerId is the cardId. with the exception of create which returns an info object, all calls from the upper layer provider an info object. having the upper layer call address with a cardId rather than an info object, would violate the encapsulation being used.
the current code takes the maximal amount of code needed for both addAddress and create and puts it in createAddress, thereby allow maximal code reuse...
There was a problem hiding this comment.
it seems like we have info in both places (being that create is the originator of the info - being it's return value)
|
@mrose17 can you add integration tests for the expected workflows? e.g. existing wallet needs to create uphold addresses for missing alt-currencies, new wallet creates all, etc? |
ready: BCH, BTG, DASH waiting on uphold: XRP
evq
force-pushed
the
add-more-alturrencies
branch
from
a0d48df
to
fcc5765
Compare
|
I cleaned up the commit history by cherry-picking onto master |
|
XRP seems to be available now too: https://www.coindesk.com/payments-platform-uphold-completes-xrp-ledger-integration/. |
bat-utils/lib/runtime-wallet.js is not updated from master
|
closing as stale |
ready: BCH, BTG, DASH, XRP
fixes #114