Adding ETH and multiple ERC20 payment support #56
Conversation
aaitor
changed the title
EscrowPayment error: stack too deep
… in ETH and additional ERC20 contracts
| ) | ||
| internal | ||
| { | ||
| _rewardAddress.transfer(_amount); |
There was a problem hiding this comment.
I guess the idea is that this contract (LockPaymentCondition) generally doesn't have any ETH, but when fulfill is called then it should have the necessary funds to transfer to rewardAddress (msg.value).
There was a problem hiding this comment.
ya, LockPaymentCondition doesn't have funds and when the user execute the fulfill it locks the funds in the _rewardAddress, typically the EscrowPaymentCondition.address
| ); | ||
| (bool sent,) = _receivers[i].call{value: _amounts[i]}(""); | ||
| require(sent, "Failed to send Ether"); | ||
| // _receivers[i].transfer(_amounts[i]); |
There was a problem hiding this comment.
Probably best to decide which one to use (for transferring ETH), and then use it here and LockPaymentCondition too.
There was a problem hiding this comment.
The payment flow support distribute multiple rewards (_amounts) to multiple addresses (receivers). So the fulfill condition do the transfers using the previously locked funds by the user in the contract using the LockPaymentCondition.
There was a problem hiding this comment.
| @@ -21,6 +21,7 @@ import '../ConditionStoreLibrary.sol'; | |||
| */ | |||
| contract EscrowReward is Reward { | |||
There was a problem hiding this comment.
Any reason to still keep the EscrowReward condition since it was replaced by EscrowPayment?
There was a problem hiding this comment.
Good point, I agree we should remove some stuff. I think I will open an issue to deprecate this and some other stuff as part of a different PR.
Description
Adding ETH and multiple ERC20 support for payment in Lock and Reward payment conditions
Is this PR related with an open issue?
Related to Issue #
Types of changes
Checklist: