nft-tutorial icon indicating copy to clipboard operation
nft-tutorial copied to clipboard

part 4 / withdrawPayments() not working

Open fixitornever opened this issue 3 years ago • 4 comments

Hello

I followed part 4 with few remarks leading me to call the withdrawPayments() to my smart contract. Each time the mintTo() method is called the smart contract is getting ETH which is what we intented to do. While calling withdrawPayments() with my account address as param I was expecting to receive the ETH hold by the smart contract however while the method is perfectly called and logged on the SC the contract is still holding the ETH and my account didn't get the amount.

I'm wondering if I'm missing something ?

Contract inheriting from PullPayment: contract NFT is ERC721, PullPayment, Ownable

Override of the withdrawPayments to add ownership right (as per the step 4):

    /// @dev Overridden in order to make it an onlyOwner function
    function withdrawPayments(address payable payee) public override onlyOwner virtual
    {
        super.withdrawPayments(payee);
    }

Instance of a call of that method on my smart contract:

Function: withdrawPayments(address payee)

MethodID: 0x31b3eb94
[0]:  0000000000000000000000008ca5b166589ba2157e177bbdec1c8b6765ed6eba

Thanks for your help

fixitornever avatar Feb 11 '22 11:02 fixitornever

The following code is in Escrow.sol, which is used by PullPayment.sol. When executing withdrawPayments(), it needs to get the value from the _deposits mapping, but _deposits has no value, because only deposits() can go to Add value to _deposits, and deposit() has never been called

    mapping(address => uint256) private _deposits;
    function deposit(address payee) public payable virtual onlyOwner {
        uint256 amount = msg.value;
        _deposits[payee] += amount;
        emit Deposited(payee, amount);
    }
    function withdraw(address payable payee) public virtual onlyOwner {
        uint256 payment = _deposits[payee];

        _deposits[payee] = 0;

        payee.sendValue(payment);

        emit Withdrawn(payee, payment);
    }

This is my understanding, if you have other ideas, please reply

liy010 avatar Feb 14 '22 10:02 liy010

Did you find a solution for this @liy010 ? It's pretty critical that we can withdraw accumulated funds from our contracts!

cronoklee avatar Apr 12 '22 13:04 cronoklee

The solution is in pull @cronoklee

liy010 avatar Apr 12 '22 13:04 liy010

Thanks a lot - adding a call to _asyncTransfer after _safeMint call seems to solve the issue tho the contract behaves a little differently now. See the pull request for details https://github.com/ProjectOpenSea/nft-tutorial/pull/1

cronoklee avatar Apr 20 '22 08:04 cronoklee