From 2f1a9ec7d7cfd09c782649e426cf96b6ec0c6d03 Mon Sep 17 00:00:00 2001 From: olivier7delf <55892112+olivier7delf@users.noreply.github.com> Date: Tue, 9 Aug 2022 15:09:08 +0200 Subject: [PATCH 1/2] fix(payment-processor): add reset approvals to run twice tests (#894) * add a reset approval function and use it on two files and update README regarding tests * Update packages/payment-processor/src/payment/utils.ts Co-authored-by: Yo <56731761+yomarion@users.noreply.github.com> * naming - using revoke * Readme re ordering Co-authored-by: Yo <56731761+yomarion@users.noreply.github.com> --- packages/payment-processor/README.md | 4 +- .../payment-processor/src/payment/utils.ts | 27 ++++++- .../test/payment/any-to-erc20-proxy.test.ts | 11 +++ .../test/payment/encoder-approval.test.ts | 72 +++++++++++-------- 4 files changed, 82 insertions(+), 32 deletions(-) diff --git a/packages/payment-processor/README.md b/packages/payment-processor/README.md index ab8cea6e38..2416d3d3dc 100644 --- a/packages/payment-processor/README.md +++ b/packages/payment-processor/README.md @@ -8,7 +8,9 @@ It contains client-side payment methods for: ### Test -To run the payment-processor tests we need a local running ganache with all our smart contracts deployed. You can open two terminals and do: +To run the payment-processor tests we need a local running ganache with all our smart contracts deployed. +Do not define "NETWORK" variable in the smart-contracts .env file. +You can open two terminals and do: ``` # Terminal 1 diff --git a/packages/payment-processor/src/payment/utils.ts b/packages/payment-processor/src/payment/utils.ts index a0dc0b72df..f3ac234fe2 100644 --- a/packages/payment-processor/src/payment/utils.ts +++ b/packages/payment-processor/src/payment/utils.ts @@ -1,4 +1,4 @@ -import { ethers, Signer, providers, BigNumber, BigNumberish } from 'ethers'; +import { ethers, Signer, providers, BigNumber, BigNumberish, ContractTransaction } from 'ethers'; import { PaymentReferenceCalculator, getDefaultProvider } from '@requestnetwork/payment-detection'; import { @@ -8,6 +8,7 @@ import { RequestLogicTypes, } from '@requestnetwork/types'; import { getCurrencyHash } from '@requestnetwork/currency'; +import { ERC20__factory } from '@requestnetwork/smart-contracts/types'; /** * Thrown when the library does not support a payment blockchain network. @@ -345,3 +346,27 @@ export function comparePnTypeAndVersion( pn?.version === getPaymentNetworkExtension(request)?.version ); } + +/** + * Revoke ERC20 approval of a token for a given `spenderAddress` + */ +export async function revokeErc20Approval( + spenderAddress: string, + paymentTokenAddress: string, + signerOrProvider: providers.Provider | Signer = getProvider(), +): Promise { + const erc20interface = ERC20__factory.connect(paymentTokenAddress, signerOrProvider).interface; + const encodedTx = erc20interface.encodeFunctionData('approve', [ + spenderAddress, + BigNumber.from(0), + ]); + + const preparedTx = { + data: encodedTx, + to: paymentTokenAddress, + value: 0, + }; + const signer = getSigner(signerOrProvider); + const tx = await signer.sendTransaction(preparedTx); + return tx; +} diff --git a/packages/payment-processor/test/payment/any-to-erc20-proxy.test.ts b/packages/payment-processor/test/payment/any-to-erc20-proxy.test.ts index 88dbce7dfc..d4a81bb9f2 100644 --- a/packages/payment-processor/test/payment/any-to-erc20-proxy.test.ts +++ b/packages/payment-processor/test/payment/any-to-erc20-proxy.test.ts @@ -16,6 +16,8 @@ import { ERC20__factory } from '@requestnetwork/smart-contracts/types'; import { currencyManager } from './shared'; import { IConversionPaymentSettings } from '../../src/index'; import { UnsupportedCurrencyError } from '@requestnetwork/currency'; +import { AnyToERC20PaymentDetector } from '@requestnetwork/payment-detection'; +import { getProxyAddress, revokeErc20Approval } from '../../src/payment/utils'; // Cf. ERC20Alpha in TestERC20.sol const erc20ContractAddress = '0x38cF23C52Bb4B13F051Aec09580a2dE845a7FA35'; @@ -81,6 +83,15 @@ const validEuroRequest: ClientTypes.IRequestData = { }; describe('conversion-erc20-fee-proxy', () => { + beforeAll(async () => { + // reset approval + await revokeErc20Approval( + getProxyAddress(validEuroRequest, AnyToERC20PaymentDetector.getDeploymentInformation), + erc20ContractAddress, + wallet.provider, + ); + }); + describe('error checking', () => { it('should throw an error if the token is not accepted', async () => { await expect( diff --git a/packages/payment-processor/test/payment/encoder-approval.test.ts b/packages/payment-processor/test/payment/encoder-approval.test.ts index df93717662..80d2ab54be 100644 --- a/packages/payment-processor/test/payment/encoder-approval.test.ts +++ b/packages/payment-processor/test/payment/encoder-approval.test.ts @@ -7,7 +7,7 @@ import { RequestLogicTypes, } from '@requestnetwork/types'; import { encodeRequestErc20ApprovalIfNeeded } from '../../src'; -import { getProxyAddress } from '../../src/payment/utils'; +import { getProxyAddress, revokeErc20Approval } from '../../src/payment/utils'; import { AnyToERC20PaymentDetector, Erc20PaymentNetwork } from '@requestnetwork/payment-detection'; import { currencyManager } from './shared'; import { IPreparedTransaction } from 'payment-processor/dist/payment/prepared-transaction'; @@ -55,6 +55,12 @@ const erc20ApprovalData = (proxy: string) => { .toLowerCase()}ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff`; }; +let proxyERC20: string; +let proxyERC20Fee: string; +let proxyERC20Conv: string; +let proxyERC20Swap: string; +let proxyERC20SwapConv: string; + const baseValidRequest: ClientTypes.IRequestData = { balance: { balance: '0', @@ -215,11 +221,40 @@ const validRequestEthConversionProxy: ClientTypes.IRequestData = { beforeAll(async () => { const mainAddress = wallet.address; wallet = Wallet.fromMnemonic(mnemonic).connect(provider); - wallet.sendTransaction({ + await wallet.sendTransaction({ to: mainAddress, value: BigNumber.from('1000000000000000000'), }); wallet = Wallet.fromMnemonic(mnemonic, mnemonicPath).connect(provider); + + // reset approvals + proxyERC20 = getProxyAddress( + baseValidRequest, + Erc20PaymentNetwork.ERC20ProxyPaymentDetector.getDeploymentInformation, + ); + await revokeErc20Approval(proxyERC20, erc20ContractAddress, wallet); + + proxyERC20Fee = getProxyAddress( + validRequestERC20FeeProxy, + Erc20PaymentNetwork.ERC20FeeProxyPaymentDetector.getDeploymentInformation, + ); + await revokeErc20Approval(proxyERC20Fee, erc20ContractAddress, wallet); + + proxyERC20Conv = getProxyAddress( + validRequestERC20ConversionProxy, + AnyToERC20PaymentDetector.getDeploymentInformation, + ); + await revokeErc20Approval(proxyERC20Conv, alphaContractAddress, wallet); + + proxyERC20Swap = erc20SwapToPayArtifact.getAddress( + validRequestERC20FeeProxy.currencyInfo.network!, + ); + await revokeErc20Approval(proxyERC20Swap, alphaContractAddress, wallet); + + proxyERC20SwapConv = erc20SwapConversionArtifact.getAddress( + validRequestERC20FeeProxy.currencyInfo.network!, + ); + await revokeErc20Approval(proxyERC20SwapConv, alphaContractAddress, wallet); }); describe('Approval encoder handles ERC20 Proxy', () => { @@ -230,13 +265,8 @@ describe('Approval encoder handles ERC20 Proxy', () => { wallet.address, ); - const proxyAddress = getProxyAddress( - baseValidRequest, - Erc20PaymentNetwork.ERC20ProxyPaymentDetector.getDeploymentInformation, - ); - expect(approvalTransaction).toEqual({ - data: erc20ApprovalData(proxyAddress), + data: erc20ApprovalData(proxyERC20), to: erc20ContractAddress, value: 0, }); @@ -267,13 +297,8 @@ describe('Approval encoder handles ERC20 Fee Proxy', () => { wallet.address, ); - const proxyAddress = getProxyAddress( - validRequestERC20FeeProxy, - Erc20PaymentNetwork.ERC20FeeProxyPaymentDetector.getDeploymentInformation, - ); - expect(approvalTransaction).toEqual({ - data: erc20ApprovalData(proxyAddress), + data: erc20ApprovalData(proxyERC20Fee), to: erc20ContractAddress, value: 0, }); @@ -306,13 +331,8 @@ describe('Approval encoder handles ERC20 Conversion Proxy', () => { }, ); - const proxyAddress = getProxyAddress( - validRequestERC20ConversionProxy, - AnyToERC20PaymentDetector.getDeploymentInformation, - ); - expect(approvalTransaction).toEqual({ - data: erc20ApprovalData(proxyAddress), + data: erc20ApprovalData(proxyERC20Conv), to: alphaContractAddress, value: 0, }); @@ -362,12 +382,8 @@ describe('Approval encoder handles ERC20 Swap Proxy', () => { }, ); - const proxyAddress = erc20SwapToPayArtifact.getAddress( - validRequestERC20FeeProxy.currencyInfo.network!, - ); - expect(approvalTransaction).toEqual({ - data: erc20ApprovalData(proxyAddress), + data: erc20ApprovalData(proxyERC20Swap), to: alphaContractAddress, value: 0, }); @@ -408,12 +424,8 @@ describe('Approval encoder handles ERC20 Swap & Conversion Proxy', () => { }, ); - const proxyAddress = erc20SwapConversionArtifact.getAddress( - validRequestERC20FeeProxy.currencyInfo.network!, - ); - expect(approvalTransaction).toEqual({ - data: erc20ApprovalData(proxyAddress), + data: erc20ApprovalData(proxyERC20SwapConv), to: erc20ContractAddress, value: 0, }); From 6b5441141a3b62bef1e633dbf9fd09ecadf76478 Mon Sep 17 00:00:00 2001 From: Yo <56731761+yomarion@users.noreply.github.com> Date: Tue, 9 Aug 2022 16:46:31 +0200 Subject: [PATCH 2/2] Update auto_assign.yml --- .github/auto_assign.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/auto_assign.yml b/.github/auto_assign.yml index cc536849c7..218b726d42 100644 --- a/.github/auto_assign.yml +++ b/.github/auto_assign.yml @@ -13,6 +13,7 @@ reviewers: - alexandre-abrioux - KolevDarko - leoslr + - omarsp-eth # A list of keywords to be skipped the process that add reviewers if pull requests include it skipKeywords: