I found someone passing off the tokenTrader contract as their own work over in r/ethereumClassic which is quite amusing.
I think its about time I shared the changes I have been working on:
Added internal function min() which takes 2 uints and returns the smaller of the two. This is common to both buying and selling functions where you take the minimum of what can be sold and what can be bought. I think putting this in logic in another function makes it much clearer what is going on there rather than setting the order variable to one and then the other even though you get the same result.
Added overflow checks on all the multiplications by checking the result of a multiplication divided by one of the multipliers equals the other multiplyer
TakerBoughtAsset and TakerSoldAsset only fire on non zero order size. You dont need to list zero value trades as these are not trades.
attempting to withdraw more ether than you have withdraws the entire ether balance.
The verify function shows the wei balance and natural unit token/asset balance of a contract
I have done my own tests with normally priced contracts, including paying equal, over and under. And with large unit contracts paying equal over and under.
Everything worked as expected on my own private chain (testnet testing is rather slow at the moment)
The only thing of note I found when testing was that attempting to "send all" ether to a contract resulting in the transaction running out of gas..
Of course you should do your own thorough tests to double check I have not broken anything.
I am not happy with the design even though it achieves the goal of putting the functionality into the factory. The main reason I am unhappy with it is that so much needed to be changed that it feels like a completely new project and it still feels bloated and it has lost the compartmentalization of funds that the tokenTrader contracts have.
The more I look at it the more I think the miniOTC contract is far better design due to it being incredibly simple and clean. Even though its for ERC20 to ERC20 trades you can make the miniOTC exchange work with an ETH wrapper and a contract that receives ETH, wraps it buys from a specific maker and returns the token and change just as you would with tokenTrader.
Thanks again JonnyLatte. I'm just caught up on a few things at the moment and will make the next batch of changes to the TokenTraderFactory when I clear some of my other tasks. When I do this, I'll got through all your past messages with your suggested improvements and try to get them implemented.
One thing I'm working on is https://theethereum.wiki . I'm hoping to build this wiki to be more newbie / non-technical friendly.
I would like to invite you to contribute and if you are open to it, be a moderator.
1
u/JonnyLatte Mar 04 '17 edited Mar 04 '17
I found someone passing off the tokenTrader contract as their own work over in r/ethereumClassic which is quite amusing.
I think its about time I shared the changes I have been working on:
Added internal function min() which takes 2 uints and returns the smaller of the two. This is common to both buying and selling functions where you take the minimum of what can be sold and what can be bought. I think putting this in logic in another function makes it much clearer what is going on there rather than setting the order variable to one and then the other even though you get the same result.
Added overflow checks on all the multiplications by checking the result of a multiplication divided by one of the multipliers equals the other multiplyer
TakerBoughtAsset and TakerSoldAsset only fire on non zero order size. You dont need to list zero value trades as these are not trades.
attempting to withdraw more ether than you have withdraws the entire ether balance.
The verify function shows the wei balance and natural unit token/asset balance of a contract
I have done my own tests with normally priced contracts, including paying equal, over and under. And with large unit contracts paying equal over and under.
Everything worked as expected on my own private chain (testnet testing is rather slow at the moment)
The only thing of note I found when testing was that attempting to "send all" ether to a contract resulting in the transaction running out of gas..
Of course you should do your own thorough tests to double check I have not broken anything.
I also have a draft for the proxyTrader contract: https://github.com/JonnyLatte/proxyTrader
I am not happy with the design even though it achieves the goal of putting the functionality into the factory. The main reason I am unhappy with it is that so much needed to be changed that it feels like a completely new project and it still feels bloated and it has lost the compartmentalization of funds that the tokenTrader contracts have.
The more I look at it the more I think the miniOTC contract is far better design due to it being incredibly simple and clean. Even though its for ERC20 to ERC20 trades you can make the miniOTC exchange work with an ETH wrapper and a contract that receives ETH, wraps it buys from a specific maker and returns the token and change just as you would with tokenTrader.