Remove terra.js dependency (if possible)

hicommonwealth / commonwealth

A platform for decentralized communities

https://commonwealth.im

GNU General Public License v3.0

67 stars 44 forks source link

Remove terra.js dependency (if possible) #6332

Open mhagel opened 8 months ago

mhagel commented 8 months ago

Description

terra.js is only referenced in our WalletConnect Terra integration. If this can be removed, it will be a win for our bundle size.

Project Owner

Engineering Requirements

Acceptance Criteria

WalletConnect still works with /terra

Additional context

follow-up to #6316 , which fixed the Station V3 web wallet integration.
A similar update for Station Mobile is coming soon.

mhagel commented 8 months ago

cc @jnaviask

dillchen commented 8 months ago

this would be great and would save on quite a bit of bundle size, probably 50 -100ms at initial load

mhagel commented 8 months ago

Note that Terra Station mobile is not updated to the newer version of WalletConnect and thus cannot connect to most apps at this time. An update (v3) should be coming out soon and once that hits mobile most would be compatible once again.

from Terra discord 1/7/24

This may not be relevant, because I have found at least one Terra dapp that connects via WalletConnect.

Our integration is currently broken, though, so I need to pinpoint when it broke to see if is because of WalletConnect, our recent Station V3 change, or something else.

mhagel commented 8 months ago

Went back to commit 9eac2818f5ac75e6a0a7836b6fa20f15e7cb1365 (before #6316) and confirmed that walletconnect-terra was not working then.

mhagel commented 8 months ago

Went back to commit 9eac2818f5ac75e6a0a7836b6fa20f15e7cb1365 (before https://github.com/hicommonwealth/commonwealth/issues/6316) and confirmed that walletconnect-terra was not working then.

Was wrong about this. There is a small bug from 6316, which I will fix here.

mhagel commented 8 months ago

It looks like we can replace the last use of terra.js with the same cosmjs method we use for keplr-like wallets. The only caveat is we will have to use a Terra RPC node instead of the LCD REST node they presumably gave us. Sound good @jnaviask ?

mhagel commented 8 months ago

Recommending we move this to backlog or unplanned. It is worth revisiting when Station Mobile has it next major upgrade, which could be within weeks.

CowMuon commented 7 months ago

Main win here is load time, but also if ~75 merits 5+ points of work at this juncture, with everything else that's needed.