2

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?

  • 2
    You by the least need to add require(msg.value >= 194740000000000) at the beginning of the constructor. – goodvibration Jun 13 at 12:52
4

If you deploy your contract with some msg.value >= 194740000000000, then everything should work fine.

Let's analyze what might happen if you deploy your contract with some msg.value < 194740000000000.

The result of msg.value - 194740000000000 stored into a uint variable will be very large.

More precisely, this variable will be equal to 2**256 + msg.value - 194740000000000.

You might think to yourself 'well, I did not transfer this huge amount upon deployment, so the transfer operation which follows will fail, the entire transaction will be reverted, and I am therefore safe from any issues in this case'.

However, it is technically possible to pass ether to your contract before it is deployed.

First, you can determine the address of your contract before deploying it.

It's the last 40 bits of the hash (sha256 output) of the string concatenated from:

  • The address of the account which you are going to deploy it with
  • The nonce of the account which you are going to deploy it with

Then, you can send ether to this address, and when you later deploy your contract, it will already own the amount which you have previously transferred to its address.

However, even if you transfer to this address the maximum possible amount (2**256 - 1 wei), the contract deployment will be reverted and nothing will get lost, because the two amounts which will be transferred upon construction are:

  • 2**256 + msg.value - 194740000000000 wei
  • 194740000000000 wei

The sum of these two amounts is larger than 2**256 - 1 wei, but you only have 2**256 - 1 in your contract.

So technically speaking, your code is safe.

However, since you probably intended for it to be deployed only with some msg.value >= 194740000000000, it would be best practice to verify this condition beforehand:

require(msg.value >= 194740000000000, "not enough ether");

Note that it will yield the exact same behavior, i.e., if msg.value < 194740000000000 then the transaction will end with a REVERT opcode whether you add this verification or not.

However, it is probably best to add it there, so that the reader will not have any doubts about the behavior of your contract.

1

Mainly you need to check for integers overflow and underflow, there is contract called safeMath, please google it and use it for mathematical calculations.

And second why would you want to create new contract for every ether movement? Contract constructor is mainly used to setup variables on contract deploy. Better practical solution is to deploy the contract once and move the your ether sending logic into new method 'sendingEther' or w/e name you wish accepting 2 parameters - sender and receiver addresses. Then when for example person A wants to send 0.5 ethers to person B they don't have to create new contract, but simply call one of your existing methods in your already deployed contract (your wallet users will pay a lot less gas cost this way).

And third today 0.05$ are 194740000000000 wei, but cryptocurrencies are not stable at all, so you need to find way how to update this number frequently.

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.