1
function createCampaign(uint deposit, address ll) public payable{
    dpst = deposit;
    require(msg.value > (40*dpst)/100 wei);
    address newCampaign = address((new Campaign).value(msg.value)(deposit, ll, token_address, 1));
    token_instance.setAllowance(dpst, token_holder, newCampaign);
    deployedCampaigns.push(newCampaign);     
}

Here's a piece of my code. In this payable function minimum amount of wei to be sent should be at least 40% of deposit. Is the require function implemented correctly?

1

Change this:

require(msg.value > 40 * dpst / 100)

To this, in order to avoid precision-loss:

require(msg.value * 100 > 40 * dpst)

Then to this, in order to avoid overflow:

require(msg.value.mul(100) > dpst.mul(40))

Then to this, in order to reduce the chances of failure:

require(msg.value.mul(10) > dpst.mul(4))

Then to this, in order to reduce the chances of failure even more:

require(msg.value.mul(5) > dpst.mul(2))

BTW, at least 40% imples that you can (and should) use >= instead of >.

0

Yes, that works. However, there are a few things to note.

1) You should use OpenZeppelin's Safemath for those operations in order to avoid overflow. Your code would look as follows:

function createCampaign(uint deposit, address ll) public payable{
    dpst = deposit;
    require(msg.value > (40.add(dpst)).div(100 wei));

2) You do not need to specify wei. You can simply divide by 100.

3) You do not need to set dpst = deposit. You can simply use deposit throughout the function.

  • But I'm getting error. transact to CampaignFactory.createCampaign errored: VM error: revert. revert The transaction has been reverted to the initial state. Note: The constructor should be payable if you send value. Debug the transaction to get more information. – Rahul Savsani Jan 28 at 6:30
  • Usage of add is wrong here. It should be mul. Usage of div is not really needed (if I were to use your words, then "You can simply divide by 100"). – goodvibration Jan 28 at 6:34
  • And as you might have noticed, dpst is not declared in the function, implying that it is a state variable. So possibly this dude needs to record the last deposit used (hence the assignment dpst = deposit). – goodvibration Jan 28 at 6:37
  • But although I've declared my function as payable I'm not able to send ether/wei with the function call. Is it a problem with remix ide or what! – Rahul Savsani Jan 28 at 6:37

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.