0

Look at this very basic solidity function:

function myfunction() external {
  address payable address1 = 0x....;
  address payable address2 = 0x....;
            
  address1.transfer(1 ether);
  address2.transfer(1 ether);
}

Let's suppose the second transfer will fail. There are many reasons for that (insuffisant funds in the contract's balance, ). I want to be sure the first transfer will be cancelled in this case.

I suppose a solidity function is "atomic". I mean if something fails in the function, every operation made in the function is canceled.

I have test and i have see the first transfer is cancelled. I am happy to see that.

But somebody told me what i was doing is a bad practice. He told me i should write a "pull-payment" pattern. My question is why should i do that.

Is there some cases where a solidity function is not "atomic" ?

Thanks a lot

*** EDIT ***

Here is what i mean by "pull payment":

https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/payment/PullPayment.sol

strategy, where the paying contract doesn't interact directly with the receiver account, which must withdraw its payments itself. Pull-payments are often considered the best practice when it comes to sending Ether

4
  • Is there some cases where a solidity function is not "atomic" ? - no. – goodvibration Dec 17 '20 at 19:48
  • Okay so what is the value add to work with pull payment pattern ? – Bob5421 Dec 17 '20 at 20:19
  • Find out what pull payment pattern means, and post it as part of your question. It is not a term that I've come across. – goodvibration Dec 17 '20 at 20:23
  • i have edit my question to explain pull payment – Bob5421 Dec 17 '20 at 21:19
0

They probably meant a pattern like this:

mapping(address => uint256) public withdrawableBalance;

function myfunction() external {
  address payable address1 = 0x....;
  address payable address2 = 0x....;
            
  withdrawableBalance[address1] += 1 ether;
  withdrawableBalance[address2] += 1 ether;
}

function withdraw() external {
  uint256 bal = withdrawableBalance[msg.sender];
  withdrawableBalance[msg.sender] = 0;
  msg.sender.transfer(bal);
}

This pattern is useful because:

  • It prevents attacks where for example address1 is a contract that can be told to reject ETH transferrals, thereby also causing payments to another user address2 to fail.

  • If address1 or address2 is a smart contract, it is able to know where it got the ETH from and perform some action based on that. If you simply .transfer( ETH to a contract, that contract has no ability to even store the address they received it from.

  • It reduces the gas cost of myfunction() and shifts that cost to the receiver of the ETH (the caller of withdraw())

  • It can reduce the total gas cost if myfunction() is called multiple times, because the balance is added up and can be transferred all at once using withdraw()

4
  • Thanks a lot for your answer but there is something i do not understand when you say: "If address1 or address2 is a smart contract, it is able to know where it got the ETH from and perform some action based on that.": I think in all case it will be able to know where it got the ETH from because Ethereum is a public blockchain and it is possible to follow transactions. So i do not understand. And can you give me an example about what kind of (nasty ?) actions can be perform in this case ? Thanks – Bob5421 Dec 23 '20 at 18:12
  • @Bob5421 Any human can follow all transactions, but a smart contract can't necessarily do so. The EVM does not have any opcodes to read transactions. – Jesbus Dec 23 '20 at 18:18
  • Thanks but what kind of actions can be made ? reentrancy ? – Bob5421 Dec 23 '20 at 18:19
  • @Bob5421 Re-entrancy only if your contract is not written to handle it correctly. Besides that, if for example address2 is a contract that reverts when it receives ETH; the entire call to myfunction() will revert. So one address can prevent the other address from receiving their ETH. – Jesbus Dec 23 '20 at 18:22
0

Just to amplify @Jesbus's answer.

Yes, it is an atomic transaction - it fails entirely if any part of it fails.

No, it is not a secure pattern. As a general heuristic, avoid interactions with more than one "untrusted" address at a time. Since msg.sender could be anyone, that's one. You could send money to msg.sender without going over-budget, i.e. the "withdrawal pattern."

That pattern prevents interference because Alice cannot interfere with Bob. More generally, it protects the contract from DoS attacks or accidental effects.

Accidents are probable. If Bob is a contract and the contract has a default, non-payable fallback function (it rejects funds) then Alice's funds would be trapped. Bob doesn't even need to be hostile to for that situation to form but everyone's funds would be trapped.

It will work as expected if you know who/what address1 and address2 are, e.g. transferring between your own contracts or addresses which is why I qualify the warning with "untrusted."

Hope it helps.

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.