Is my Solidity code correct and safe to use

Question

I am very new using Solidity to create Ethereum contracts and would like to know if the contract I have created is first correct and safe to use.

What I am doing in this contract is allowing the user to send some ETH from their wallet to a friends wallet but also take a $0.05 fee (194740000000000 wei at time of writing) for using my services.

Now when testing the contract using remix all works fine, but as I am new to solidity contracts I just wanted to get some expert opinions to make sure I am doing everything correctly and safety.

My contract code is:

 pragma solidity ^0.5.1;

 contract Forwarder {

 address payable public receiversAddress = 0x14723A09ACff6D2A60DcdF7aA4AFf308FDDC160C;
 address payable public feeAddress = 0xdD870fA1b7C4700F2BD7f44238821C26f7392148;

 constructor() payable public {
   uint amountToSendReceiver = msg.value-194740000000000;
   receiversAddress.transfer(amountToSendReceiver);
   feeAddress.transfer(194740000000000);
  }

 }

As mentioned everything works fine in remix, but is my contract code correct and safe to use? There is no way to use the contract after to maybe steal funds or have any vulnerabilities?

Yegor Zaremba Yegor Zaremba · Accepted Answer · 2019-06-18T06:58:16

Yes, your code is pretty safe. The only thing I would like to add is line with require condition for code maintainability

And if your vars are not changeable, its better to use constant to make your contract cheaper for deploy ;)

Functions can be declared constant in which case they promise not to modify the state.

pragma solidity ^0.5.1;

contract Forwarder {

   address payable public constant receiversAddress = 0x14723A09ACff6D2A60DcdF7aA4AFf308FDDC160C;
   address payable public constant feeAddress = 0xdD870fA1b7C4700F2BD7f44238821C26f7392148;
   uint256 constant feeAmount = 194740000000000;

   constructor() payable public {
      require(msg.value >= feeAmount); // for maintainability, your tx will be reverted anyway, just to show why
      feeAddress.transfer(feeAmount); // transfer fee
      receiversAddress.transfer(address(this).balance); // transfer remaining amount of msg.value
   }

}

Is my Solidity code correct and safe to use

1 Answers