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

Unnecessary domainSeparator field in Request #181

Open
raullaprida opened this issue Dec 3, 2021 · 1 comment
Open

Unnecessary domainSeparator field in Request #181

raullaprida opened this issue Dec 3, 2021 · 1 comment
Assignees
Labels
enhancement New feature or request

Comments

@raullaprida
Copy link
Collaborator

raullaprida commented Dec 3, 2021

The domainSeparator allows dApp developers be guaranteed that there can be no signature collision between multiple DApps.
Having said that, there's no need to include it as part of the Relay Data of the Request

Right now the included domainSeparator is only used on the signature verification, the RIF Relay system is not filtering nor registering specific domains (which could be used to allowing only specific registered domainSeparators to participate on RIF Relay). The specific SmartWallet Template in use for the invoked SmartWallet instance is the one that knows what domainSeparator to accept
For that reason, including the domainSeparator on the request is only making the relay call more expensive.

The default SmartWallet template included in our repo already has all the information to calculate the domainSeparator as a constant value:

require( keccak256(abi.encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), keccak256("RSK Enveloping Transaction"), //DOMAIN_NAME DATA_VERSION_HASH, getChainID(), address(this))) == domainSeparator, "Invalid domain separator" );

Once the domainSeparator attribute is removed from the request, this verification can be deleted. That won't affect the signature verification

require( RSKAddrValidator.safeEquals( keccak256(abi.encodePacked( "\x19\x01", domainSeparator, keccak256(_getEncoded(suffixData, req))) ).recover(sig), req.from), "signature mismatch" );

We just need to replace domainSeparator attribute which was received as an input parameter with the constant value that we already know:

bytes32 private constant domainSeparator = keccak256(abi.encode( keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), keccak256("RSK Enveloping Transaction"), //DOMAIN_NAME DATA_VERSION_HASH, getChainID(), address(this)))

Just in case background information regarding EIP712 and how to use it is needed, I leave a link to a medium article from Metamask on how to use EIP712

@raullaprida raullaprida added the enhancement New feature or request label Dec 3, 2021
@mortelli
Copy link
Contributor

mortelli commented Mar 3, 2022

PP-137

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants