Skip to content

Commit

Permalink
add constructor, FE dependencies and structure
Browse files Browse the repository at this point in the history
  • Loading branch information
Omar Hafez committed Jan 28, 2018
1 parent 813687e commit f7e5c94
Show file tree
Hide file tree
Showing 7 changed files with 109 additions and 4 deletions.
29 changes: 29 additions & 0 deletions contracts/Splitter.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,27 @@ contract Splitter is Ownable {
address public carolAddress;
uint aliceContribution;

function Splitter(address _bob, address _carol)

This comment has been minimized.

Sign in to view

Copy link
@xavierlepretre

xavierlepretre Jan 31, 2018

Due to how the EVM works, a missing parameter is in fact 0. Are you comfortable with your addresses being 0?
public
{
bobAddress = _bob;
carolAddress = _carol;
}

function setBobAddress(address _address)
external
onlyOwner
{

This comment has been minimized.

Sign in to view

Copy link
@rob-Hitchens

rob-Hitchens Jan 31, 2018

Consider returns(bool success) ... return true.

My habit (it's not universal) is to always return something and it is useful in some cases.
bobAddress = _address;
}

This comment has been minimized.

Sign in to view

Copy link
@rob-Hitchens

rob-Hitchens Jan 31, 2018

You should emit an every for every important state change that unfolds in the contract. Try to make is possible, in theory, to reconstruct the entire state history from logs (all the transactions that ever happened in the order they happened).

This is a good go-to default strategy because logs are super useful for clients that are interested in the contract.

function setCarolAddress(address _address)
external
onlyOwner
{
carolAddress = _address;
}

function splitFunds(uint _aliceContribution)

This comment has been minimized.

Sign in to view

Copy link
@rob-Hitchens

rob-Hitchens Jan 31, 2018

You're confusing data with value. Don't rely on Alice to tell you how much money she sent. That's available as msg.value and you have strong assurance you can trust it. It might be useful in another use case to add extra parameters to a payable function for something like function buyProduct(bytes32 buyWhat) payable ... but in this case, we want the contract to fairly split the funds and there isn't anything else we need to know from Alice.
public
payable
Expand All @@ -29,4 +50,12 @@ contract Splitter is Ownable {
return (aliceContribution == this.balance);

This comment has been minimized.

Sign in to view

Copy link
@rob-Hitchens

rob-Hitchens Jan 31, 2018

This isn't going to hold because the contract should actually be out of money if your split works.
}

function kill()
public
{
if (msg.sender == owner) {
selfdestruct(owner);

This comment has been minimized.

Sign in to view

Copy link
@rob-Hitchens

rob-Hitchens Jan 31, 2018

This is a nasty solution that's almost always sub-optimal. It creates sinks where money can go it but no function exists to pull it out. It's almost always better to have a run switch / pause switch and a modifier onlyIfRunning. Thay way, if the owner decides to halt the contract, all the functions will start throwing/reverting and, importantly, returning funds sent.
}
}

}
14 changes: 14 additions & 0 deletions dist/index.html
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<link href='https://fonts.googleapis.com/css?family=Open+Sans:400,700' rel='stylesheet'>
<title>Splitter Contract</title>
</head>
<body>
<div id="root"></div>

<script src="build.js"></script>
</body>
</html>
17 changes: 16 additions & 1 deletion package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,20 @@
"bugs": {
"url": "https://github.com/hafezoa/splitter/issues"
},
"homepage": "https://github.com/hafezoa/splitter#readme"
"homepage": "https://github.com/hafezoa/splitter#readme",
"devDependencies": {
"babel-core": "^6.26.0",
"babel-loader": "^7.1.2",
"babel-preset-es2015": "^6.24.1",
"babel-preset-react": "^6.24.1",
"babel-preset-stage-2": "^6.24.1",
"css-loader": "^0.28.9",
"json-loader": "^0.5.7",
"react": "^16.2.0",
"react-dom": "^16.2.0",
"style-loader": "^0.20.1",
"truffle": "^4.0.5",
"web3": "^0.20.4",
"webpack": "^3.10.0"
}
}
4 changes: 4 additions & 0 deletions src/css/index.css
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
body{
font-family: 'open sans';
margin: 0;
}
16 changes: 16 additions & 0 deletions src/js/index.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import * as React from 'react';
import ReactDOM from 'react-dom';
import Web3 from 'web3';
import './../css/index.css';

class App extends React.Component {

render() {
<div>Hello Splitter</div>
}
}

ReactDOM.render(
<App />,
document.getElementById('root')
)
8 changes: 5 additions & 3 deletions test/splitter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,15 @@ var Splitter = artifacts.require("./Splitter.sol");

contract("Splitter contract", (accounts) => {
let ownerAlice = accounts[0];
let Bob = accounts[2];
let Carol = accounts[3];
let _bob = accounts[1];
let _carol = accounts[2];

beforeEach(() => {
return Splitter.new({ from: owner })
return Splitter.new(_bob, _carol, { from: owner })
.then(instance => {
contract = instance;
bobAddress = _bob;
carolAddress = _carol;

This comment has been minimized.

Sign in to view

Copy link
@xavierlepretre

xavierlepretre Jan 31, 2018

Not sure why you would set those, they become global variables since no scope has been declared.
});
});

Expand Down
25 changes: 25 additions & 0 deletions webpack.config.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
const path = require('path')
module.exports = {
entry: path.join(__dirname, 'src/js', 'index.js'), // Our frontend will be inside the src folder
output: {
path: path.join(__dirname, 'dist'),
filename: 'build.js' // The final file will be created in dist/build.js
},
module: {
loaders: [{
test: /\.css$/, // To load the css in react
use: ['style-loader', 'css-loader'],
include: /src/
}, {
test: /\.jsx?$/, // To load the js and jsx files
loader: 'babel-loader',
exclude: /node_modules/,
query: {
presets: ['es2015', 'react', 'stage-2']
}
}, {
test: /\.json$/, // To load the json files
loader: 'json-loader'
}]
}
}

0 comments on commit f7e5c94

Please sign in to comment.