Luna Classic New Forum Join Community Mobile Error

cowballzach commented 8 months ago

Describe the bug

When signing in on mobile using a new email address for Magic to login an error message where new users cannot join the community or post threads. The message states, "Link an Address that holds TERR to participate in governance."

To Reproduce

On mobile (chrome) sign in using magic and a new email address to try and join the community https://commonwealth.im/terra-luna-classic-lunc/discussions. Joining the community and posting are both not working.

Expected behavior

New users signing in through magic should be able to join the community and post right away

Additional context

https://www.loom.com/share/92e63391a22249ca88ea5b3a58e65777 watch from 1:55 onward to save time. loom is from our partner. (inspect console from desktop) https://www.loom.com/share/2b1182b42dfd4e2a88e5dab9052d35b8 (skip to 3:45)

My attempted recreation on desktop: https://www.loom.com/share/33a36907cc0248f7813928992211296e?sid=c1cfcfa6-5942-48a7-9e7a-80a1d0ea5317

cowballzach commented 8 months ago

I was able to recreate on mobile. I tried to recreate on Desktop wasn't able too but I just attached a loom from them with the same issue on desktop.

jnaviask commented 8 months ago

cc @mhagel I wonder if the issue here with joining is because on desktop if you have multiple cosmos addresses, a modal pops up that lets you select which address to join with, but for whatever reason that modal isn't appearing on mobile? Worth looking into.

jnaviask commented 8 months ago

and cc @cowballzach the banner will be removed entirely in #6172 which is shipping in release v0.9.0 (= next up release).

cowballzach commented 8 months ago

I was able to recreate their issue on desktop. I'm on chrome using a mac. I'm using the most recent version of chrome. Here is the loom https://www.loom.com/share/bf6ef566a3694ddc9d5cd7958804a75b It won't let me join the community with a email, just keeps prompting me to sign in after I hit join community.

mhagel commented 8 months ago

Notes:

The addresses being added to the DB are 0x addresses with community_id "ethereum"
Magic login is creating a terra address, but it doesn't get saved to the DB
This is probably why joining a community fails - address mismatch
terra has a different address-derivation than standard cosmos (coinType 330 for terra vs 118 for cosmos) <-- this may or may not be relevant
Magic has a separate extension for Terra login, but we may not want to deal with that right now

Side notes:

I am focusing on the New Email Sign-in case. Existing email sign-in seems OK.
Thinking this is not a mobile-only issue, can repro on desktop. But I will test mobile once I find a solution.

mhagel commented 8 months ago

Magic Join-Community is failing for all new emails in Cosmos. I have pinpointed the trouble area. Should have fix soon.

mhagel commented 8 months ago

It looks like there is a request made to magicCosmosAPI, which is an API route that doesn't exist. It was a WIP in https://github.com/hicommonwealth/commonwealth/pull/5103 , which was closed.

Currently, we don't really have magic support for Cosmos communities when using a new email.

The problem is around Cosmos RPC URLs. They are whitelisted by Magic, so we can't use our own ChainNode records.

I propose we only use cosmos1... or osmo1... addresses for simplicity, and Magic users can join Cosmos communities with those. I have asked Magic for some whitelisted URLs.

@jnaviask

cc @raykyri

jnaviask commented 8 months ago

@mhagel asking them for whitelisted URLs works for me! Easy to convert bech32 header per community requirements. We also do have /cosmosAPI/ which may serve the same function? But not sure exactly what Magic needs.

mhagel commented 8 months ago

response from Magic:

You will actually add any RPC URL you want to use to your app's Content Security Policy. You can do this by going to your app's settings in the Magic Dashboard, then scroll down to Content Security Policy, click Edit, add your RPC URLs to the input field, click Add, and then click Done. You can add more than one URL to the input field separated by commas.

cc @jnaviask

mhagel commented 8 months ago

Whitelist issue resolved. Now seeing a problem in verifying onchain addresses:

Error signing session: Error: Magic RPC Error: [-32603] Internal error: Account 'cosmos1xkrtv0skcqkkanud973mpu5nyud44hlm2s2qvg' does not exist on chain. Send some tokens there before trying to query sequence.

If our verification process requires confirming onchain addresses, we will have a problem, because a newly created address is recognized onchain until it has tokens or a TX associated with it. Newly-created addresses do not meet this criteria.

I'm not sure what to do at this point. I've been retracing @raykyri 's steps, and it looks like the idea was to intercept the request to verify an onchain account, and provide our own verification (since the onchain check will always fail).

Should I continue down this path, or should we pause and sync up on this? @jnaviask

mhagel commented 8 months ago

Tried upgrading Magic packages and working with a proxy. Still seeing the above error, so I have opened a ticket with Magic. I will pause work on this until we get a response.

@jnaviask

mhagel commented 8 months ago

Blocked until response from Magic

mhagel commented 7 months ago

Update: Moving forward with strategy discussed with @jnaviask and @raykyri. Draft #6328 has a working implementation, but I may revert some more changes still. Should be ready for review tomorrow.

mhagel commented 7 months ago

Magic responded that they still do not support signArbitrary, so we can't sign with Magic.

I'm feeling a bit stuck. I have a PR to fix the bug, but I'm not sure how to verify the cosmos address on the back-end without a session payload and signature.

mhagel commented 7 months ago

Scope expanded to address replay issue effort, so upping points

jnaviask commented 7 months ago

Not fixed as of 0.9.4.

mhagel commented 7 months ago

Moved back to boom because this wasn't part of 0.9.4

CowMuon commented 7 months ago

JTN: This work is completed, dev/cicd tested and ready to go to QA on 2/8 in release v0.9.5

hicommonwealth / commonwealth