Skip to content

Commit

Permalink
use eip 191 standard for presigned transactions (#15)
Browse files Browse the repository at this point in the history 
* implement standard from EIP 191 and fix tests

* remove unused code

* fix changes caused by bad rebase

* merge conflicts and fix eslint and solium errors

* remove settings not needed

* store msgHash in the Transaction for parameter validation

* fix eslint
  • Loading branch information
@shine2lay @yuantu1919
shine2lay authored and yuantu1919 committed May 18, 2018
1 parent 499daaa commit 2c18211
Show file tree
Hide file tree
Showing 9 changed files with 125 additions and 148 deletions.
33 changes: 15 additions & 18 deletions contracts/MainChain.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,25 +101,23 @@ contract MainChain is Freezable {
emit Deposit(msg.sender, to, msg.value);
}

/// @dev submit a transaction to be processed
/// @param msgHash sha3 hash of txHash destination value data and VERSION
/// @param txHash transaction hash of the deposit tx in side chain
/// @param destination destination provided in deposit tx in side chain
/// @param value msg.value of deposit tx in side chain
/// @param data data of deposit tx in side chain
/// @param v list of v part of sig
/// @param r list of r part of sig
/// @param s list of s part of sig
function submitTransaction(bytes32 msgHash, bytes32 txHash, address destination, uint256 value, bytes data, uint8[] v, bytes32[] r, bytes32[] s)
notNull(destination)
transactionDoesNotExists(txHash)
txNotBlackListed(txHash)
onlyWhenNotFrozen
public {
/// @dev submit a transaction to be processed
/// @param txHash transaction hash of the deposit tx in side chain
/// @param destination destination provided in deposit tx in side chain
/// @param value msg.value of deposit tx in side chain
/// @param data data of deposit tx in side chain
/// @param v list of v part of sig
/// @param r list of r part of sig
/// @param s list of s part of sig
function submitTransaction(bytes32 txHash, address destination, uint256 value, bytes data, uint8[] v, bytes32[] r, bytes32[] s)
notNull(destination)
transactionDoesNotExists(txHash)
txNotBlackListed(txHash)
onlyWhenNotFrozen
public {
require(v.length >= required);

bytes32 hashedTxParams = keccak256(txHash, destination, value, data, VERSION);
require(hashedTxParams == msgHash);
bytes32 msgHash = keccak256(byte(0x19), VERSION, address(this),txHash, destination, value, data);

// execute the transaction after all checking the signatures
require (hasEnoughRequiredSignatures(msgHash, v, r, s));
Expand Down Expand Up @@ -160,7 +158,6 @@ contract MainChain is Freezable {
/// @param destination target destination
/// @param value ether value
/// @param data data payload
// @TODO still need to write tests for emergency withdrawal
function emergencyWithdrawal(address destination, uint256 value, bytes data)
onlyWhenFrozen
ownerExists(msg.sender)
Expand Down
91 changes: 47 additions & 44 deletions contracts/SideChain.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,33 @@ contract SideChain is Freezable {
////////////////////////
// Storage Variables
////////////////////////
mapping (bytes32 => Transaction) public sideChainTx;
mapping (bytes32 => SideChainTransaction) public sideChainTx;
mapping (bytes32 => mapping(address => bool)) public isSignedSC;
uint256 sideChainTxCount;

mapping (bytes32 => Transaction) mainChainTxs;
mapping (bytes32 => MainChainTransaction) mainChainTxs;
mapping (bytes32 => Signatures) mainChainSigs;
mapping (bytes32 => mapping(address => bool)) public isSignedMC;
uint256 mainChainTxCount;

////////////////////////
// Structs
////////////////////////
struct Transaction {
struct SideChainTransaction {
bytes32 msgHash;
address destination;
uint256 value;
bytes data;
bool executed;
}


struct MainChainTransaction {
address destination;
uint256 value;
bytes data;
}

struct Signatures {
uint8[] v;
bytes32[] r;
Expand All @@ -76,30 +84,26 @@ contract SideChain is Freezable {
emit Deposit(msg.sender, to, msg.value);
}

/// @dev submit transaction to be processed pending the approvals
/// @param msgHash sha3 hash of txHash destination value data and VERSION
/// @param txHash transaction hash of the deposit tx in main chain
/// @param destination destination provided in deposit tx in main chain
/// @param value msg.value of deposit tx in main chain
/// @param data data of deposit tx in main chain
/// @param v part of sig
/// @param r part of sig
/// @param s part of sig
function submitTransactionSC(bytes32 msgHash, bytes32 txHash, address destination, uint256 value, bytes data, uint8 v, bytes32 r, bytes32 s)
/// @dev submit transaction to be processed pending the approvals
/// @param txHash transaction hash of the deposit tx in main chain
/// @param destination destination provided in deposit tx in main chain
/// @param value msg.value of deposit tx in main chain
/// @param data data of deposit tx in main chain
function submitTransactionSC(bytes32 txHash, address destination, uint256 value, bytes data)
ownerExists(msg.sender)
notNull(destination)
public {
require(!isSignedSC[txHash][msg.sender]);
address signer = ecrecover(msgHash, v, r, s);
require(msg.sender == signer);
bytes32 hashedTxParams = keccak256(txHash, destination, value, data, VERSION);
require(hashedTxParams == msgHash);

bytes32 msgHash = keccak256(txHash, destination, value, data, VERSION);

if (sideChainTx[txHash].destination == address(0)) {
addTransactionSC(txHash, destination, value, data);
} else {
require(sideChainTx[txHash].destination == destination);
require(sideChainTx[txHash].value == value);
addTransactionSC(msgHash, txHash, destination, value, data);
}

// this validates that all the parameters are the same as the previous submitter
require(sideChainTx[txHash].msgHash == msgHash);

isSignedSC[txHash][msg.sender] = true;
emit Confirmation(msg.sender, txHash);
executeTransaction(txHash);
Expand All @@ -124,7 +128,7 @@ contract SideChain is Freezable {
require(isSignedSC[txHash][msg.sender]);
require(!sideChainTx[txHash].executed);
if (isConfirmed(txHash)) {
Transaction storage txn = sideChainTx[txHash];
SideChainTransaction storage txn = sideChainTx[txHash];
txn.executed = true;
if (external_call(txn.destination, txn.value, txn.data.length, txn.data))
emit Execution(txHash);
Expand All @@ -142,7 +146,7 @@ contract SideChain is Freezable {
public
returns(bool) {
uint count = 0;
for (uint i=0; i<owners.length; i++) {
for (uint i = 0; i<owners.length; i++) {
if (isSignedSC[txHash][owners[i]])
count += 1;
if (count == required)
Expand All @@ -156,7 +160,7 @@ contract SideChain is Freezable {
view
public
returns(uint count) {
for (uint i=0; i<owners.length; i++) {
for (uint i = 0; i<owners.length; i++) {
if (isSignedSC[txHash][owners[i]]) {
count += 1;
}
Expand All @@ -172,13 +176,13 @@ contract SideChain is Freezable {
address[] memory isSignedSCTemp = new address[](owners.length);
uint count = 0;
uint i;
for (i=0; i<owners.length; i++)
for (i = 0; i<owners.length; i++)
if (isSignedSC[txHash][owners[i]]) {
isSignedSCTemp[count] = owners[i];
count += 1;
}
_isSignedSC = new address[](count);
for (i=0; i<count; i++)
for (i = 0; i<count; i++)
_isSignedSC[i] = isSignedSCTemp[i];
}

Expand All @@ -188,14 +192,16 @@ contract SideChain is Freezable {


/// @dev Adds a new transaction to the transaction mapping, if transaction does not exist yet.
/// @param msgHash keccek256 hash of the parameters
/// @param txHash Transaction transactionHash
/// @param destination Transaction target address.
/// @param value Transaction ether value.
/// @param data Transaction data payload.
function addTransactionSC(bytes32 txHash, address destination, uint value, bytes data)
function addTransactionSC(bytes32 msgHash, bytes32 txHash, address destination, uint value, bytes data)
notNull(destination)
private {
sideChainTx[txHash] = Transaction({
sideChainTx[txHash] = SideChainTransaction({
msgHash: msgHash,
destination: destination,
value: value,
executed: false,
Expand Down Expand Up @@ -230,27 +236,25 @@ contract SideChain is Freezable {
// SUBMIT SIGNATURES TO WITHDRAW ON MAINCHAIN
//////////////////////////////////////////////

/// @dev store signature to be submitted in mainchain
/// @param msgHash sha3 hash of txHash destination value data and VERSION
/// @param txHash transaction hash of the deposit tx in side chain
/// @param destination destination provided in deposit tx in side chain
/// @param value msg.value of deposit tx in side chain
/// @param data data of deposit tx in side chain
/// @param v part of sig
/// @param r part of sig
/// @param s part of sig
function submitSignatureMC(bytes32 msgHash, bytes32 txHash, address destination, uint256 value, bytes data, uint8 v, bytes32 r, bytes32 s)
/// @dev store signature to be submitted in mainchain
/// @param txHash transaction hash of the deposit tx in side chain
/// @param destination destination provided in deposit tx in side chain
/// @param value msg.value of deposit tx in side chain
/// @param data data of deposit tx in side chain
/// @param v part of sig
/// @param r part of sig
/// @param s part of sig
function submitSignatureMC(bytes32 txHash, address destination, uint256 value, bytes data, uint8 v, bytes32 r, bytes32 s)
ownerExists(msg.sender)
notNull(destination)
public {
require(!isSignedMC[txHash][msg.sender]);

bytes32 msgHash = keccak256(byte(0x19), VERSION, address(this),txHash, destination, value, data);

address signer = ecrecover(msgHash, v, r, s);
require(msg.sender == signer);

bytes32 hashedTxParams = keccak256(txHash, destination, value, data, VERSION);
require(hashedTxParams == msgHash);

if(mainChainTxs[txHash].destination == address(0)){
addTransactionMC(txHash, destination, value, data);
}
Expand All @@ -276,10 +280,9 @@ contract SideChain is Freezable {
function addTransactionMC(bytes32 txHash, address destination, uint256 value, bytes data)
notNull(destination)
private {
mainChainTxs[txHash] = Transaction({
mainChainTxs[txHash] = MainChainTransaction({
destination: destination,
value: value,
executed: false,
data: data
});
mainChainTxCount += 1;
Expand All @@ -291,7 +294,7 @@ contract SideChain is Freezable {
view
public
returns(address destination, uint256 value, bytes data, uint8[] v, bytes32[] r, bytes32[] s) {
Transaction storage tempTx = mainChainTxs[txHash];
MainChainTransaction storage tempTx = mainChainTxs[txHash];
Signatures storage tempSigs = mainChainSigs[txHash];
return (tempTx.destination, tempTx.value, tempTx.data, tempSigs.v, tempSigs.r, tempSigs.s);
}
Expand Down
8 changes: 4 additions & 4 deletions contracts/libs/Freezable.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ contract Freezable is MultiSigOwnable {
if (frozenAt == 0 || approvalCount == 0) return false;
else {
uint256 frozenDuration = minTimeFrozen * (frozenDurationRatio ** (approvalCount - 1));
if (now < (frozenAt + frozenDuration)) return true;
if (block.timestamp < (frozenAt + frozenDuration)) return true;
}
return false;
}
Expand Down Expand Up @@ -85,8 +85,8 @@ contract Freezable is MultiSigOwnable {
approvalCount++;

if (frozenAt == 0) {
frozenAt = now;
emit contractFrozen(now);
frozenAt = block.timestamp;
emit contractFrozen(block.timestamp);
}
}

Expand All @@ -100,7 +100,7 @@ contract Freezable is MultiSigOwnable {

if (approvalCount == 0) {
frozenAt = 0;
emit contractUnFrozen(now);
emit contractUnFrozen(block.timestamp);
}
}
}
13 changes: 6 additions & 7 deletions contracts/libs/MultiSigOwnable.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,10 @@ contract MultiSigOwnable {
}

modifier validRequirement(uint ownerCount, uint8 _required) {
require(
ownerCount <= MAX_OWNER_COUNT
&& _required <= ownerCount
&& _required != 0
&& ownerCount != 0);
require(ownerCount <= MAX_OWNER_COUNT);
require(_required <= ownerCount);
require(_required != 0);
require(ownerCount != 0);
_;
}

Expand Down Expand Up @@ -84,9 +83,9 @@ contract MultiSigOwnable {
ownerExists(owner)
public {
isOwner[owner] = false;
for (uint i=0; i< owners.length - 1; i++) {
for (uint i = 0; i < owners.length - 1; i++) {
if (owners[i] == owner) {
owners[i] = owners[ owners.length - 1];
owners[i] = owners[owners.length - 1];
break;
}
}
Expand Down
10 changes: 6 additions & 4 deletions test/MainChainUnitTest/addBlackListUnitTest.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ let txHash;
let toAddress;
let value;
let version;
let contractAddress;

contract('MainChain: addBlackList Unit Test', function(accounts) {
beforeEach(async function() {
Expand All @@ -27,6 +28,7 @@ contract('MainChain: addBlackList Unit Test', function(accounts) {
toAddress = mainchainInstance.address;
value = 0;
version = await mainchainInstance.VERSION.call();
contractAddress = mainchainInstance.address;
});

it('checks that addBlackList work as intended if all the condition are valid', async function() {
Expand All @@ -44,14 +46,14 @@ contract('MainChain: addBlackList Unit Test', function(accounts) {
);
const sigs = utils.multipleSignedTransaction(
[0, 1],
contractAddress,
txHash,
toAddress,
value,
addBlackListData,
version,
);
await mainchainInstance.submitTransaction(
sigs.msgHash,
txHash,
toAddress,
value,
Expand Down Expand Up @@ -90,14 +92,14 @@ contract('MainChain: addBlackList Unit Test', function(accounts) {
);
let sigs = utils.multipleSignedTransaction(
[0, 1],
contractAddress,
txHash,
toAddress,
value,
addBlackListData,
version,
);
await mainchainInstance.submitTransaction(
sigs.msgHash,
txHash,
toAddress,
value,
Expand All @@ -114,14 +116,14 @@ contract('MainChain: addBlackList Unit Test', function(accounts) {
// const data = mainchainInstance.contract.addBlackList.getData(txHash);
sigs = utils.multipleSignedTransaction(
[0, 1],
contractAddress,
txHashes[1],
toAddress,
value,
addBlackListData,
version,
);
const res = await mainchainInstance.submitTransaction(
sigs.msgHash,
txHashes[1],
toAddress,
value,
Expand All @@ -141,14 +143,14 @@ contract('MainChain: addBlackList Unit Test', function(accounts) {
);
const sigs = utils.multipleSignedTransaction(
[0, 1],
contractAddress,
txHash,
toAddress,
value,
addBlackListData,
version,
);
const res = await mainchainInstance.submitTransaction(
sigs.msgHash,
txHash,
toAddress,
value,
Expand Down
Loading

0 comments on commit 2c18211

Please sign in to comment.