feat: processor for ERC20FeeProxy on Near #1096
Conversation
| @@ -77,8 +77,8 @@ export function encodeSwapToPayAnyToErc20Request( | |||
| signerOrProvider: providers.Provider | Signer = getProvider(), | |||
| options: IRequestPaymentOptions, | |||
| ): string { | |||
| const conversionSettings = options?.conversion; | |||
| const swapSettings = options?.swap; | |||
| const conversionSettings = options.conversion; | |||
There was a problem hiding this comment.
Not related, these arguments are not optional anymore.
| @@ -107,7 +106,7 @@ export function getRequestPaymentValues(request: ClientTypes.IRequestData): { | |||
| expectedStartDate, | |||
| tokensAccepted, | |||
| maxRateTimespan, | |||
| network, | |||
| network: network ?? request.currencyInfo.network, | |||
There was a problem hiding this comment.
Not related but it looks obvious and convenient.
github-actions
bot
requested review from
alexandre-abrioux,
kevindavee,
leoslr and
olivier7delf
packages/payment-processor/test/payment/erc20-fee-proxy-near.test.ts
Outdated
Show resolved
Hide resolved
packages/payment-processor/test/payment/erc20-fee-proxy-near.test.ts
Outdated
Show resolved
Hide resolved
| ); | ||
| } | ||
| const proxyAddress = erc20FeeProxyArtifact.getAddress(network, 'near'); | ||
| if (!(await isReceiverReady(walletConnection, request.currencyInfo.value, proxyAddress))) { |
There was a problem hiding this comment.
Just to make it clear, everytime we support a new ERC20 on near, we will have to fund our proxy contract with at least MIN_STORAGE_FOR_FUNGIBLE of this token, correct ?
There was a problem hiding this comment.
@leoslr Yes, that's correct. I plan to check how other dapps do, especially, our contract should probably handle the storage in the same transaction, in a future version. cc @alexandre-abrioux
| return balance.gte(amount); | ||
| }); | ||
| } | ||
| if (token.type === RequestLogicTypes.CURRENCY.ERC20) { |
There was a problem hiding this comment.
In this case, shouldn't we also check the account solvency in native currency for:
- The transaction costs (not sure how this works on Near)
- The mandatory deposit of 1 yoctoNEAR
If you add the condition, having the associated test would be nice
There was a problem hiding this comment.
That's a very good point, I did not think of that. The wallet UX is quite good when it comes to lack of NEAR (for both fees and deposit), so I think it's OK to let it handle this case.
There was a problem hiding this comment.
| * @param tokenAddress | ||
| * @param paymentAddress | ||
| */ | ||
| export const storageDeposit = async ( |
There was a problem hiding this comment.
Not directly related to this PR, but if the payee does not have the required storage deposit, how do we plan to handle this on the FE.
Is it the payer who will have to perform an additional transaction to send the storage deposit amount to the payee's account, and then perform the actual payment ?
There was a problem hiding this comment.
So that's a key question I haven't answered yet. I should check how other dapps do. I think we should assume that the payer pays for this storage in this case (and we should thus increase the transaction deposit).
There was a problem hiding this comment.
There was a problem hiding this comment.
Looks good to me!
@leoslr had good comments/questions, I'm also interested in those. Apart from that, approved 👌
Description of the changes
feat:
payment-processormethod for paying an ERC20FeeProxy request on Near, by sending fungible tokens to a proxy contract.chore: deployment address for mainnet.