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.

Sign up for GitHub

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

PP-137/Unnecessary domainSeparator field in Request #34

Closed
wants to merge 6 commits into from
Closed

PP-137/Unnecessary domainSeparator field in Request #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4fe08c0
Reduces the cost of transactions by avoiding sending the domainSepara…
AndresQuijano Jun 8, 2022
d580af1
Changes CustomSmartWallet contract in order to avoid using the domain…
AndresQuijano Jun 9, 2022
6e306a9
Eliminate the use of domainSeparator in customSmartWallets with custo…
AndresQuijano Jun 9, 2022
f2d2a66
Eliminate the use of the domainSeparator as part of the request to cr…
AndresQuijano Jun 14, 2022
18d1110
Eliminates comments and refine details regarding the elimination of t…
AndresQuijano Jun 16, 2022
3add688
Update the dist folder
antomor Jun 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 17 additions & 21 deletions contracts/factory/CustomSmartWalletFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {
bytes14 private constant RUNTIME_END = hex"5AF43D923D90803E602B57FD5BF3";
address public masterCopy; // this is the ForwarderProxy contract that will be proxied
bytes32 public constant DATA_VERSION_HASH = keccak256("2");
bytes32 public domainSeparator;

// Nonces of senders, used to prevent replay attacks
mapping(address => uint256) private nonces;
Expand All @@ -90,8 +91,21 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {
*/
constructor(address forwarderTemplate) public {
masterCopy = forwarderTemplate;
buildDomainSeparator();
}

function buildDomainSeparator() internal {
domainSeparator = keccak256(
abi.encode(
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), //hex"8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f",
keccak256("RSK Enveloping Transaction"), //DOMAIN_NAME hex"d41b7f69f4d7734774d21b5548d74704ad02f9f1545db63927a1d58479c576c8"
DATA_VERSION_HASH,
getChainID(),
address(this)
)
);
}


function runtimeCodeHash() external override view returns (bytes32){
return keccak256(
Expand Down Expand Up @@ -156,13 +170,12 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {

function relayedUserSmartWalletCreation(
IForwarder.DeployRequest memory req,
bytes32 domainSeparator,
bytes32 suffixData,
bytes calldata sig
) external override {
(sig);
require(msg.sender == req.relayHub, "Invalid caller");
_verifySig(req, domainSeparator, suffixData, sig);
_verifySig(req, suffixData, sig);
nonces[req.from]++;

//e6ddc71a => initialize(address owner,address logic,address tokenAddr,address tokenRecipient,uint256 tokenAmount,uint256 tokenGas,bytes initParams)
Expand Down Expand Up @@ -278,7 +291,7 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {
return
abi.encodePacked(
keccak256(
"RelayRequest(address relayHub,address from,address to,address tokenContract,address recoverer,uint256 value,uint256 nonce,uint256 tokenAmount,uint256 tokenGas,uint256 index,bytes data,RelayData relayData)RelayData(uint256 gasPrice,bytes32 domainSeparator,address relayWorker,address callForwarder,address callVerifier)"
"RelayRequest(address relayHub,address from,address to,address tokenContract,address recoverer,uint256 value,uint256 nonce,uint256 tokenAmount,uint256 tokenGas,uint256 index,bytes data,RelayData relayData)RelayData(uint256 gasPrice,address relayWorker,address callForwarder,address callVerifier)"
),
abi.encode(
req.relayHub,
Expand Down Expand Up @@ -306,29 +319,12 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {

function _verifySig(
IForwarder.DeployRequest memory req,
bytes32 domainSeparator,
bytes32 suffixData,
bytes memory sig
) internal view {
//Verify nonce
require(nonces[req.from] == req.nonce, "nonce mismatch");

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

require(
RSKAddrValidator.safeEquals(
keccak256(
Expand All @@ -341,7 +337,7 @@ contract CustomSmartWalletFactory is ICustomSmartWalletFactory {
.recover(sig),
req.from
),
"signature mismatch"
"Signature mismatch"
);
}
}
35 changes: 16 additions & 19 deletions contracts/factory/SmartWalletFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ contract SmartWalletFactory is ISmartWalletFactory {
bytes14 private constant RUNTIME_END = hex"5AF43D923D90803E602B57FD5BF3";
address public masterCopy; // this is the ForwarderProxy contract that will be proxied
bytes32 public constant DATA_VERSION_HASH = keccak256("2");
bytes32 public domainSeparator;

// Nonces of senders, used to prevent replay attacks
mapping(address => uint256) private nonces;
Expand All @@ -89,8 +90,21 @@ contract SmartWalletFactory is ISmartWalletFactory {
*/
constructor(address forwarderTemplate) public {
masterCopy = forwarderTemplate;
buildDomainSeparator();
}

function buildDomainSeparator() internal {
domainSeparator = keccak256(
abi.encode(
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), //hex"8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f",
keccak256("RSK Enveloping Transaction"), //DOMAIN_NAME hex"d41b7f69f4d7734774d21b5548d74704ad02f9f1545db63927a1d58479c576c8"
DATA_VERSION_HASH,
getChainID(),
address(this)
)
);
}

function runtimeCodeHash() external override view returns (bytes32){
return keccak256(
abi.encodePacked(RUNTIME_START, masterCopy, RUNTIME_END)
Expand Down Expand Up @@ -137,13 +151,12 @@ contract SmartWalletFactory is ISmartWalletFactory {

function relayedUserSmartWalletCreation(
IForwarder.DeployRequest memory req,
bytes32 domainSeparator,
bytes32 suffixData,
bytes calldata sig
) external override {

require(msg.sender == req.relayHub, "Invalid caller");
_verifySig(req, domainSeparator, suffixData, sig);
_verifySig(req, suffixData, sig);
nonces[req.from]++;

//a6b63eb8 => initialize(address owner,address tokenAddr,address tokenRecipient,uint256 tokenAmount,uint256 tokenGas)
Expand Down Expand Up @@ -239,7 +252,7 @@ contract SmartWalletFactory is ISmartWalletFactory {
) public pure returns (bytes memory) {
return
abi.encodePacked(
keccak256("RelayRequest(address relayHub,address from,address to,address tokenContract,address recoverer,uint256 value,uint256 nonce,uint256 tokenAmount,uint256 tokenGas,uint256 index,bytes data,RelayData relayData)RelayData(uint256 gasPrice,bytes32 domainSeparator,address relayWorker,address callForwarder,address callVerifier)"),
keccak256("RelayRequest(address relayHub,address from,address to,address tokenContract,address recoverer,uint256 value,uint256 nonce,uint256 tokenAmount,uint256 tokenGas,uint256 index,bytes data,RelayData relayData)RelayData(uint256 gasPrice,address relayWorker,address callForwarder,address callVerifier)"),
abi.encode(
req.relayHub,
req.from,
Expand All @@ -266,28 +279,13 @@ contract SmartWalletFactory is ISmartWalletFactory {

function _verifySig(
IForwarder.DeployRequest memory req,
bytes32 domainSeparator,
bytes32 suffixData,
bytes memory sig
) internal view {

//Verify nonce
require(nonces[req.from] == req.nonce, "nonce mismatch");

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

require(
RSKAddrValidator.safeEquals(
keccak256(abi.encodePacked(
Expand All @@ -297,5 +295,4 @@ contract SmartWalletFactory is ISmartWalletFactory {
).recover(sig), req.from),"signature mismatch"
);
}

}
1 change: 0 additions & 1 deletion contracts/interfaces/EnvelopingTypes.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import "./IForwarder.sol";
interface EnvelopingTypes {
struct RelayData {
uint256 gasPrice;
bytes32 domainSeparator;
address relayWorker;
address callForwarder;
address callVerifier;
Expand Down
3 changes: 0 additions & 3 deletions contracts/interfaces/IForwarder.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ interface IForwarder {
* revert if either signature or nonce are incorrect.
*/
function verify(
bytes32 domainSeparator,
bytes32 suffixData,
ForwardRequest calldata forwardRequest,
bytes calldata signature
Expand All @@ -52,7 +51,6 @@ interface IForwarder {
/**
* execute a transaction
* @param forwardRequest - all transaction parameters
* @param domainSeparator - domain used when signing this request
* @param suffixData - the extension data used when signing this request.
* @param signature - signature to validate.
*
Expand All @@ -62,7 +60,6 @@ interface IForwarder {
* are reported using the returned "success" and ret string
*/
function execute(
bytes32 domainSeparator,
bytes32 suffixData,
ForwardRequest calldata forwardRequest,
bytes calldata signature
Expand Down
1 change: 0 additions & 1 deletion contracts/interfaces/IWalletCustomLogic.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ interface IWalletCustomLogic {
* Lets a relayer execute the custom logic
*/
function execute(
bytes32 domainSeparator,
bytes32 suffixData,
IForwarder.ForwardRequest calldata forwardRequest,
bytes calldata signature
Expand Down
1 change: 0 additions & 1 deletion contracts/interfaces/IWalletFactory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ interface IWalletFactory {

function relayedUserSmartWalletCreation(
IForwarder.DeployRequest memory req,
bytes32 domainSeparator,
bytes32 suffixData,
bytes calldata sig
) external;
Expand Down
37 changes: 19 additions & 18 deletions contracts/smartwallet/CustomSmartWallet.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,26 @@ contract CustomSmartWallet is IForwarder {

uint256 public override nonce;
bytes32 public constant DATA_VERSION_HASH = keccak256("2");
bytes32 public domainSeparator;

function buildDomainSeparator() internal {
domainSeparator = keccak256(
abi.encode(
keccak256("EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)"), //hex"8b73c3c69bb8fe3d512ecc4cf759cc79239f7b179b0ffacaa9a75d522b39400f",
keccak256("RSK Enveloping Transaction"), //DOMAIN_NAME hex"d41b7f69f4d7734774d21b5548d74704ad02f9f1545db63927a1d58479c576c8"
DATA_VERSION_HASH,
getChainID(),
address(this)
)
);
}

function verify(
bytes32 domainSeparator,
bytes32 suffixData,
ForwardRequest memory req,
bytes calldata sig
) external override view {

_verifySig(domainSeparator, suffixData, req, sig);
_verifySig(suffixData, req, sig);
}

function getOwner() private view returns (bytes32 owner){
Expand Down Expand Up @@ -101,7 +112,6 @@ contract CustomSmartWallet is IForwarder {
}

function execute(
bytes32 domainSeparator,
bytes32 suffixData,
ForwardRequest memory req,
bytes calldata sig
Expand All @@ -118,7 +128,7 @@ contract CustomSmartWallet is IForwarder {
(sig);
require(msg.sender == req.relayHub, "Invalid caller");

_verifySig(domainSeparator, suffixData, req, sig);
_verifySig(suffixData, req, sig);
nonce++;

if(req.tokenAmount > 0){
Expand Down Expand Up @@ -179,7 +189,6 @@ contract CustomSmartWallet is IForwarder {
}

function _verifySig(
bytes32 domainSeparator,
bytes32 suffixData,
ForwardRequest memory req,
bytes memory sig
Expand All @@ -193,24 +202,14 @@ contract CustomSmartWallet is IForwarder {

//Verify nonce
require(nonce == req.nonce, "nonce mismatch");

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"
);

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

Expand All @@ -220,7 +219,7 @@ contract CustomSmartWallet is IForwarder {
) private pure returns (bytes memory) {
return
abi.encodePacked(
keccak256("RelayRequest(address relayHub,address from,address to,address tokenContract,uint256 value,uint256 gas,uint256 nonce,uint256 tokenAmount,uint256 tokenGas,bytes data,RelayData relayData)RelayData(uint256 gasPrice,bytes32 domainSeparator,address relayWorker,address callForwarder,address callVerifier)"), //requestTypeHash,
keccak256("RelayRequest(address relayHub,address from,address to,address tokenContract,uint256 value,uint256 gas,uint256 nonce,uint256 tokenAmount,uint256 tokenGas,bytes data,RelayData relayData)RelayData(uint256 gasPrice,address relayWorker,address callForwarder,address callVerifier)"), //requestTypeHash,
abi.encode(
req.relayHub,
req.from,
Expand Down Expand Up @@ -322,6 +321,8 @@ contract CustomSmartWallet is IForwarder {
)
}
}

buildDomainSeparator();
}

function _fallback() private {
Expand Down
Loading