Handle Tally provider type #67
Conversation
src/js/sharedEth/utils.js
Outdated
| if (provider.constructor.name === 'InpageBridge') { | ||
| return 3; | ||
| } else { | ||
| return 3; |
There was a problem hiding this comment.
is this intentional to return 3 on both of these conditions? if so we can probably remove the check for InpageBridge above.
src/js/sharedEth/utils.js
Outdated
| return 3; | ||
| } | ||
| } else { | ||
| return null; |
There was a problem hiding this comment.
Here we probably want to return 3 instead of null? would not recommend to return a non number from this function lest something that uses it may crash.
There was a problem hiding this comment.
A lot of this is actually redundant code. I have removed it in e2b169b
|
Yes @torreyatcitty . I think it is because web3-eth is using wallet_requestPermissions for permission requests which Tally has not yet implemented. A |
| @@ -158,7 +158,7 @@ function subscribeToTryConnect(app, eth, globEthereum, defaultNetworkId) { | |||
|
|
|||
| // We'll try to set to the user's last chosen provider, otherwise | |||
| // defaulting to Web3. | |||
| let providerType = Number(storage('chosenProvider').get(PROVIDER_TYPE_WEB3)); | |||
| let providerType = providerTypeId(globEthereum); | |||
There was a problem hiding this comment.
Would recommend using the chosenProvider option first and then pass in the provider type id as the default value.
let providerType = Number(storage('chosenProvider').get(providerTypeId(globEthereum)));
There was a problem hiding this comment.
Awesome! Appreciate your eyes on this. You know this codebase. Change made here 416209e
|
Thanks @torreyatcitty. Could you let me know when this gets merged into the palisade frontend? |
@0xzoz can you verify on one of these deployed endpoints?
They correspond to a build from this PR: compound-finance/palisade#19 |
|
@torreyatcitty LGTM. Tested on Brave and Chrome. |
Fix issue #66 - Tally provider was not correctly being captured. Fix to properly track Tally provider.