From 21d014d481245feed4757bfd4dc27e2f6a09f957 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 24 Oct 2019 19:04:50 -0300 Subject: [PATCH] Improve PullPayment docs (#1965) * Improve PullPayment docs * Reword escrow note * Update contracts/payment/PullPayment.sol Co-Authored-By: Francisco Giordano (cherry picked from commit 9e19d90cd9df3897e499753aa7951c0bc7759df7) --- contracts/payment/PullPayment.sol | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/contracts/payment/PullPayment.sol b/contracts/payment/PullPayment.sol index a37728cb9..594d618f2 100644 --- a/contracts/payment/PullPayment.sol +++ b/contracts/payment/PullPayment.sol @@ -3,9 +3,18 @@ pragma solidity ^0.5.0; import "./escrow/Escrow.sol"; /** - * @title PullPayment - * @dev Base contract supporting async send for pull payments. Inherit from this - * contract and use {_asyncTransfer} instead of send or transfer. + * @dev Simple implementation of a + * https://consensys.github.io/smart-contract-best-practices/recommendations/#favor-pull-over-push-for-external-calls[pull-payment] + * 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, security-wise. It prevents recipients from blocking execution, and + * eliminates reentrancy concerns. + * + * To use, derive from the `PullPayment` contract, and use {_asyncTransfer} + * instead of Solidity's `transfer` function. Payees can query their due + * payments with {payments}, and retrieve them with {withdrawPayments}. */ contract PullPayment { Escrow private _escrow; @@ -15,15 +24,20 @@ contract PullPayment { } /** - * @dev Withdraw accumulated balance. - * @param payee Whose balance will be withdrawn. + * @dev Withdraw accumulated payments. + * @param payee Whose payments will be withdrawn. + * + * Note that _any_ account can call this function, not just the `payee`. + * This means that contracts unaware of the `PullPayment` protocol can still + * receive funds this way, by having a separate account call + * {withdrawPayments}. */ function withdrawPayments(address payable payee) public { _escrow.withdraw(payee); } /** - * @dev Returns the credit owed to an address. + * @dev Returns the payments owed to an address. * @param dest The creditor's address. */ function payments(address dest) public view returns (uint256) { @@ -34,6 +48,9 @@ contract PullPayment { * @dev Called by the payer to store the sent amount as credit to be pulled. * @param dest The destination address of the funds. * @param amount The amount to transfer. + * + * Funds sent in this way are stored in an intermediate {Escrow} contract, so + * there is no danger of them being spent before withdrawal. */ function _asyncTransfer(address dest, uint256 amount) internal { _escrow.deposit.value(amount)(dest);