Support connection from smart wallets #416
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). the-game – ./packages/web🔍 Inspect: https://vercel.com/metafam/the-game/BVRoiR6YbDW7XFJC22tyduwxHt6V [Deployment for d820fc7 failed] testing-the-game – ./packages/web🔍 Inspect: https://vercel.com/metafam/testing-the-game/4tLeEmU1nvaU7bxEN73wM83e4rHb [Deployment for d820fc7 failed] |
pacwoodson
added
backend
packages/utils/src/ethereumHelper.ts
Outdated
|
|
||
| return returnValue; | ||
| } | ||
| throw new Error('unsupported wallet type'); |
There was a problem hiding this comment.
This code is unreachable.
Instead do you think we should wrap the previous logic in a try catch and throw 'unsupported wallet type' if something throws inside?
I say this because (as you have mentioned) other smart wallets will have non-0x code but may not always support isValidSignature.
There was a problem hiding this comment.
You're right, this condition is here because I started to use the argent-sdk which returns a specific type for argent.
I'll correct this. But I was more asking for help on the bigger picture, see the PR main post ;)
There was a problem hiding this comment.
Yes I've shared my thoughts below :)
| const hexArray = utils.arrayify(hexMessage); | ||
| const hashMessage = utils.hashMessage(hexArray); | ||
|
|
||
| const contract = new Contract(address, smartWalletABI, provider); |
There was a problem hiding this comment.
This also throws sometimes if the contract doesn't support the ABI, and should be inside the try-catch
There was a problem hiding this comment.
Are you sure about this ? It is a constructor, so it's sync, so I don't think it can go check the contract code and throw here
There was a problem hiding this comment.
True. I've had some issues with ethersJS sometimes though. But never mind. Lets go ahead with this
There was a problem hiding this comment.
Just a small change, otherwise looks good!
In general, its great that we are supporting smart-wallets! 🤓️
Regarding supporting other wallets: It will be tedious to support every kind of wallet out there. We can strive to support standards only such as eip-1271 in this case.
Regarding the issue of speed: The speed issue appears only in the case of smart-wallets, and imo this is a trade-off the user is bearing vs the added security that they gain by the use of smart-contract wallets.
If you feel it's not a problem to have long query time, lets go for it, but we may have to find some trick later ... |
Correct me if I'm wrong but this long query time is only in case of smart-wallets, which were rendered useless earlier? We can create a separate issue to research this and find a better solution if-need-be in the future. Along with that maybe we could do some user testing or a survey to find out how many of our users really use smart-wallets and how many of them have an issue with the delay. |
Yes the delay will only happen for users logged in with smart wallet, but will apply to every request they will do. Lets go for this and see how we can do better later |
* Support connection from smart wallets * Fix unreachable code
* Support connection from smart wallets * Fix unreachable code
Trying to fix #413
As this does change the backend, preview will not be working here.
Smart wallets use a different mechanism when signing messages. See https://docs.argent.xyz/wallet-connect-and-argent
This implementation works with Argent, but may not work with other wallets which doesn't implement the EIP in the same fashion.
The biggest problem here is that the authentication hook, which is called everytime there is a request on hasura, has to query on-chain information (contract code + verify signature), which will slow down a lot execution times. Auth hooks must be super-fast.
The mechanism we have setup previously only required to verify a signature off-chain, which is not super cheap but neither super expensive. Now we're going through async requests on infura nodes ... But has the advantage to be trust-less, with the user self-signing tokens.
We have to lookup for better solutions to be able to authenticate in a DID (decentralized-identity) way, without lags on requests. Maybe IDX will provide something better for that. At the moment I don't see any faster way of doing it apart from the traditional centralized JWS mechanism.