Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible incorrect type in Trezor Connect for ethereumSignTypedData function #4973

Closed
lukeramsden opened this issue Feb 21, 2022 · 1 comment
Labels
bug Something isn't working as expected connect Connect API related (ie. fee calculation) feature Product related issue visible for end user

Comments

@lukeramsden
Copy link

The documentation function ethereumSignTypedData in trezor-connect stipulates you can call the function like so:

TrezorConnect.ethereumSignTypedData({
    path: "m/44'/60'/0'",
    data: eip712Data,
    metamask_v4_compat: true,
    // These are optional, but required for Trezor Model 1 compatibility
    domain_separator_hash,
    message_hash,
});

However, the types for ethereumSignTypedData list two possible function overloads:

function ethereumSignTypedData<T extends Ethereum.EthereumSignTypedDataTypes>(
    params: P.CommonParams & Ethereum.EthereumSignTypedData<T>,
): P.Response<Protobuf.EthereumTypedDataSignature>;
function ethereumSignTypedData(
    params: P.CommonParams & Ethereum.EthereumSignTypedHash,
): P.Response<Protobuf.EthereumTypedDataSignature>;

This leads to a type error if you try to call ethereumSignTypedData with both data/metamask_v4_compat AND the hash fields, despite this being ostensibly (and in my experience so far, demonstrably) supported by the implementation of the function itself.

Therefore, consumer code must resort to hacky workarounds such as:

private ethereumSignTypedData<T extends EthereumSignTypedDataTypes>(
  params: CommonParams & EthereumSignTypedData<T>,
): Response<EthereumTypedDataSignature>;
private ethereumSignTypedData<T extends EthereumSignTypedDataTypes>(
  params: CommonParams & EthereumSignTypedData<T> & EthereumSignTypedHash,
): Response<EthereumTypedDataSignature>;
private ethereumSignTypedData(
  params: CommonParams & EthereumSignTypedHash,
): Response<EthereumTypedDataSignature>;
private ethereumSignTypedData<
  T extends EthereumSignTypedDataTypes,
>(params: CommonParams & EthereumSignTypedData<T> & EthereumSignTypedHash): Response<EthereumTypedDataSignature> {
  // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
  return TrezorConnect.ethereumSignTypedData(params as any);
}

Am I missing something, and this is intentional? Or can this possibly be rectified?

@hynek-jina hynek-jina added connect Connect API related (ie. fee calculation) feature Product related issue visible for end user labels Feb 22, 2022
@sime sime added bug Something isn't working as expected HIGH labels Mar 28, 2022
@mroz22 mroz22 mentioned this issue May 4, 2022
42 tasks
@hynek-jina hynek-jina removed the HIGH label Jun 8, 2022
@szymonlesisz
Copy link
Contributor

i believe this issue was already resolved in connect migration and this test proofs that using both data/metamask_v4_compat AND the hash fields should not throw type error

@github-project-automation github-project-automation bot moved this to 🤝 Needs QA in Issues Suite Sep 6, 2023
@bosomt bosomt moved this from 🤝 Needs QA to ✅ Approved in Issues Suite Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as expected connect Connect API related (ie. fee calculation) feature Product related issue visible for end user
Projects
Archived in project
Development

No branches or pull requests

4 participants