re-add PullPayment (with immutable ownership of the Escrow)

contracts/security/PullPayment.sol

@@ -0,0 +1,70 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.8.0) (security/PullPayment.sol)
+
+pragma solidity ^0.8.0;
+
+import "./PullPaymentEscrow.sol";
+
+/**
+ * @dev Simple implementation of a
+ * https://consensys.github.io/smart-contract-best-practices/development-recommendations/general/external-calls/#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.
+ *
+ * TIP: If you would like to learn more about reentrancy and alternative ways
+ * to protect against it, check out our blog post
+ * https://blog.openzeppelin.com/reentrancy-after-istanbul/[Reentrancy After Istanbul].
+ *
+ * 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}.
+ */
+abstract contract PullPayment {
+    PullPaymentEscrow private immutable _escrow = new PullPaymentEscrow();
+
+    /**
+     * @dev Withdraw accumulated payments, forwarding all gas to the recipient.
+     *
+     * 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}.
+     *
+     * WARNING: Forwarding all gas opens the door to reentrancy vulnerabilities.
+     * Make sure you trust the recipient, or are either following the
+     * checks-effects-interactions pattern or using {ReentrancyGuard}.
+     *
+     * @param payee Whose payments will be withdrawn.
+     *
+     * Causes the `escrow` to emit a {Withdrawn} event.
+     */
+    function withdrawPayments(address payable payee) public virtual {
+        _escrow.withdraw(payee);
+    }
+
+    /**
+     * @dev Returns the payments owed to an address.
+     * @param dest The creditor's address.
+     */
+    function payments(address dest) public view returns (uint256) {
+        return _escrow.depositsOf(dest);
+    }
+
+    /**
+     * @dev Called by the payer to store the sent amount as credit to be pulled.
+     * Funds sent in this way are stored in an intermediate {Escrow} contract, so
+     * there is no danger of them being spent before withdrawal.
+     *
+     * @param dest The destination address of the funds.
+     * @param amount The amount to transfer.
+     *
+     * Causes the `escrow` to emit a {Deposited} event.
+     */
+    function _asyncTransfer(address dest, uint256 amount) internal virtual {
+        _escrow.deposit{value: amount}(dest);
+    }
+}

contracts/security/PullPaymentEscrow.sol

@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: MIT
+// OpenZeppelin Contracts (last updated v4.7.0) (utils/escrow/Escrow.sol)
+
+pragma solidity ^0.8.0;
+
+import "../utils/Address.sol";
+import "../utils/Context.sol";
+
+/**
+ * @title Escrow
+ * @dev Base escrow contract, holds funds designated for a payee until they
+ * withdraw them.
+ *
+ * Intended usage: This contract (and derived escrow contracts) should be a
+ * standalone contract, that only interacts with the contract that instantiated
+ * it. That way, it is guaranteed that all Ether will be handled according to
+ * the `Escrow` rules, and there is no need to check for payable functions or
+ * transfers in the inheritance tree. The contract that uses the escrow as its
+ * payment method should be its owner, and provide public methods redirecting
+ * to the escrow's deposit and withdraw.
+ */
+contract PullPaymentEscrow is Context {
+    using Address for address payable;
+
+    address public immutable owner = msg.sender;
+
+    event Deposited(address indexed payee, uint256 weiAmount);
+    event Withdrawn(address indexed payee, uint256 weiAmount);
+
+    mapping(address => uint256) private _deposits;
+
+    modifier onlyOwner() {
+        require(owner == _msgSender(), "Ownable: caller is not the owner");
+        _;
+    }
+
+    function depositsOf(address payee) public view returns (uint256) {
+        return _deposits[payee];
+    }
+
+    /**
+     * @dev Stores the sent amount as credit to be withdrawn.
+     * @param payee The destination address of the funds.
+     *
+     * Emits a {Deposited} event.
+     */
+    function deposit(address payee) public payable virtual onlyOwner {
+        uint256 amount = msg.value;
+        _deposits[payee] += amount;
+        emit Deposited(payee, amount);
+    }
+
+    /**
+     * @dev Withdraw accumulated balance for a payee, forwarding all gas to the
+     * recipient.
+     *
+     * WARNING: Forwarding all gas opens the door to reentrancy vulnerabilities.
+     * Make sure you trust the recipient, or are either following the
+     * checks-effects-interactions pattern or using {ReentrancyGuard}.
+     *
+     * @param payee The address whose funds will be withdrawn and transferred to.
+     *
+     * Emits a {Withdrawn} event.
+     */
+    function withdraw(address payable payee) public virtual onlyOwner {
+        uint256 payment = _deposits[payee];
+
+        _deposits[payee] = 0;
+
+        payee.sendValue(payment);
+
+        emit Withdrawn(payee, payment);
+    }
+}

contracts/security/README.adoc

@@ -5,13 +5,16 @@ NOTE: This document is better viewed at https://docs.openzeppelin.com/contracts/
 
 These contracts aim to cover common security practices.
 
+* {PullPayment}: A pattern that can be used to avoid reentrancy attacks.
 * {ReentrancyGuard}: A modifier that can prevent reentrancy during certain functions.
 * {Pausable}: A common emergency response mechanism that can pause functionality while a remediation is pending.
 
 TIP: For an overview on reentrancy and the possible mechanisms to prevent it, read our article https://blog.openzeppelin.com/reentrancy-after-istanbul/[Reentrancy After Istanbul].
 
 == Contracts
 
+{{PullPayment}}
+
 {{ReentrancyGuard}}
 
 {{Pausable}}

docs/modules/ROOT/pages/utilities.adoc

@@ -87,7 +87,7 @@ Easy!
 
 Want to split some payments between multiple people? Maybe you have an app that sends 30% of art purchases to the original creator and 70% of the profits to the current owner; you can build that with xref:api:finance.adoc#PaymentSplitter[`PaymentSplitter`]!
 
-In Solidity, there are some security concerns with blindly sending money to accounts, since it allows them to execute arbitrary code. You can read up on these security concerns in the https://consensys.github.io/smart-contract-best-practices/[Ethereum Smart Contract Best Practices] website.
+In Solidity, there are some security concerns with blindly sending money to accounts, since it allows them to execute arbitrary code. You can read up on these security concerns in the https://consensys.github.io/smart-contract-best-practices/[Ethereum Smart Contract Best Practices] website. One of the ways to fix reentrancy and stalling problems is, instead of immediately sending Ether to accounts that need it, you can use xref:api:security.adoc#PullPayment[`PullPayment`], which offers an xref:api:security.adoc#PullPayment-_asyncTransfer-address-uint256-[`_asyncTransfer`] function for sending money to something and requesting that they xref:api:security.adoc#PullPayment-withdrawPayments-address-payable-[`withdrawPayments()`] it later.
 
 [[collections]]
 == Collections
@@ -101,7 +101,7 @@ Want to keep track of some numbers that increment by 1 every time you want anoth
 
 === Base64
 
-xref:api:utils.adoc#Base64[`Base64`] util allows you to transform `bytes32` data into its Base64 `string` representation.
+xref:api:utils.adoc#Base64[`Base64`] util allows you to transform `bytes32` data into its Base64 `string` representation. 
 
 This is especially useful for building URL-safe tokenURIs for both xref:api:token/ERC721.adoc#IERC721Metadata-tokenURI-uint256-[`ERC721`] or xref:api:token/ERC1155.adoc#IERC1155MetadataURI-uri-uint256-[`ERC1155`]. This library provides a clever way to serve URL-safe https://developer.mozilla.org/docs/Web/HTTP/Basics_of_HTTP/Data_URIs/[Data URI] compliant strings to serve on-chain data structures.
 
@@ -120,7 +120,7 @@ contract My721Token is ERC721 {
     using Strings for uint256;
 
     constructor() ERC721("My721Token", "MTK") {}
-
+    
     ...
 
     function tokenURI(uint256 tokenId)
@@ -138,7 +138,7 @@ contract My721Token is ERC721 {
 
         return string(
             abi.encodePacked(
-                "data:application/json;base64,",
+                "data:application/json;base64,", 
                 Base64.encode(dataURI)
             )
         );

test/security/PullPayment.test.js

@@ -0,0 +1,52 @@
+const { balance, ether } = require('@openzeppelin/test-helpers');
+
+const { expect } = require('chai');
+
+const PullPaymentMock = artifacts.require('$PullPayment');
+
+contract('PullPayment', function (accounts) {
+  const [payer, payee1, payee2] = accounts;
+
+  const amount = ether('17');
+
+  beforeEach(async function () {
+    this.contract = await PullPaymentMock.new();
+    await web3.eth.sendTransaction({ from: payer, to: this.contract.address, value: amount });
+  });
+
+  describe('payments', function () {
+    it('can record an async payment correctly', async function () {
+      await this.contract.$_asyncTransfer(payee1, 100, { from: payer });
+      expect(await this.contract.payments(payee1)).to.be.bignumber.equal('100');
+    });
+
+    it('can add multiple balances on one account', async function () {
+      await this.contract.$_asyncTransfer(payee1, 200, { from: payer });
+      await this.contract.$_asyncTransfer(payee1, 300, { from: payer });
+      expect(await this.contract.payments(payee1)).to.be.bignumber.equal('500');
+    });
+
+    it('can add balances on multiple accounts', async function () {
+      await this.contract.$_asyncTransfer(payee1, 200, { from: payer });
+      await this.contract.$_asyncTransfer(payee2, 300, { from: payer });
+
+      expect(await this.contract.payments(payee1)).to.be.bignumber.equal('200');
+
+      expect(await this.contract.payments(payee2)).to.be.bignumber.equal('300');
+    });
+  });
+
+  describe('withdrawPayments', function () {
+    it('can withdraw payment', async function () {
+      const balanceTracker = await balance.tracker(payee1);
+
+      await this.contract.$_asyncTransfer(payee1, amount, { from: payer });
+      expect(await this.contract.payments(payee1)).to.be.bignumber.equal(amount);
+
+      await this.contract.withdrawPayments(payee1);
+
+      expect(await balanceTracker.delta()).to.be.bignumber.equal(amount);
+      expect(await this.contract.payments(payee1)).to.be.bignumber.equal('0');
+    });
+  });
+});