Skip to content

sign_typed_data_v4 TODO: is the eth_signTypedData_v4 override still needed? #467

Description

@teyrebaz33

Describe the question

crates/seismic/rpc/src/eth/ext.rs's sign_typed_data_v4 handler carries an author TODO:

/// Handler for: `eth_signTypedData_v4`
///
/// TODO: determine if this should be removed, seems the same as eth functionality
async fn sign_typed_data_v4(&self, from: Address, data: TypedData) -> RpcResult<String> {
    debug!(target: "reth-seismic-rpc::eth", "Serving seismic eth_signTypedData_v4 extension");
    let signature = EthTransactions::sign_typed_data(&self.eth_api, &data, from)
        .map_err(|err| err.into())?;
    let signature = alloy_primitives::hex::encode(signature);
    Ok(format!("0x{signature}"))
}

What I found

Upstream reth's own core.rs (reth_rpc_eth_api) implements eth_signTypedData (no _v4 suffix) and returns raw Bytes:

/// Handler for: `eth_signTypedData`
async fn sign_typed_data(&self, address: Address, data: TypedData) -> RpcResult<Bytes> {
    trace!(target: "rpc::eth", ?address, ?data, "Serving eth_signTypedData");
    Ok(EthTransactions::sign_typed_data(self, &data, address)?)
}

So the two aren't actually the same "eth functionality":

  • Method name: standard reth exposes eth_signTypedData; this override additionally exposes eth_signTypedData_v4. greping the crate, eth_signTypedData (no _v4) is never separately registered here — only the _v4 name is served.
  • Response shape: standard returns raw Bytes; this override returns a hex String prefixed with 0x.

_v4 is the name real wallets and dapp libraries actually call — MetaMask's docs recommend it for EIP-712 signing, and other client libraries (e.g. seaport-js) explicitly fall back between eth_signTypedData_v4 and eth_signTypedData because node/wallet support for the two names differs. Removing this override would mean the node no longer answers eth_signTypedData_v4 at all, breaking anything that calls it by that name specifically.

Suggested resolution

Doesn't look like it should be removed. I think the TODO can be resolved by replacing it with a comment explaining the naming/response-shape difference (so a future reader doesn't have to re-derive this), rather than removing the handler. Happy to open a doc-only PR for that if useful.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions