2

I have write a small smart contract for lottery system. however there is problem. please see below full code.

 pragma solidity >=0.4.22 <0.6.0;

contract Lottery{
address[] public players; //dynamic array with players addresses
address public manager; //contract manager

//contract constructor, runs once at contract deployment
constructor() public{
    //the manager is account address that deploys the contract
    manager = msg.sender; 
}

//this fallback payable function will be automatically called when 
   somebody
//sends ether to our contract address
function  ()  payable  external {
    require(msg.value >= 0.01 ether);
    players.push(msg.sender); //add the address of the account that 

                              //ether to players array
   }

function get_balance() public view returns(uint){
    require(msg.sender == manager);
    return address(this).balance; //return contract balance
}

//returns a very big pseodo-random integer no.
function random() public view returns(uint256){
return uint256(keccak256(block.difficulty, block.timestamp, 
players.length));  
}

function selectWinner() public {
    require(msg.sender == manager);


    uint r = random();

    address winner;

    //a random index
    uint index = r % players.length;
    winner = players[index];

    //transfer contract balance to the winner address
    winner.transfer(address payable(this).balance);

    players = new address[](0); //resetting the players dynamic array
}
}

I am getting below issues.

1) I am getting ParserError: Expected ',' but got 'payable' winner.transfer(address payable (this).balance); on line.

    winner.transfer(address payable(this).balance);

2) There is one security issue on below code.They are not really random because the minor can modify or can set a specific time stamp.That difficulty can be known by the minor in advance and also somehow the minor could know how many players entered the lottery. so there is other way to secure the contract from above the issue as mentioned.

function random() public view returns(uint256){
   return uint256(keccak256(block.difficulty, block.timestamp, 
  players.length));
}
2

1) avoid payable in casting:

 winner.transfer(address payable(this).balance);

must be written a as:

    winner.transfer(address(this).balance);   

because casting to payable address has to be without “payable”. Your contract is of type “payable” just because a payable fallback does exist. This is enough. Refer to: https://solidity.readthedocs.io/en/v0.5.1/050-breaking-changes.html?highlight=Address%20payable

In general, if you face the problem to convert “address” to “address payable” you need to pass through uint160 (not in this case!):

// We can define a library for explicitly converting ``address``
// to ``address payable`` as a workaround.
library address_make_payable {
   function make_payable(address x) internal pure returns (address payable) {
  return address(uint160(x));
   }
}

2) your random function is no random at all for the miner of the block, who can manipulate the result slightly changing the time stamp of the block in order to obtain some desired result.This is a well known issue. You should use an external source for randomness because on Ethereum blockchain nothing can be considered truly random by definition.

  • Thanks@Rick, I have replaced winner.transfer(address payable(this).balance); with winner.transfer(address(this).balance); as you mentioned however new issue is come with "TypeError: Member "transfer" not found or not visible after argument-dependent lookup in address. winner.transfer(address(this).balance);" – Abhijeet Samanta Dec 19 '18 at 15:32
  • and also new error on line 28 come "TypeError: Wrong argument count for function call: 3 arguments given but expected 1. This function requires a single bytes argument. Use abi.encodePacked(...) to obtain the pre-0.5.0 behaviour or abi.encode(...) to use ABI encoding. return uint256(keccak256(block.difficulty, block.timestamp, players.length)); which difficult to undersand as it was compiled fine last version. which is difficult to understand as it was compiled fine old version. – Abhijeet Samanta Dec 19 '18 at 15:33
  • 1) declare address payable for type winner 2) simply use “keccak256(abi.encodePacked(block.difficulty, block.timestamp, players.lenght)) – Rick Park Dec 19 '18 at 15:42
  • Thanks@Rick, It is awesome my code is running now with help your suggestion and help. Thanks alot. – Abhijeet Samanta Dec 19 '18 at 16:04
  • Could you give some external source for randomness so I can use my code or study about it? – Abhijeet Samanta Dec 19 '18 at 16:22
1

You need to set all addresses to be payable if you want to transfer fund from contract to winner.

You have to set payable addresses like this in your smartcontract code:

1). address payable[] public players;

2). address payable winner;

3). players = new address payable[](0);

  • Thanks@Mahesh Rajput however its still giving error, which I mention to @Rick comment. – Abhijeet Samanta Dec 19 '18 at 15:38

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.