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

updated contract to use a single signer' #51

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
34 changes: 19 additions & 15 deletions src/contracts/account/ProxyAccountDeployer.sol
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
pragma solidity 0.6.2;
pragma experimental ABIEncoderV2;

import "@openzeppelin/contracts/cryptography/ECDSA.sol";
import "./ReplayProtection.sol";
import "../ops/BatchInternal.sol";
import "../SingleSigner.sol";

/**
* We deploy a new contract to bypass the msg.sender problem.
*/
contract ProxyAccount is ReplayProtection, BatchInternal {
contract ProxyAccount is SingleSigner, ReplayProtection, BatchInternal {

address public owner;
event MetaTxInfo(bytes replayProtection, address replayProtectionAuthority, bytes32 indexed txid);

struct MetaTx {
address to;
Expand All @@ -18,14 +20,6 @@ contract ProxyAccount is ReplayProtection, BatchInternal {
CallType callType;
}

/**
* Due to create clone, we need to use an init() method.
*/
function init(address _owner) public {
require(owner == address(0), "Owner is already set");
owner = _owner;
}

/**
* We check the signature has authorised the call before executing it.
* @param _metaTx A single meta-transaction that includes to, value and data
Expand All @@ -40,10 +34,16 @@ contract ProxyAccount is ReplayProtection, BatchInternal {
bytes memory _signature) public returns (bool, bytes memory) {

// Assumes that ProxyAccountDeployer is ReplayProtection.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this comment out of date?

bytes memory encodedData = abi.encode(_metaTx.callType, _metaTx.to, _metaTx.value, _metaTx.data);
bytes memory encodedCallData = abi.encode(_metaTx.callType, _metaTx.to, _metaTx.value, _metaTx.data);
bytes memory encodedTxData = abi.encode(encodedCallData, _replayProtection, _replayProtectionAuthority, address(this), getChainID());
bytes32 txid = keccak256(encodedTxData);

// Reverts if fails.
// Signer/owner is derived from SingleSigner
authenticate(txid, _signature);
replayProtection(getOwner(), _replayProtection, _replayProtectionAuthority);
emit MetaTxInfo(_replayProtection, _replayProtectionAuthority, txid);

// // Reverts if fails.
require(owner == verify(encodedData, _replayProtection, _replayProtectionAuthority, _signature), "Owner did not sign this meta-transaction.");
require(_metaTx.callType == CallType.CALL || _metaTx.callType == CallType.DELEGATE, "Signer did not pick a valid calltype");

bool success;
Expand Down Expand Up @@ -80,10 +80,14 @@ contract ProxyAccount is ReplayProtection, BatchInternal {
address _replayProtectionAuthority,
bytes memory _signature) public {

bytes memory encodedData = abi.encode(CallType.BATCH, _metaTxList);
bytes memory encodedCallData = abi.encode(CallType.BATCH, _metaTxList);
bytes memory encodedTxData = abi.encode(encodedCallData, _replayProtection, _replayProtectionAuthority, address(this), getChainID());
bytes32 txid = keccak256(encodedTxData);

// Reverts if fails.
require(owner == verify(encodedData, _replayProtection, _replayProtectionAuthority, _signature), "Owner did not sign this meta-transaction.");
authenticate(txid, _signature);
replayProtection(getOwner(), _replayProtection, _replayProtectionAuthority);
emit MetaTxInfo(_replayProtection, _replayProtectionAuthority, txid);

// Runs the batch function in MultiSend.
// It supports CALL and DELEGATECALL.
Expand Down
30 changes: 25 additions & 5 deletions src/contracts/account/RelayHub.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pragma solidity 0.6.2;
pragma experimental ABIEncoderV2;

import "@openzeppelin/contracts/cryptography/ECDSA.sol";
import "@openzeppelin/contracts/utils/Create2.sol";
import "./ReplayProtection.sol";
import "./CallTypes.sol";
Expand All @@ -25,11 +26,27 @@ contract RelayHub is ReplayProtection, CallTypes, RevertMessage {
bool revertOnFail;
}

/**
* Compute the signed message and fetch the signer's address.
* @param _encodedCallData Encoded call data
* @param _replayProtection Encoded Replay Protection
* @param _replayProtectionAuthority Identify the Replay protection, default is address(0)
* @param _signature Signature from signer
*/
function getSigner(bytes memory _encodedCallData, bytes memory _replayProtection, address _replayProtectionAuthority,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in the chat, I think we should use this everywhere

bytes memory _signature) public view returns(address) {
bytes memory encodedTxData = abi.encode(_encodedCallData, _replayProtection, _replayProtectionAuthority, address(this), getChainID());
bytes32 txid = keccak256(encodedTxData);
address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(txid), _signature);
return signer;
}

/**
* Each signer has a contract account (signers address => contract address).
* We check the signer has authorised the target contract and function call. Then, we pass it to the
* signer's contract account to perform the final execution (to help us bypass msg.sender problem).
* @param _metaTx A single meta-transaction that includes to, value and data
* @param _replayProtection Encoded Replay Protection
* @param _replayProtectionAuthority Identify the Replay protection, default is address(0)
* @param _signer Signer's address
* @param _signature Signature from signer
Expand All @@ -41,11 +58,13 @@ contract RelayHub is ReplayProtection, CallTypes, RevertMessage {
address _signer,
bytes memory _signature) public returns(bool, bytes memory){

bytes memory encodedData = abi.encode(CallType.CALL, _metaTx.to, _metaTx.data);
// Verify the signature
bytes memory encodedCallData = abi.encode(CallType.CALL, _metaTx.to, _metaTx.data);

// // Reverts if fails.
require(_signer == verify(encodedData, _replayProtection, _replayProtectionAuthority, _signature),
// Reverts if fails.
require(_signer == getSigner(encodedCallData, _replayProtection, _replayProtectionAuthority, _signature),
"Signer did not sign this meta-transaction.");
replayProtection(_signer, _replayProtection, _replayProtectionAuthority);

// Does not revert. Lets us save the replay protection if it fails.
(bool success, bytes memory returnData) = _metaTx.to.call(abi.encodePacked(_metaTx.data, _signer));
Expand All @@ -71,10 +90,11 @@ contract RelayHub is ReplayProtection, CallTypes, RevertMessage {
address _replayProtectionAuthority,
address _signer,
bytes memory _signature) public {
bytes memory encodedData = abi.encode(CallType.BATCH, _metaTxList);
bytes memory encodedCallData = abi.encode(CallType.BATCH, _metaTxList);

// Reverts if fails.
require(_signer == verify(encodedData, _replayProtection, _replayProtectionAuthority, _signature), "Owner did not sign this meta-transaction.");
require(_signer == getSigner(encodedCallData, _replayProtection, _replayProtectionAuthority, _signature),
"Owner did not sign this meta-transaction.");

// Go through each revertable meta transaction and/or meta-deployment.
for(uint i=0; i<_metaTxList.length; i++) {
Expand Down
30 changes: 11 additions & 19 deletions src/contracts/account/ReplayProtection.sol
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
pragma solidity 0.6.2;
pragma experimental ABIEncoderV2;

import "@openzeppelin/contracts/cryptography/ECDSA.sol";
import "./IReplayProtectionAuthority.sol";

contract ReplayProtection {
Expand All @@ -28,42 +27,33 @@ contract ReplayProtection {
*
* Why is there no signing authority? An attacker can supply an address that returns a fixed signer
* so we need to restrict it to a "pre-approved" list of authorities (DAO).
* @param _callData Function name and data to be called
* @param _signer Address of the signer
* @param _replayProtectionAuthority What replay protection will we check?
* @param _replayProtection Encoded replay protection
* @param _signature Signer's signature
*/
function verify(bytes memory _callData,
function replayProtection(address _signer,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should be called checkAndUpdateReplayProtection

bytes memory _replayProtection,
address _replayProtectionAuthority,
bytes memory _signature) internal returns(address){

// Extract signer's address.
bytes memory encodedData = abi.encode(_callData, _replayProtection, _replayProtectionAuthority, address(this), getChainID());
bytes32 txid = keccak256(encodedData);
address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(txid), _signature);
address _replayProtectionAuthority) internal {

// Check the user's replay protection.
if(_replayProtectionAuthority == multiNonceAddress) {
// Assumes authority returns true or false. It may also revert.
require(nonce(signer, _replayProtection), "Multinonce replay protection failed");
require(nonce(_signer, _replayProtection), "Multinonce replay protection failed");
} else if (_replayProtectionAuthority == bitFlipAddress) {
require(bitflip(signer, _replayProtection), "Bitflip replay protection failed");
require(bitflip(_signer, _replayProtection), "Bitflip replay protection failed");
} else {
require(IReplayProtectionAuthority(_replayProtectionAuthority).updateFor(signer, _replayProtection), "Replay protection from authority failed");
// The final "else" ensures this require() is always hit and reverts if its bad.
require(IReplayProtectionAuthority(_replayProtectionAuthority).updateFor(_signer, _replayProtection), "Replay protection from authority failed");
}

emit ReplayProtectionInfo(_replayProtectionAuthority, _replayProtection, txid);

return signer;
}

/**
* MultiNonce replay protection.
* Explained: https://github.com/PISAresearch/metamask-comp#multinonce
* Allows a user to send N queues of transactions, but transactions in each queue are accepted in order.
* If queue==0, then it is a single queue (e.g. NONCE replay protection)
* @param _replayProtection Contains a single nonce
* @param _signer Signer's address
* @param _replayProtection Contains the two nonces
*/
function nonce(address _signer, bytes memory _replayProtection) internal returns(bool) {
uint256 queue;
Expand All @@ -85,6 +75,8 @@ contract ReplayProtection {
* Bitflip Replay Protection
* Explained: https://github.com/PISAresearch/metamask-comp#bitflip
* Signer flips a bit for every new transaction. Each queue supports 256 bit flips.
* @param _signer Signer's address
* @param _replayProtection Contains the two nonces
*/
function bitflip(address _signer, bytes memory _replayProtection) internal returns(bool) {
(uint256 queue, uint256 bitsToFlip) = abi.decode(_replayProtection, (uint256, uint256));
Expand Down
2 changes: 1 addition & 1 deletion src/contracts/account/RevertMessage.sol
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ pragma solidity 0.6.2;
pragma experimental ABIEncoderV2;

/**
* Common CALL functionality for the proxy contract and relayhub
* Extracts the revert message if a call fails.
*/
contract RevertMessage {

Expand Down
32 changes: 32 additions & 0 deletions src/contracts/account/SingleSigner.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
pragma solidity 0.6.2;
pragma experimental ABIEncoderV2;

import "@openzeppelin/contracts/cryptography/ECDSA.sol";

/**
* Authenticates a single signer
*/
contract SingleSigner {

address public owner;

/// @dev Due to create clone, we need to use an init() method.
function init(address _owner) public {
require(owner == address(0), "Owner is already set");
owner = _owner;
}

/// @dev Authenticates the user's signature
/// @param _txid Hash of meta-tx
/// @param _signature Signature of hash
function authenticate(bytes32 _txid, bytes memory _signature) public view {
address signer = ECDSA.recover(ECDSA.toEthSignedMessageHash(_txid), _signature);
require(signer == owner, "Owner of the proxy account did not authorise the tx");
}

/// @dev Return owner.
function getOwner() internal view returns (address) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this funtion? Cant we just call owner?

return owner;
}

}
4 changes: 4 additions & 0 deletions src/contracts/ops/Echo.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,8 @@ contract Echo {
emit Broadcast(_message);
}

function onlyBroadcast(string memory _message) public {
emit Broadcast(_message);
}

}
10 changes: 5 additions & 5 deletions src/contracts/ops/ReplayProtectionWrapper.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ contract ReplayProtectionWrapper is ReplayProtection {
/**
* Easy wrapper to access ReplayProtection.verify(), an internal method.
*/
function verifyPublic(bytes memory _callData,
function replayProtectionPublic(
address _signer,
bytes memory _replayProtection,
address _replayProtectionAuthority,
address signer,
bytes memory _signature) public {
address _replayProtectionAuthority
) public {

require(signer == verify(_callData, _replayProtection, _replayProtectionAuthority, _signature), "Not expected signer");
replayProtection(_signer, _replayProtection, _replayProtectionAuthority);
}

function noncePublic(address _signer, bytes memory _replayProtection) public {
Expand Down
8 changes: 4 additions & 4 deletions src/deployment/addresses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@ export const DELEGATE_DEPLOYER_SALT_STRING = "GLOBAL_DEPLOYER";
// addresses for the salts above
// IF YOU'RE CHANGING THESE THINK ABOUT DOWNSTREAM SYSTEMS
export const PROXY_ACCOUNT_DEPLOYER_ADDRESS =
"0x902059Ee36702DAbdf44e7EBd760f9a9fEaE668F";
"0x302b402DB8766A9feb8a9661E05693a05e6bd7fA";
export const BASE_ACCOUNT_ADDRESS =
"0x92085d7d57C9A261cDA2B0d4F016EB050d89F8d7";
export const RELAY_HUB_ADDRESS = "0x27cfA763272f561CcaE1A4d17800d0Db78F91719";
export const MULTI_SEND_ADDRESS = "0x3802FEf4c0967d908476d4452c5e25337e40CEFF";
"0xfeD75Ff3D1067A228cbd3a373a3e6FE8693f6E82";
export const RELAY_HUB_ADDRESS = "0x66fB0dF80DF228c5E8388777f403D9B940116a29";
export const MULTI_SEND_ADDRESS = "0x55a4ACF2638dC588DAC38264b84393a15644e154";
export const DELEGATE_DEPLOYER_ADDRESS =
"0xf0B36554191c92f3fE9f6E25cEd4FaD08c969E38";
3 changes: 3 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ export { CallWrapperFactory } from "./typedContracts/CallWrapperFactory";
export { RevertMessageTester } from "./typedContracts/RevertMessageTester";
export { RevertMessageTesterFactory } from "./typedContracts/RevertMessageTesterFactory";

export { SingleSigner } from "./typedContracts/SingleSigner";
export { SingleSignerFactory } from "./typedContracts/SingleSignerFactory";

export { MultiSender } from "./ts/batch/MultiSend";

export { deployMetaTxContracts } from "./deployment/deploy";
Loading