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?

2 Answers 2

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.

4
  • 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. Commented Jan 28, 2019 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"). Commented Jan 28, 2019 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). Commented Jan 28, 2019 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! Commented Jan 28, 2019 at 6:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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