celo-org / developer-tooling

🛠️ SDKs and CLI for interacting with Celo
Apache License 2.0
9 stars 4 forks source link

Social-connect Developer Tooling Codapendency #220

Open aaronmgdr opened 3 months ago

aaronmgdr commented 3 months ago

The following is a thread about the remaining codependencies between the developer tooling repo. and the social -connect repo, the impacts felt so far, speculated impacts, potential solutions and issues with the solutions.

why is it like this?

The key part is that the @celo/identity package (from social-connect) has contractkit as a dependency, AND is itself a dependency of @celo/celocli this means that via the identity package the celocli will often have multiple versions of contractkit ( and any of its subdependencies)

bad so far

Until now this was at most an annoyance solved by ignoring some ts errors. However there is a specific bug that keeps popping up related to multiple cross-fetch versions (only with patch difference).

why it gets worse

furthermore with the upgrade from web3 v1 to v4 it is almost guaranteed there will be incompatibilities that cannot be ignored since a web3 instance must be passed in from celocli to function in identity package.

half solutions

remove identity as dependency of cli

One possibility which we have discussed is breaking some functionality of celocli into plugins. a social-connect plugin could live in social-connect repo and contain the commands under the current celocli identity namespace

why it isnt that simple

However There is also a command in the account namespace offchain-write which also imports from @celo/identity

remove contractkit as dependency of identity

Most of what identity uses from contractkit is in the@celo/contractkit/lib/identity/claims folder. As this is not really core we have discussed moving it out to its own celo/claims package. https://github.com/celo-org/developer-tooling/issues/15

however most is not all. WalletKeySigner which is one possible shape type of AuthSigner , takes a contractKit instance. so far it seems this is only used to call the sign function. so

export interface WalletKeySigner {
  authenticationMethod: AuthenticationMethod.WALLET_KEY
  contractKit: ContractKit
}

might become

export interface WalletKeySigner {
  authenticationMethod: AuthenticationMethod.WALLET_KEY
  // A method that will sign the data 
  sign: (dataToSign: string, address: string) => Promise<string>
}

this does change the external API though so it might not be liked. on the other hand it would mean possibly that users could use other libraries to sign beyond ck. like viem or ethers.

conclusions

I think there are benefits to doing all these things. even if they are not full solutions. but i dont know the full answer yet .

aaronmgdr commented 2 months ago

completing #223 could be part of the solution

aaronmgdr commented 2 months ago

my thinking now is that this needs to be sorted out before #64 happens and although its tbd if its needed for cel2 it the codependency is a ticking timebomb which may become an enigma