How do I write a withdraw function correctly in Solidity?

Question

What's wrong with this withdraw function? It keeps reverting with the error below. I have tried different methods to withdraw (see commented lines) but the error is the same.

• I'm sure I'm parsing the correct contract address. I have copy pasted it from thirdweb dashboard myself.

For the whole code, check out My Repo

Error

Error occurred while withdrawing funds Error: 


╔═══════════════════╗
║ TRANSACTION ERROR ║
╚═══════════════════╝

Reason: missing revert data in call exception; Transaction reverted without a reason string


╔═════════════════════════╗
║ TRANSACTION INFORMATION ║
╚═════════════════════════╝

from:      0x13A19933267ec307c96f3dE8Ff8A2392C39263EB
to:        0x0a1c4c84213CB67C2517d442b93A2f1B0110158D (CrowdFunding)
chain:     sepolia (11155111)
rpc:       sepolia.rpc.thirdweb.com
data:      0x388a7ec10000000000000000000000000000000000000000000000000000000000000000
method:    withdrawDonations(0)


╔═════════════════════╗
║ DEBUGGING RESOURCES ║
╚═════════════════════╝

Need helping debugging? Join our Discord: https://discord.gg/thirdweb


    at ContractWrapper.formatError (contract-publisher-7f0a5ce8.browser.esm.js:7754:12)
    at async ContractWrapper.sendTransactionByFunction (contract-publisher-7f0a5ce8.browser.esm.js:7696:17)
    at async ContractWrapper.sendTransaction (contract-publisher-7f0a5ce8.browser.esm.js:7652:18)
    at async ContractWrapper.call (contract-publisher-7f0a5ce8.browser.esm.js:7611:23)
    at async withdraw (index.jsx:104:20)
    at async handleWithdraw (WithdrawFromCampaigns.jsx:31:7)

Withdraw Function

    // Constructor
  constructor(uint256 _platformFee) payable {
        manager == msg.sender;
        platformFee = _platformFee;
        balances[msg.sender] = msg.value;
    }

    // withdraw donations
    function withdrawDonations(
        uint256 _id
    ) public authorisedPerson(_id) returns (bool) {
        (uint256 raisedAmount, uint256 fee) = calculatePlatformFee(_id);

        //balances[msg.sender] = 0; // updating adress balance before atually withdrawing to prevent re-entracy attacks.

        //send to campaign owner
        //_payTo(campaigns[_id].owner, (raisedAmount - fee));
        //payable(campaigns[_id].owner).transfer(raisedAmount - fee);

        //send to platform
        //_payTo(manager, fee);
        payable(manager).transfer(fee);

        emit Action(_id, "Funds Withdrawn", msg.sender, block.timestamp);

        return true;
    }

    // function _payTo(address to, uint256 amount) internal {
    //     require(amount > 0, "Can't send 0");
    //     (bool success, ) = payable(to).call{value: amount}("");
    //     require(success);
    // }

    /* ... */

    function calculatePlatformFee(
        uint256 _id
    ) public view returns (uint, uint) {
        uint raisedAmount = campaigns[_id].amountCollected;
        uint fee = (raisedAmount * platformFee) / 100;
        return (raisedAmount, fee);
    }

Answer 1

Assuming that your commented out code remains commented out, there are 2 likely locations for the error to be thrown:

• calculatePlatformFee(_id), and
• payable(manager).transfer(fee)

Looking at the function implementation of calculatePlatformFee, it does not look like there is anything amiss there.

That leaves payable(manager).transfer(fee).

Note that (address payable).transfer is not longer recommended for sending the native currency, and there are two alternatives to doing so:

• (address payable).send
• (address payable).call{value: msg.value}("")

The recommended option is the latter. This is what your code would look like when using that approach:

(bool success, bytes memory data) = payable(manager).call{value: fee}("");
require(sent, "transfer failed");

If you try this approach, that might fix your issue. But if it does not, it should reveal more information to figure out the underlying reason. Instead of getting this message, which is basically no reason,

Reason: missing revert data in call exception; Transaction reverted without a reason string

... you will get "transfer failed" in the error message. If this happens, then you can isolate your problem, and confirm if indeed the transfer was the issue.

There are several possible reasons that the transfer might fail:

(1) The manager address is a smart contract address (not an EOA address), and it rejects transfers. This can happen whether or not the smart contract defines receive or fallback functions, and whether or not they have the payable modifier.

(2) The value of fee is calculated such that it is less than the balance of the smart contract

For (1): This is correct, and failing the transaction is indeed the correct outcome/ behaviour for your smart contract to do.

For (2): This is incorrect, and indicates that your smart contract implementation may have errors. Whether or not this is the case, I'd recommend that you employ defensive programming here, and add a require statement to ensure that the pre-requisites are met, after calculating fee, and before performing the transfer.

Your function implementation (without any of the currently commented out code) would look something like this:

    function withdrawDonations(
        uint256 _id
    ) public authorisedPerson(_id) returns (bool) {
        (uint256 raisedAmount, uint256 fee) = calculatePlatformFee(_id);

        require(fee <= (address(this).balance), "fee in excess of balance");

        payable(manager).transfer(fee);
        emit Action(_id, "Funds Withdrawn", msg.sender, block.timestamp);
        return true;
    }

---

NOTE: I've edited your question to include the function implementation code of calculatePlatformFee, as that is important to assess where the error is.