zerodevapp / kernel

https://docs.zerodev.app/
MIT License
177 stars 62 forks source link

isValidSignature can't actually verify signatures for external contracts #92

Closed sirpy closed 6 months ago

sirpy commented 6 months ago

lets say contract X wants to verify an eip712 signature. Now contract X would already have his own typed_structure which he uses to contract the digest hash. So when this contract would call the smartaccount isValidSignature(digest...) The verification will fail because the kernel isValidSignature calculates a digest for the digest. I believe isValidSignature is supposed to be used to verify requests from other contracts and not signatures designated to the smartaccount.

https://github.com/zerodevapp/kernel/blob/31180268020830c4ef794f68b3bb60dac9ecfa05/src/Kernel.sol#L304

sirpy commented 6 months ago

Ok i've figure that you in order to verify an eip signature you actually have to sign a wrapped version of the original digest. This is really not standard and not supported easily by frameworks like ethers. But i was able to make it work like that

// create the digest instead of signing it
const digest = _TypedDataEncoder.hash(
        yourContractDomain
        yourContractDomainTypes
       yourContractValues
      );

      // wrap the digest in eip712 domain
      // required for Kernel smart contract isValidSignature
      const wrappedDigest = utils.keccak256(
        hexConcat([
          "0x1901",
          _TypedDataEncoder.hashDomain({
            name: "Kernel",
            version: kernelVersion,
            chainId,
            verifyingContract: smartAccountAddress
          }),
          digest
        ])
      );
      signer?.signMessage(utils.arrayify(wrappedDigest));
leekt commented 6 months ago

actually, this is how this has to be handled, without wrapping the digest, it will face the signature replay issue if signer owns more than 1 smart contract wallet

sirpy commented 6 months ago

i'd suggest adding this to the docs somewhere