Does Solidity function with payable modifier allow to perform msg.sender.call{}

Question

Found out that there are error in my own code. Answered my own question

I am beginner of Solidity programming, and came across with reentrancy risk. I have tested script below in remix and it works per expectation. https://solidity-by-example.org/hacks/re-entrancy/

contract EtherStore {
    // Withdrawal limit = 1 ether / week
    uint constant public WITHDRAWAL_LIMIT = 1 ether;
    mapping(address => uint) public lastWithdrawTime;
    mapping(address => uint) public balances;

    function deposit() public payable {
        balances[msg.sender] += msg.value;
    }

    function withdraw(uint _amount) public {
        require(balances[msg.sender] >= _amount);
        require(_amount <= WITHDRAWAL_LIMIT);
        require(block.timestamp >= lastWithdrawTime[msg.sender] + 1 weeks);

        (bool sent, ) = msg.sender.call{value: _amount}("");
        require(sent, "Failed to send Ether");

        balances[msg.sender] -= _amount;
        lastWithdrawTime[msg.sender] = block.timestamp;
    }

    // Helper function to check the balance of this contract
    function getBalance() public view returns (uint) {
        return address(this).balance;
    }
}

The attack contract stop working (return false) the moment msg.sender.call function is included at the end of deposit function (see below code)
However, if the new deposit function is called directly via remix, the msg.sender.call function will go through smoothly without any exceptions

contract EtherStore {
  // Withdrawal limit = 1 ether / week
  uint constant public WITHDRAWAL_LIMIT = 1 ether;
  mapping(address => uint) public lastWithdrawTime;
  mapping(address => uint) public balances;

  function deposit() public payable {
      balances[msg.sender] += msg.value;

      (bool sent, ) = msg.sender.call{value: 1 ether}("");
      require(sent, "Failed to send Ether");
  }

  function withdraw(uint _amount) public {
      require(balances[msg.sender] >= _amount);
      require(_amount <= WITHDRAWAL_LIMIT);
      require(block.timestamp >= lastWithdrawTime[msg.sender] + 1 weeks);

      (bool sent, ) = msg.sender.call{value: _amount}("");
      require(sent, "Failed to send Ether");

      balances[msg.sender] -= _amount;
      lastWithdrawTime[msg.sender] = block.timestamp;
  }

  // Helper function to check the balance of this contract
  function getBalance() public view returns (uint) {
      return address(this).balance;
  }
}

May i check if anyone know what is causing the behavioural differences in both function call.

My objective of testing the new deposit() function is to check if attacker can launch direct attack using single deposit() call, with the assumption that deposit() function will transfer small balance back to msg.sender.

According to my understanding, the expected behavior should be calling the msg.sender fallback() function. But this doesnt happen in the test (remix), instead it returns false straight away

Thanks in advance

**Found out that there are bugs in my logic, thanks for the answe

Answer 1

The two deposit functions are

old

 function deposit() public payable {
    balances[msg.sender] += msg.value;
}

new

    function deposit() public payable {
      balances[msg.sender] += msg.value;

      (bool sent, ) = msg.sender.call{value: 1 ether}("");
      require(sent, "Failed to send Ether");
  }

In the second function you add a check which prevents against re-entrancy attacks with custom fallback function on msg.sender.

{value: 1 ether}("");

https://ethereum.stackexchange.com/questions/42521/what-does-msg-sender-call-do-in-solidity

https://consensys.github.io/smart-contract-best-practices/known_attacks/

https://quantstamp.com/blog/what-is-a-re-entrancy-attack