1
function () public payable {
    uint tokens;
    
    /// this math isnt safe 
    tokens = msg.value * 10;
    
    /// this didnt give us the address we needed
    address tokenOwner;

    
    balances[msg.sender] = safeAdd(balances[msg.sender], tokens);
    _totalSupply = safeAdd(_totalSupply, tokens);
    emit Transfer(address(0), msg.sender, tokens);
    tokenOwner.transfer(msg.value);

    }

im trying to grok how to write a function where the contract takes ETH and gives tokens back to the sender of the ETH and send the ETH to the contract owner.

I have to problems

  1. It sent the wrong amount - i sent .1 ETH to the contract and it send me like 10,000,000,000,000 coins or something.

  2. The tokenOwner never got the .1 ETH sent -- neither did the contract.

Any ideas where I went wrong?

1 Answer 1

2

You should not adjust the scale like this. msg.value is already scaled properly.

tokens = msg.value * 10;

This is useless. A balances mapping is usually used to track liabilities owed by the contract to other users. But you forward the funds immediately so you have no need for this.

balances[msg.sender] = safeAdd(balances[msg.sender], tokens);

It's not clear where tokenOwner came from.

tokenOwner.transfer(msg.value);

Try

contract Thing {

  address payable owner;

  function forwarder() public payable {
    owner.transfer(msg.value);
  }
}

UPDATE

Owner is 0x0... because we didn't set it to anything. You could go nuts and import transferable ownership and make the owner the destination.


import "@openzeppelin/contracts/access/Ownable.sol";

contract Thing is Ownable {

  function forwarder() public payable {
    payable(owner()).transfer(msg.value); // Ownable sets it in the constructor and presents an owner() function 
  }
}

Hope it helps.

4
  • this helped a lot! was abgle to fix the token amount, and the code above helped in not getting a compile error on owner.transfer(msg.value) however the ETH seems to go to 0x00000000000000000000 and not back to the token owner contract Apr 15, 2021 at 20:02
  • You have to extrapolate off of the example. Set owner to where it should go. I assume not msg.sender but also not 0.0... as in my little sketch. That's what I meant by "It's not clear where tokenOwner came from" - I just don't know where the correct value comes from. Possibly set it up in the constructor with Ownable. Apr 15, 2021 at 20:05
  • Is it ever bad form to code in the address implicitly into the .transfer function? Apr 15, 2021 at 20:27
  • I would say "usually." There are practical reasons why it gets in the way. There are exceptions. An argument in favor of it would be gas optimization. IMHO, constants should be declared up front rather than deep in the code. Thanks for accepting my answer. Apr 15, 2021 at 20:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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