encodeHash() not changing the hash to EIP 191 formart

[Kilonzo88]

In Testnet Zksync demo of account abstraction, we are advised to forgo the line below on the validateTransaction function.

bytes32 convertedHash = MessageHashUtils.toEthSignedMessageHash(txHash);

When I do this the magic returns false as shown below

FAIL: assertion failed: 0x0000000000000000000000000000000000000000000000000000000000000000 != 0x202bcce700000000000000000000000000000000000000000000000000000000] testZkValidateTransaction() (gas: 282847)

[PatrickAlphaC]

Where/why are we advised to do such?

[Kilonzo88]

In Testnet Zksync demo lesson of account abstraction. Here is the full code snippet of the validateTransaction function

function _validateTransaction(Transaction memory _transaction) internal returns (bytes4 magic) {
        ... 
        //The issue is with the check signature
     
        // Check the signature
        bytes32 txHash = _transaction.encodeHash();
        bytes32 convertedHash = MessageHashUtils.toEthSignedMessageHash(txHash);
        address signer = ECDSA.recover(convertedHash, _transaction.signature);
        bool isValidSigner = signer == owner();
        if (isValidSigner) {
            magic = ACCOUNT_VALIDATION_SUCCESS_MAGIC;
        } else {
            magic = bytes4(0);
        }
        return magic;
    }

You've even commented out bytes32 convertedHash on the repository. Your reasoning was that MessageTransactionHelper.sol handles the conversion to EIP 191 format as below.

function _encodeHashEIP712Transaction(Transaction memory _transaction) private view returns (bytes32) {
        bytes32 structHash = keccak256(
            abi.encode(
                EIP712_TRANSACTION_TYPE_HASH,
                _transaction.txType,
                _transaction.from,
                _transaction.to,
                _transaction.gasLimit,
                _transaction.gasPerPubdataByteLimit,
                _transaction.maxFeePerGas,
                _transaction.maxPriorityFeePerGas,
                _transaction.paymaster,
                _transaction.nonce,
                _transaction.value,
                // boo, less efficient cuz not calldata
                // EfficientCall.keccak(_transaction.data),
                keccak256(_transaction.data),
                keccak256(abi.encodePacked(_transaction.factoryDeps)),
                // EfficientCall.keccak(_transaction.paymasterInput)
                keccak256(_transaction.paymasterInput)
            )
        );

        bytes32 domainSeparator =
            keccak256(abi.encode(EIP712_DOMAIN_TYPEHASH, keccak256("zkSync"), keccak256("2"), block.chainid));

        return keccak256(abi.encodePacked("\x19\x01", domainSeparator, structHash));
    }

But this doesn't work so I had to use

bytes32 convertedHash = MessageHashUtils.toEthSignedMessageHash(txHash);

[PatrickAlphaC]

Are you using foundry or foundry-zksync? Can you share with me the command you're using to run this test?

[Kilonzo88]

foundry-zkysync I guess

forge test --mt testZkValidateTransaction --zksync -vvvv

I've also noted something odd noticed something odd. The test now reverts and only works when I've commented out the increment nonce part of ZkMinimalAccount.sol. I'm sure it wasn't like this before. Please help

// Call nonceholder
        // increment nonce
        // call(x, y, z) -> system contract call
        // SystemContractsCaller.systemCallWithPropagatedRevert(
        //     uint32(gasleft()), //remaining gas limit
        //     address(NONCE_HOLDER_SYSTEM_CONTRACT), //address to
        //     0, //value
        //     abi.encodeCall(INonceHolder.incrementMinNonceIfEquals, (_transaction.nonce)) //data
        // );