thirdweb-dev / js

Best in class web3 SDKs for Browser, Node and Mobile apps
https://thirdweb.com
Apache License 2.0
437 stars 350 forks source link

[SDK v5][InApp Wallet][Custom Auth][Invalid signatures] #3301

Closed thevan-bic closed 4 months ago

thevan-bic commented 4 months ago

After upgrading the SDK from v4 to v5, i get an invalid signature when trying to send a signed userop.

{
    "jsonrpc": "2.0",
    "id": 1,
    "error": {
        "code": -32507,
        "message": "Invalid UserOp signature or paymaster signature"
    }
}

I downgraded the SDK to v4 and everything is working normally again. What issue is occurring with the combining of 2/3 shares?

thevan-bic commented 4 months ago

I combined to get the private key and created a wallet from that private key using Ether, then signed a message. It worked perfectly.

joaquim-verges commented 4 months ago

@thevan-bic do you have a minimal code snippet to reproduce this?

We use inApp + smart accounts extensively and haven't seen this before, wanna make sure we didn't miss a case somewhere.

Any info you can provide would be super helpful!

thevan-bic commented 4 months ago

This is my code:

export class ThirdwebV5Provider implements IProvider {
    private _clientId: string;
    private _account: Account;
    private _chain: Chain;
    private _client: ThirdwebClient;

    constructor(options: ProviderOptions<'thirdwebv5'>) {
        const {clientId , chain } =  options;
        this._clientId = clientId;
        this._client = createThirdwebClient({
            clientId: clientId,
        });
        this._chain = chain;
    }

    public async login(authRequest: AuthRequest): Promise<WalletDetail> {
        const {
            payload,
            options: {encryptionKey},
        } = authRequest;
        const authReq: WalletConnectionOption<"inApp"> = {
            client: this._client,
            chain: this._chain,
            strategy: "auth_endpoint",
            payload: JSON.stringify(payload),
            encryptionKey: encryptionKey,
        };
        const [account, chain] = await connectInAppWallet(authReq);
        this._account = account;
        return {localAddress: account.address};
    }

    public async signMessage(message: Uint8Array): Promise<string> {
        return this._account.signMessage({message})// Not working
        //===========================
        const singer = await this.getSinger();
        return singer.signMessage(message) // Not working
    }

    public getSinger(): Promise<Signer> {
        return ethers5Adapter.signer.toEthers({
            client: this._client,
            chain: this._chain,
            account: this._account,
        }) as unknown as Promise<Signer>;
    }
}

Version of SDK is 5.26.0.

When I tried to understand what was going on, I discovered that even though I was using version 5.26.0, thirdweb's iframe wallet seemed to be using version 5.30.0. This was evident in the instance when I used two shares to reconstruct the private key and only the function using sss proved successful

async function getWalletPrivateKeyFromShares(shares: string[]) {
  const sss = await import("./sss.js");
  let privateKeyHex = sss.secrets.combine(shares, 0);
  if (!isHex(privateKeyHex)) {
    privateKeyHex = `0x${privateKeyHex}`;
  }
  const prefixPrivateKey = hexToString(privateKeyHex as Hex);
  if (!prefixPrivateKey.startsWith("thirdweb_")) {
    throw new Error("Invalid private key reconstructed from shares");
  }
  const privateKey = prefixPrivateKey.replace("thirdweb_", "");
  return privateKey;
}

After all I exported the private key and imported it back into ether5, viem and ether6. Only ether6 works.

joaquim-verges commented 4 months ago

@thevan-bic i think the issue is that you're signing raw data - which looks like we were not handling well in our adapter. Just put up #3366 which fixes this.

you should be able to verify that's the issue by changing this in your code

// this 
return this._account.signMessage({ message: raw: { message } })

// instead of this
return this._account.signMessage({message})
thevan-bic commented 4 months ago

@thevan-bic i think the issue is that you're signing raw data - which looks like we were not handling well in our adapter. Just put up #3366 which fixes this.

you should be able to verify that's the issue by changing this in your code

// this 
return this._account.signMessage({ message: raw: { message } })

// instead of this
return this._account.signMessage({message})

@joaquim-verges Thank you, it worked.