0

I have written a very basic smart contract

  • It can accept ethers using a deposit function

  • ppl can withdraw from their balance using withdraw function

Everything works fine except withdraw function, when I test the contract over Ganance and Remix..

Screenshots are below

Contract Function

Error in Remix

enter image description here

Error while tried with Ganance

Test Case while tried with Ganance

Deposit function

am I missing any thing here?

Adding the Code blocks

pragma solidity >= 0.4.25;

contract Bank {
    uint256 private bankLiquidity;       // Balance of whole contract
    address public owner;
    mapping(address => uint256) private customerBalance;
    event fallbackCalled(address, uint256);
    event deposit(address, uint256);
    event withdrawal(address, uint256);

    constructor() public {
        owner = msg.sender;
    }

    modifier ownerOnly() {
        require(msg.sender == owner, "message.sender is not the bank owner");
        _;
    }

    /** Customer Deposit payable function */
    function depositFunds(uint256 _value) external payable returns(bool){
        require(_value > 0, "values not greater then zero");
        customerBalance[msg.sender] += _value;
        bankLiquidity += _value;
        this.Balance = msg.value;
        emit deposit(msg.sender, _value);
        return true;
    }

    /** Pull and Push / Check Effects and Interactions patterns */
    function withdrawFunds(uint256 _value) public payable {
        // Checks 
        require(_value > 0, "a non Positive value");                                            
        // Checked the input values 
        require(_value <= customerBalance[msg.sender], "Insufficent customerBalance");  // Validate the Contract state 

    // Effects 
    customerBalance[msg.sender] -= _value;                                          
    // Update the state 
   // bankLiquidity -= _value;
    // Interactions
    /** Transfer is better way to send funds as it is equivalant to required(msg.sender.send(_value)) 
        as well with limit quota of gas allocation 2300, it prevents for reentracny attack as well refunds any
        remaining unused gas in case of any exceptions */
        msg.sender.transfer(_value);                                                                
        // Push funds
        emit withdrawal(msg.sender, _value);
    }

    /** Fetch Bank Liquidity */
    function getBankLiquidity() external view ownerOnly returns(uint256) {
        return bankLiquidity;
    }

    /** Fetch Customer Balance */
    function getCustomerBalance() public view returns(uint256) {
        return customerBalance[msg.sender];
    }

    function () public payable {
        emit fallbackCalled(msg.sender, msg.value);
    }
}

Working Contract after fixes

pragma solidity >= 0.4.25;

/**

1 - Bank will hold total funds and only owner will able to get the details by call getter getBankLiquidity() 2 - Customers will have account balance 3 - Deposit payable function will work for everyone in Public 4 - Withdraw is a Pull and Push function and anyone with valid balance should able to withdraw funds 5 - Customers should able to check their balances using getCustomerBalance() 6 - There is a payable function */

contract Bank { address public owner; mapping(address => uint256) private customerBalance; event fallbackCalled(address, uint256); event deposit(address, uint256); event withdrawal(address, uint256);

constructor() public payable {
    owner = msg.sender;
    customerBalance[msg.sender] += msg.value;
}

modifier ownerOnly() {
    require(msg.sender == owner, "message.sender is not the bank owner");
    _;
}

/** Customer Deposit payable function */
function depositFunds() external payable returns(bool){
    require(msg.value > 0, "values not greater then zero");
    customerBalance[msg.sender] += msg.value;
    emit deposit(msg.sender, msg.value);
    return true;
}

/** Pull and Push / Check Effects and Interactions patterns */
function withdrawFunds(uint256 _value) public payable {
    require(_value <= customerBalance[msg.sender], "account balance is low");
    customerBalance[msg.sender] -= _value;                                          // Update the state 
    msg.sender.transfer(_value);                                                    // Push funds
    emit withdrawal(msg.sender, _value);
}

/** withdraw all funds and distory the contract*/
function withdrawFundsEmergency() external ownerOnly {
    require(address(this).balance > 0, "account balance is low");
    msg.sender.transfer(address(this).balance);                                    // Push funds
    emit withdrawal(msg.sender, address(this).balance);
    selfdestruct(address(this));
}

/** Fetch Bank Liquidity */
function getBankLiquidity() external view ownerOnly returns(uint256) {
    return address(this).balance;
}

/** Fetch Customer Balance */
function getCustomerBalance() public view returns(uint256) {
    return customerBalance[msg.sender];
}

function () external payable {
    emit fallbackCalled(msg.sender, msg.value);
}

}

  • 5
    Have you ever heard of text copy-pasting? It's a brand new technology, aimed at replacing the technology of taking a photo of your monitor with a Polaroid camera. You might wanna consider switching to that. – goodvibration Feb 7 at 15:49
  • @goodvibration lol my Bad! – Devraj Singh Rawat Feb 8 at 4:46
1

There is no evident correlation between _value and the possible ethers present in the contract balance.

You should at least check the true balance of the contract for being >= _value.

_value <= customerBalance[msg.sender] 

could be, or not, adequate and correctly calculated. How can I check this from your screenshots?

Begin commenting your requires out until found that which revert, then reason about it.

(Ps: @goodvibration is right in asking you to text/paste instead of screenshot your monitor)

  • Hi @goodvibration ... I have used that cutting edge technology and edited the question with addition details .. Pls suggest.. – Devraj Singh Rawat Feb 8 at 4:56
  • I guess I have understood your pointer for contract ether value .. actually I had used the bankLiquidity variable that must hold the all incoming ethers But I think there must be an internal variable which I need to update if I m thinking in right direction... Pls suggest contract Bank { uint256 private bankLiquidity; // Balance of whole contract address public owner; mapping(address => uint256) private customerBalance; – Devraj Singh Rawat Feb 8 at 5:01
  • 1
    Just compare contract balance given by with your evaluation with address(this).balance. that it is NOT a variable to write! You can only read it! Your deposit function is wrong. As I said you should comment out the two "require" you have in your withdraw function and understand what is that which make the function revert. After it you can understand how to modify the code. By the way: you are required to upvote useful comments and to mark as answered your question when done. – Rick Park Feb 8 at 8:53
1

Looked at your contract in remix. Could not reproduce error in withdrawFunds. Its working as intended. Apart from that I have some suggestions for your contract.

  1. No need to maintain bankLiquidity variable. You can always get your contract total balance with address(this).balance.
  2. withdrawFunds need not be marked as payable.
  3. depositFunds does not need the parameter _value. You can get incoming ethers in units of wei with msg.value.
0

Worked with below fixes function withdrawFunds(uint256 _value) public payable { require(_value <= customerBalance[msg.sender], "account balance is low"); customerBalance[msg.sender] -= _value; // Update the state msg.sender.transfer(_value); // Push funds emit withdrawal(msg.sender, _value); }

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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