Programming Project - Phase 1

Ethereum Smart Contract Programming 201
#236

@FrankB
Thanks for reaching out!

  1. sendMoney function not working when called via main.js; its will be nice if you can upload on git and give us the link to fully understand what could be going wrong
  2. the way you have handle big number is completely fine
  3. nested sequence of promise is good work
1 Like
#237

Thank you so much, Taha, I really appreciate your quick reply.

Here is the link to my files on Github that you were asking for:

I changed the settings to “public”, so you should be able to access everything.

#238

One piece of information that I should perhaps add, Taha, is that there are no error messages concerning the failure to execute a transfer via main.js. Everything runs smoothly except for the fact that there is no transfer. I do get error messages with respect to the BigNumber problem, but with respect to the transfer problem all the various system components are altogether silent. This suggests to me that the cause of the problem may not be trivial, because trivial problems would surely be detected by some standard error check…

#239

Hi @JeffE

Good job with your Dapp, regarding your “start” method i think you can improve it.
You should use less condition and variables as it cost you gas.
As you can see bellow only one condition is needed

    //Function that chooses random number
    function random() private view returns(uint8) {
        return uint8(now % 2);
    }

    //Function that starts game, receives payment, and updates eth in contract after a win or a lose.
    function start(uint8 _headsOrTails) public payable minBet(0.1 ether) maxBet(2 ether) lessThanContractValue(contractValue) {
        require(_headsOrTails == 0 || _headsOrTails == 1, "Choice needs to be 0 or 1");
        
        if (_headsOrTails == random()){
            resultBool = true;
            contractValue -= msg.value;
            msg.sender.transfer(msg.value*2);
            if (_headsOrTails == 1){
              emit logResult("Winner", "Heads", "Heads", msg.value);
            } else {
              emit logResult("Winner", "Tails", "Tails", msg.value);                
            }
        } else {
            resultBool = false;
            contractValue += msg.value;
            if (_headsOrTails == 1){
              emit logResult("Loser", "Tails", "Heads", msg.value);
            } else {
              emit logResult("Loser", "Heads", "Tails", msg.value);                
            }
        }
    }

You can also remove the resultBool global variable and look at the result you are emitting to know the result

2 Likes
#240

Hi @Filipo24

Good job, but be careful you are not checking if there is enough ETH in you contract before placing a bet.
It will be nice to add a way to track your contract balance and to display it to the user.
You can also add a function to add funds to your contract. Because you can only send funds in the constructor so when you contract is empty you can’t play anymore.

Why did you comment the result / sendWinnings / deleteBet / insertGambler function ?

2 Likes
#241

Hi @FrankB

As you are modifying the state of your contract you should use send instead of call

 contractInstance.methods.sendMoney(customerAddress, gain).send({from: owner}).then(function(transferAmount){

There is a logical issue in your Dapp, why the random function and the sendMoney are callable only by the owner “onlyOwner” ?
If someone else than the contract owner is playing this function will be reverted .
The owner is playing then he is supposed to send ETH to a customer ?

Split your function into smaller functions, your submitAndPlay is too big and confusing, all the code inside your callback should be inside separate functions.

Finally the random function is working fine with

now()%2

As it’s taking the timestamp when a block is mined, it will produce 1 or 0.

Your way of generating a random number is interesting but you can’t let the user choose for this number.
Keep in mind that the user is able to modify the frontend code of your Dapp.

Inside the console

encoding
"RETEEssdf34fgdcccvv565(some random string)"
encoding = 0
0

Now you will pass date + 0 to your random function and do a module 10 on it. So i just have to create a javascript function which call your submitAndPlay function when i am able to win. It’s really easy to broke.
This is not secure at all, all the operation have to be done inside your smart contract and not in your frontend

An other point, why aren’t you using the accounts[0] value instead of asking the user to enter his own address ?

2 Likes
#242

@filip So I am really struggling with understanding how this .transfer() works. When you deploy a contract the address that deploys it becomes the owner. So if I wanted to just send from that owner address to whoever activates the function it should go like this, right?

function justSendingEther ()public payable{
    	uint amount= 1 ether;
	address payable receiverAdr =msg.sender;
        receiverAdr.transfer(amount);
				
}

But when I try this I keep getting this error in Remix.

“transact to Coinflip.justSendingEther errored: VM error: revert.
revert The transaction has been reverted to the initial state.
Note: The called function should be payable if you send value and the value you send should be less than your current balance. Debug the transaction to get more information.”

Any Ideas? I appreciate any help you can give! :laughing:

#243

Hi @gabba,

thanks for the feedback that is much appreciated. I will look for implementing those features during the second part of the project!

If I can ask - for checking if the balance of the contract is high enough is it better to implement it on the front end by calling getBalance and then deciding if the balance isn’t high enough send message back to user or implement in within smart contract function via require() statement at the start that checks that condition?

#244

@mikekai
We are there to help you out 24*7

  1. it is not necessary that the contract deployer will always be the contract owner.
    we can have different contract owners.
    We can do this by explicitly defining the address in the constructor.
  2. the error says you don’t have enough balance to send 1 ETH. As I can see in your function justSendingEther, the caller of the function is sending 1 ETH to himself but it seems from the error that the caller of the function does NOT have enough balance to send 1 ETH in the first place. :slight_smile:
1 Like
#245

Thank you so much, Gabba, your comments look very helpful to me. I’ll definitely try using :“send” instead of “call”. I guess I don’t really understand yet in general when one is used as compared to the other. My rationale for requiring “onlyOwner” for the sendMoney function was that without this restriction anybody can just call that function and withdraw all the money that is stored in the contract. Filip had specified the “onlyOwner” attribute for a function that withdraws the contract balance.

As far as the encoding is concerned, I completely understand that there is no encoding at all if the front end code is accessible. But it still seems to me to be the case that the nature of the hash function is such that in absence of any access to the chosen string. you can never uncover the deterministic pattern that underlies the pseudo-random output. If the string is kept secure, like a private key, (and that may be the real challenge) then there is almost no way to break the code. Maybe I’m missing something, but if my understanding of the hash function is correct, then the encoding is secure if access to the encoding string can be denied.

Thank you also for your feedback on the structure of my program. I was asking about this because I realized that things can become arbitrarily complicated as the size of a program increases. I believe that I don’t fully understand the nature and purpose the “then” statement yet. I used “then” in my program to reflect the sequencing of calls to the contract, but I noticed that there was something not right about this and that is why I was asking for comments. I have to think about this some more and I’ll let you know if I have additional questions.

#246

@Filipo24
Good question!
In traditional as well as blockchain systems, it is always better to use front-end (javascript) as much as you can to reduce proccessing load on servers and virtual machines.
That is why, javascript is so famous, because to create a lightweight application it is always best to handle most of the validation on browser side / end-user side.

1 Like
#247

Thanks for the reply @Taha that’s good to know.

I have actually implemented in the javascript logic to allow maximum bet but that value is hardcoded, so sounds like I just need to make that logic more dynamic based on the contract balance :slight_smile:

#248

@Filipo24
Yeah, you are right!
Btw, you can have look into the blockchain bootcamp from the academy, it will help you to understand and do both front-end and back-end (as a full-stack dev), ultimately to be able to create your own dApps, also you become a complete blockchain expert/developer.

1 Like
#249

It works now! And I also understand (I believe) the distinction between call and send. Use send whenever the state of the contract is changed. That makes perfect sense. I had thought that send was used when money was sent, but now I understand. Thank you so much.

I still don’t see, though, why the sendMoney function should be callable by anyone other than the owner. The contract needs to have a certain amount of money stored in it in order to be able to operate. So if, as you seemed to be saying, the front end code is completely insecure, then wouldn’t it be possible to simply issue a call to the sendMoney function and withdraw all the money from the contract? As I mentioned in my previous reply, Filip had explained in his video that a withdraw-balance function should only be accessible to the owner. So why is the game contract handled differently?

#250

Thank you for your reply!

  1. That helps! Thanks.

  2. This I am still not sure what is going on. As I am testing things in remix and deploying first with an account that has 100 ETH, and then trying the function out with another account that has 100 ETH. So I am not sure why it keeps saying that. I even went back to the original function created in the lessons to see if that works and I am still getting the same issue. Is it an issue with Remix?

1 Like
#251

Ok, I think I figured it out. When I first deploy the contract I was not adding any value to it so therefore when I tried to withdraw from it there was no balance to pull from. I found this example that really helped me to understand this. https://solidity.readthedocs.io/en/v0.6.4/common-patterns.html#withdrawal-pattern

Thanks for your help!

1 Like
#252

Thank you so much!

I appreciate the feedback. I see how this is more efficient.

1 Like
#253

Hi @FrankB

The withdrawALL function should be used by the owner only. But when you are winning a bet you should be able to transfer the amount the player bet.
You can create a mapping with the user address as a key and store the amount he bets as value.
Then when the player win you can send back the value by accessing the mapping

2 Likes
#254

Gabba, that makes sense, I didn’t think about the mapping. Thank you so much for your quick response.

And by the way, this is a very good project. I feel that I am learning a lot. I also changed the structure of my program to make it look less cumbersome. Maybe I got my first taste of what Ivan called “call-back hell” in one of his videos

Frank

#255

Another question here, haha. I am trying to double the amount that was sent as the bet but instead of sending 2 and receiving 4, only 2 are sent back. I check to make sure that the balance of the contract actually has 5 available ether but it still only returns 2 ETH. Any ideas?

contract Coinflip{

 uint public balance;

 function withdraw(uint amount) public{
  
  	uint doubleAmount = amount*2;
    balance-=doubleAmount;
    msg.sender.transfer(doubleAmount);

}

}