Skip to content

Commit

Permalink
code audit changes
Browse files Browse the repository at this point in the history
  • Loading branch information
zetherz committed Sep 29, 2017
1 parent 5ea96a8 commit 49a7648
Show file tree
Hide file tree
Showing 3 changed files with 96 additions and 48 deletions.
59 changes: 35 additions & 24 deletions contracts/EthMatch.sol
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
pragma solidity ^0.4.15;

import "./base/ERC23Contract.sol";
import "./base/Lib.sol";
import "zeppelin-solidity/contracts/ownership/Ownable.sol";
import "zeppelin-solidity/contracts/math/SafeMath.sol";

Expand All @@ -20,18 +21,28 @@ contract EthMatch is Ownable, ERC23Contract {

uint256 public startTime; // start timestamp when matches may begin
address public master; // current Matchmaster
uint256 public gasReq; // only a var in case it ever needs to be updated for future Ethereum releases
uint256 public gasReq; // require same gas every time in maker()

event MatchmakerPrevails(address indexed matchmaster, address indexed matchmaker, uint256 sent, uint256 actual, uint256 winnings);
event MatchmasterPrevails(address indexed matchmaster, address indexed matchmaker, uint256 sent, uint256 actual, uint256 winnings);
event MatchmasterTakeover(address indexed matchmasterPrev, address indexed matchmasterNew, uint256 balanceNew);

function EthMatch(uint256 _startTime) public {
// can be funded at init if desired
function EthMatch(uint256 _startTime) public payable {
require(_startTime >= now);

startTime = _startTime;
master = msg.sender; // initial
gasReq = 21000;
gasReq = 42000;
}

// ensure proper state
modifier isValid(address _addr) {
require(_addr != 0x0);
require(!Lib.isContract(_addr)); // ban contracts
require(now >= startTime);

_;
}

// fallback function
Expand All @@ -41,12 +52,8 @@ contract EthMatch is Ownable, ERC23Contract {
}

// make a match (and specify payout address)
function maker(address _payoutAddr) public payable {
require(this.balance > 0); // else we haven't started yet
require(msg.gas >= gasReq); // require same amount every time (overages auto-returned)

require(now >= startTime);
require(_payoutAddr != 0x0);
function maker(address _addr) isValid(_addr) public payable {
require(msg.gas >= gasReq); // require same gas every time (overages auto-returned)

uint256 weiPaid = msg.value;
require(weiPaid > 0);
Expand All @@ -56,21 +63,21 @@ contract EthMatch is Ownable, ERC23Contract {
if (balPrev == weiPaid) {
// maker wins
uint256 winnings = weiPaid.add(balPrev.div(2));
pay(_payoutAddr, winnings);
MatchmakerPrevails(master, _payoutAddr, weiPaid, balPrev, winnings);
pay(_addr, winnings);
MatchmakerPrevails(master, _addr, weiPaid, balPrev, winnings);
} else {
// master wins
pay(master, weiPaid);
MatchmasterPrevails(master, _payoutAddr, weiPaid, balPrev, weiPaid);
MatchmasterPrevails(master, _addr, weiPaid, balPrev, weiPaid);
}
}

// send proceeds
function pay(address _payoutAddr, uint256 _amount) internal {
function pay(address _addr, uint256 _amount) internal {
require(_amount > 0);

uint256 payout = _amount.mul(PAYOUT_PCT).div(100);
_payoutAddr.transfer(payout);
_addr.transfer(payout);

uint256 remainder = _amount.sub(payout);
owner.transfer(remainder);
Expand All @@ -82,11 +89,7 @@ contract EthMatch is Ownable, ERC23Contract {
}

// become the new master (and specify payout address)
function mastery(address _payoutAddr) public payable {
require(this.balance > 0); // else we haven't started yet
require(now >= startTime);
require(_payoutAddr != 0x0);

function mastery(address _addr) isValid(_addr) public payable {
uint256 weiPaid = msg.value;
require(weiPaid >= MASTERY_THRESHOLD);

Expand All @@ -95,20 +98,28 @@ contract EthMatch is Ownable, ERC23Contract {

pay(master, balPrev);

MatchmasterTakeover(master, _payoutAddr, weiPaid); // called before new master set
MatchmasterTakeover(master, _addr, weiPaid); // called before new master set

master = _payoutAddr; // set after event
master = _addr; // must be set after event logged
}

// in case it ever needs to be updated for future Ethereum releases
// in case it ever needs to be updated for future Ethereum releases, etc
function setGasReq(uint256 _gasReq) onlyOwner external {
gasReq = _gasReq;
}

// initial funding
function fund() onlyOwner external payable {
require(this.balance == msg.value); // ensures balance was 0 before this, i.e. uninitialized
require(msg.value >= MASTERY_THRESHOLD);
require(now < startTime); // otherwise can just call mastery()

// it is possible that funds can be forced in via selfdestruct, so
// just ensure balance is enough, at least after receiving this call (msg.value)
require(this.balance >= MASTERY_THRESHOLD);
}

// explicit balance getter
function getBalance() external constant returns (uint256) {
return this.balance;
}

}
16 changes: 16 additions & 0 deletions contracts/base/Lib.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
pragma solidity ^0.4.15;

/**
* @title Lib
* @dev Lib of common funcs
*/
library Lib {
// whether given address is a contract or not based on bytecode
function isContract(address addr) internal constant returns (bool) {
uint size;
assembly {
size := extcodesize(addr)
}
return (size > 1); // testing returned size "1" for non-contract accounts, so we're using that.
}
}
69 changes: 45 additions & 24 deletions flattened/flat-EthMatch.sol
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,28 @@ contract EthMatch is Ownable, ERC23Contract {

uint256 public startTime; // start timestamp when matches may begin
address public master; // current Matchmaster
uint256 public gasReq; // only a var in case it ever needs to be updated for future Ethereum releases
uint256 public gasReq; // require same gas every time in maker()

event MatchmakerPrevails(address indexed matchmaster, address indexed matchmaker, uint256 sent, uint256 actual, uint256 winnings);
event MatchmasterPrevails(address indexed matchmaster, address indexed matchmaker, uint256 sent, uint256 actual, uint256 winnings);
event MatchmasterTakeover(address indexed matchmasterPrev, address indexed matchmasterNew, uint256 balanceNew);

function EthMatch(uint256 _startTime) public {
// can be funded at init if desired
function EthMatch(uint256 _startTime) public payable {
require(_startTime >= now);

startTime = _startTime;
master = msg.sender; // initial
gasReq = 21000;
gasReq = 42000;
}

// ensure proper state
modifier isValid(address _addr) {
require(_addr != 0x0);
require(!Lib.isContract(_addr)); // ban contracts
require(now >= startTime);

_;
}

// fallback function
Expand All @@ -110,12 +120,8 @@ contract EthMatch is Ownable, ERC23Contract {
}

// make a match (and specify payout address)
function maker(address _payoutAddr) public payable {
require(this.balance > 0); // else we haven't started yet
require(msg.gas >= gasReq); // require same amount every time (overages auto-returned)

require(now >= startTime);
require(_payoutAddr != 0x0);
function maker(address _addr) isValid(_addr) public payable {
require(msg.gas >= gasReq); // require same gas every time (overages auto-returned)

uint256 weiPaid = msg.value;
require(weiPaid > 0);
Expand All @@ -125,21 +131,21 @@ contract EthMatch is Ownable, ERC23Contract {
if (balPrev == weiPaid) {
// maker wins
uint256 winnings = weiPaid.add(balPrev.div(2));
pay(_payoutAddr, winnings);
MatchmakerPrevails(master, _payoutAddr, weiPaid, balPrev, winnings);
pay(_addr, winnings);
MatchmakerPrevails(master, _addr, weiPaid, balPrev, winnings);
} else {
// master wins
pay(master, weiPaid);
MatchmasterPrevails(master, _payoutAddr, weiPaid, balPrev, weiPaid);
MatchmasterPrevails(master, _addr, weiPaid, balPrev, weiPaid);
}
}

// send proceeds
function pay(address _payoutAddr, uint256 _amount) internal {
function pay(address _addr, uint256 _amount) internal {
require(_amount > 0);

uint256 payout = _amount.mul(PAYOUT_PCT).div(100);
_payoutAddr.transfer(payout);
_addr.transfer(payout);

uint256 remainder = _amount.sub(payout);
owner.transfer(remainder);
Expand All @@ -151,11 +157,7 @@ contract EthMatch is Ownable, ERC23Contract {
}

// become the new master (and specify payout address)
function mastery(address _payoutAddr) public payable {
require(this.balance > 0); // else we haven't started yet
require(now >= startTime);
require(_payoutAddr != 0x0);

function mastery(address _addr) isValid(_addr) public payable {
uint256 weiPaid = msg.value;
require(weiPaid >= MASTERY_THRESHOLD);

Expand All @@ -164,21 +166,40 @@ contract EthMatch is Ownable, ERC23Contract {

pay(master, balPrev);

MatchmasterTakeover(master, _payoutAddr, weiPaid); // called before new master set
MatchmasterTakeover(master, _addr, weiPaid); // called before new master set

master = _payoutAddr; // set after event
master = _addr; // must be set after event logged
}

// in case it ever needs to be updated for future Ethereum releases
// in case it ever needs to be updated for future Ethereum releases, etc
function setGasReq(uint256 _gasReq) onlyOwner external {
gasReq = _gasReq;
}

// initial funding
function fund() onlyOwner external payable {
require(this.balance == msg.value); // ensures balance was 0 before this, i.e. uninitialized
require(msg.value >= MASTERY_THRESHOLD);
require(now < startTime); // otherwise can just call mastery()

// it is possible that funds can be forced in via selfdestruct, so
// just ensure balance is enough, at least after receiving this call (msg.value)
require(this.balance >= MASTERY_THRESHOLD);
}

// explicit balance getter
function getBalance() external constant returns (uint256) {
return this.balance;
}

}

library Lib {
// whether given address is a contract or not based on bytecode
function isContract(address addr) internal constant returns (bool) {
uint size;
assembly {
size := extcodesize(addr)
}
return (size > 1); // testing returned size "1" for non-contract accounts, so we're using that.
}
}

0 comments on commit 49a7648

Please sign in to comment.