CORIONplatform / solidity

GNU General Public License v3.0
12 stars 9 forks source link

schellingLight.sol -transferEvent handler #159

Open gundas opened 7 years ago

gundas commented 7 years ago

Since schelling is a contract, when transferring COR to it the 'Token._transferToContract(...)' function will be used - it expects the receiving contract to implement the receiveToken function (and it fails in unpredictable ways if that function is not available - see issue 119 ).

It could work only if all of the COR transfers to schelling contract are made using only the Token.transferFromByModule(..) function (since it bypasses _transferToContract(...) ). But this looks kind of fragile and inconsistent.

Also, the schelling.transferEvent(...) function does not handle the case if/when the schelling contract's balance is being decreased, so potentially the rewards variable could become bigger than the balanceOf(schelling) and the newSchellingRound(...) function will fail forever until the contract is replaced.

iFA88 commented 7 years ago

Until the new schelling token transfer to the schelling contract is not allowed. The Token._transferToContract will be throw here: https://github.com/CORIONplatform/solidity/blob/rc/token.sol#L392

We dont need that. Because when a new Schelling round is announced, we send the reward to an address, and we set that to 0. https://github.com/CORIONplatform/solidity/blob/rc/schellingLight.sol#L67-L68 This means, the schelling contract will receive the tokens and the amount will be to the reward added: https://github.com/CORIONplatform/solidity/blob/rc/schellingLight.sol#L25 Which situation should be the reward greater than the Token.balanceOf(schellingAddress) ?

gundas commented 7 years ago

Which situation should be the reward greater than the Token.balanceOf(schellingAddress) ?

Currently there is not code which would do it - it requires using Token.transferFromByModule(..) to transfer some COR out of Schelling.

But I really do not understand the business case - what is the purpose of transferring reward to escrow on each new schelling round? Wouldn't it be easier just to implement a separate method which transfers entire schelling contract COR balance to the escrow'? Then there would be no need to keep the track on thereward`.

iFA88 commented 7 years ago

This is only temporally until we start the normal Schelling. The amount on escrow address will be back into the real Schelling round rewards.