Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upBitgo Integration #136
Bitgo Integration #136
Conversation
|
minor nit for next time: instead of copy-pasting files like 581cb84 from another branch, it's preferable to cherry-pick the commit and then move it in a new commit so it preserves the commit history/authorship |
| if (window.chrome && window.chrome.braveWallet) { | ||
| window.chrome.braveWallet.getBitGoSeed(key, resolve) | ||
| } else { | ||
| // polyfill |
This comment has been minimized.
This comment has been minimized.
|
@diracdeltas authorship restored - 5e9287b |
| return resp | ||
| } | ||
|
|
||
| async sendTx (id, amount, destinationAddress) { |
This comment has been minimized.
This comment has been minimized.
diracdeltas
May 1, 2020
Member
just an FYI, amount is the amount in base value (ex: satoshis) as a string not a number
| this.proxyOrigin = opts.isProduction | ||
| ? 'https://bitgo-proxy.brave.com/' | ||
| : 'http://localhost:3000/' | ||
| this.store = new ObservableStore(initState) |
This comment has been minimized.
This comment has been minimized.
diracdeltas
May 1, 2020
Member
i think for this to work correctly between restarts, there needs to be something added to persist values in the store to disk and read it into initState when the controller is subsequently constructed
This comment has been minimized.
This comment has been minimized.
diracdeltas
May 1, 2020
Member
unfortunately since the server can't tell which wallets were created by which users, i don't think the state for each crypto wallets user can be fetched from the server on init
|
|
||
| for (let coin in supportedCoins) { | ||
| if (!bitGoCreatedWallets.includes(coin)) { | ||
| this.bitGoCreateableAssets[coin] = supportedCoins[coin] |
This comment has been minimized.
This comment has been minimized.
diracdeltas
May 1, 2020
Member
if i understand correctly, this is only showing assets that aren't already created as bitgo wallets as createable? FWIW bitgo supports having multiple wallets for the same asset for the same user, but i'm not sure if that's something we want to support in the UI.
This comment has been minimized.
This comment has been minimized.
ryanml
May 1, 2020
Author
Member
That's correct yes, as far as I saw we didn't support naming specific wallets with our proxy. I think this is fine for now, maybe in the future we can have ux around managing multiple wallets of one type cc: @jamesmudgett
This comment has been minimized.
This comment has been minimized.
diracdeltas
May 1, 2020
Member
right, there is a wallet label but this is a random phrase generated by the proxy server. there's no way currently for users to change the label.
| <h3> | ||
| {'Select all crypto assets you would like to interact with.'} | ||
| </h3> | ||
| <p>{'A new wallet will be generated for each coin checked. Two of three private keys are stored on BitGo. The third key will be stored securely on Brave.'}</p> |
This comment has been minimized.
This comment has been minimized.
diracdeltas
May 1, 2020
Member
this message should be inverted. (2/3 keys are stored on brave, 1 is stored on bitgo)
may be worth mentioning that any transaction requires 2/3 keys
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| ) | ||
| } | ||
|
|
||
| renderTransactions = () => { |
This comment has been minimized.
This comment has been minimized.
ryanml commentedMay 1, 2020
No description provided.