From 4fe36c8fa4c470e2553a4e6632a7d4cc544e5e4c Mon Sep 17 00:00:00 2001 From: Jason Raymond Bell Date: Thu, 20 May 2021 23:25:51 +0100 Subject: [PATCH] Introduce registry of valid Augustus addresses Set in constructor of BaseParaSwapSellAdapter and validates before swap. Created mock registry that only validates one address. Changed the test fixtures to accomodate registry and added two new tests. Updated deployment script. Registry address left as a placeholder in package.json since not known yet. Fixes MixBytes Warning 1. --- .../adapters/BaseParaSwapSellAdapter.sol | 12 +++- .../adapters/ParaSwapLiquiditySwapAdapter.sol | 6 +- .../interfaces/IParaSwapAugustusRegistry.sol | 7 ++ contracts/mocks/swap/MockParaSwapAugustus.sol | 8 +-- .../swap/MockParaSwapAugustusRegistry.sol | 17 +++++ helpers/contracts-deployments.ts | 14 +++- helpers/contracts-getters.ts | 9 +++ helpers/types.ts | 1 + package.json | 2 +- .../deploy-ParaSwapLiquiditySwapAdapter.ts | 7 +- test-suites/test-aave/__setup.spec.ts | 7 +- .../paraswapAdapters.liquiditySwap.spec.ts | 65 ++++++++++++++++++- 12 files changed, 138 insertions(+), 17 deletions(-) create mode 100644 contracts/interfaces/IParaSwapAugustusRegistry.sol create mode 100644 contracts/mocks/swap/MockParaSwapAugustusRegistry.sol diff --git a/contracts/adapters/BaseParaSwapSellAdapter.sol b/contracts/adapters/BaseParaSwapSellAdapter.sol index 5b750287..93d3cc5b 100644 --- a/contracts/adapters/BaseParaSwapSellAdapter.sol +++ b/contracts/adapters/BaseParaSwapSellAdapter.sol @@ -5,6 +5,7 @@ pragma experimental ABIEncoderV2; import {BaseParaSwapAdapter} from './BaseParaSwapAdapter.sol'; import {PercentageMath} from '../protocol/libraries/math/PercentageMath.sol'; import {IParaSwapAugustus} from '../interfaces/IParaSwapAugustus.sol'; +import {IParaSwapAugustusRegistry} from '../interfaces/IParaSwapAugustusRegistry.sol'; import {ILendingPoolAddressesProvider} from '../interfaces/ILendingPoolAddressesProvider.sol'; import {IERC20Detailed} from '../dependencies/openzeppelin/contracts/IERC20Detailed.sol'; @@ -16,10 +17,15 @@ import {IERC20Detailed} from '../dependencies/openzeppelin/contracts/IERC20Detai abstract contract BaseParaSwapSellAdapter is BaseParaSwapAdapter { using PercentageMath for uint256; + IParaSwapAugustusRegistry public immutable AUGUSTUS_REGISTRY; + constructor( - ILendingPoolAddressesProvider addressesProvider + ILendingPoolAddressesProvider addressesProvider, + IParaSwapAugustusRegistry augustusRegistry ) public BaseParaSwapAdapter(addressesProvider) { - // This is only required to initialize BaseParaSwapAdapter + // Do something on Augustus registry to check the right contract was passed + require(!augustusRegistry.isValidAugustus(address(0))); + AUGUSTUS_REGISTRY = augustusRegistry; } /** @@ -42,6 +48,8 @@ abstract contract BaseParaSwapSellAdapter is BaseParaSwapAdapter { uint256 amountToSwap, uint256 minAmountToReceive ) internal returns (uint256 amountReceived) { + require(AUGUSTUS_REGISTRY.isValidAugustus(address(augustus)), 'INVALID_AUGUSTUS'); + { uint256 fromAssetDecimals = _getDecimals(assetToSwapFrom); uint256 toAssetDecimals = _getDecimals(assetToSwapTo); diff --git a/contracts/adapters/ParaSwapLiquiditySwapAdapter.sol b/contracts/adapters/ParaSwapLiquiditySwapAdapter.sol index c5ff6305..7cc1105d 100644 --- a/contracts/adapters/ParaSwapLiquiditySwapAdapter.sol +++ b/contracts/adapters/ParaSwapLiquiditySwapAdapter.sol @@ -4,6 +4,7 @@ pragma experimental ABIEncoderV2; import {BaseParaSwapSellAdapter} from './BaseParaSwapSellAdapter.sol'; import {ILendingPoolAddressesProvider} from '../interfaces/ILendingPoolAddressesProvider.sol'; +import {IParaSwapAugustusRegistry} from '../interfaces/IParaSwapAugustusRegistry.sol'; import {IERC20Detailed} from '../dependencies/openzeppelin/contracts/IERC20Detailed.sol'; import {IERC20WithPermit} from '../interfaces/IERC20WithPermit.sol'; import {IParaSwapAugustus} from '../interfaces/IParaSwapAugustus.sol'; @@ -16,8 +17,9 @@ import {ReentrancyGuard} from '../dependencies/openzeppelin/contracts/Reentrancy */ contract ParaSwapLiquiditySwapAdapter is BaseParaSwapSellAdapter, ReentrancyGuard { constructor( - ILendingPoolAddressesProvider addressesProvider - ) public BaseParaSwapSellAdapter(addressesProvider) { + ILendingPoolAddressesProvider addressesProvider, + IParaSwapAugustusRegistry augustusRegistry + ) public BaseParaSwapSellAdapter(addressesProvider, augustusRegistry) { // This is only required to initialize BaseParaSwapSellAdapter } diff --git a/contracts/interfaces/IParaSwapAugustusRegistry.sol b/contracts/interfaces/IParaSwapAugustusRegistry.sol new file mode 100644 index 00000000..f10e13dd --- /dev/null +++ b/contracts/interfaces/IParaSwapAugustusRegistry.sol @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: agpl-3.0 +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +interface IParaSwapAugustusRegistry { + function isValidAugustus(address augustus) external view returns (bool); +} diff --git a/contracts/mocks/swap/MockParaSwapAugustus.sol b/contracts/mocks/swap/MockParaSwapAugustus.sol index 8a6874c6..1cf32171 100644 --- a/contracts/mocks/swap/MockParaSwapAugustus.sol +++ b/contracts/mocks/swap/MockParaSwapAugustus.sol @@ -8,7 +8,7 @@ import {IERC20} from '../../dependencies/openzeppelin/contracts/IERC20.sol'; import {MintableERC20} from '../tokens/MintableERC20.sol'; contract MockParaSwapAugustus is IParaSwapAugustus { - MockParaSwapTokenTransferProxy _tokenTransferProxy; + MockParaSwapTokenTransferProxy immutable TOKEN_TRANSFER_PROXY; bool _expectingSwap; address _expectedFromToken; address _expectedToToken; @@ -17,11 +17,11 @@ contract MockParaSwapAugustus is IParaSwapAugustus { uint256 _receivedAmount; constructor() public { - _tokenTransferProxy = new MockParaSwapTokenTransferProxy(); + TOKEN_TRANSFER_PROXY = new MockParaSwapTokenTransferProxy(); } function getTokenTransferProxy() external view override returns (address) { - return address(_tokenTransferProxy); + return address(TOKEN_TRANSFER_PROXY); } function expectSwap( @@ -50,7 +50,7 @@ contract MockParaSwapAugustus is IParaSwapAugustus { require(toToken == _expectedToToken, 'Unexpected to token'); require(fromAmount >= _expectedFromAmountMin && fromAmount <= _expectedFromAmountMax, 'From amount out of range'); require(_receivedAmount >= toAmount, 'Received amount of tokens are less than expected'); - _tokenTransferProxy.transferFrom(fromToken, msg.sender, address(this), fromAmount); + TOKEN_TRANSFER_PROXY.transferFrom(fromToken, msg.sender, address(this), fromAmount); MintableERC20(toToken).mint(_receivedAmount); IERC20(toToken).transfer(msg.sender, _receivedAmount); _expectingSwap = false; diff --git a/contracts/mocks/swap/MockParaSwapAugustusRegistry.sol b/contracts/mocks/swap/MockParaSwapAugustusRegistry.sol new file mode 100644 index 00000000..c8998ec1 --- /dev/null +++ b/contracts/mocks/swap/MockParaSwapAugustusRegistry.sol @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: agpl-3.0 +pragma solidity 0.6.12; +pragma experimental ABIEncoderV2; + +import {IParaSwapAugustusRegistry} from '../../interfaces/IParaSwapAugustusRegistry.sol'; + +contract MockParaSwapAugustusRegistry is IParaSwapAugustusRegistry { + address immutable AUGUSTUS; + + constructor(address augustus) public { + AUGUSTUS = augustus; + } + + function isValidAugustus(address augustus) external view override returns (bool) { + return augustus == AUGUSTUS; + } +} diff --git a/helpers/contracts-deployments.ts b/helpers/contracts-deployments.ts index 9f5e4eee..97331547 100644 --- a/helpers/contracts-deployments.ts +++ b/helpers/contracts-deployments.ts @@ -36,6 +36,7 @@ import { MockATokenFactory, MockFlashLoanReceiverFactory, MockParaSwapAugustusFactory, + MockParaSwapAugustusRegistryFactory, MockStableDebtTokenFactory, MockVariableDebtTokenFactory, MockUniswapV2Router02Factory, @@ -620,9 +621,20 @@ export const deployMockParaSwapAugustus = async (verify?: boolean) => verify ); -export const deployParaSwapLiquiditySwapAdapter = async ( +export const deployMockParaSwapAugustusRegistry = async ( args: [tEthereumAddress], verify?: boolean +) => + withSaveAndVerify( + await new MockParaSwapAugustusRegistryFactory(await getFirstSigner()).deploy(...args), + eContractid.MockParaSwapAugustusRegistry, + args, + verify + ); + +export const deployParaSwapLiquiditySwapAdapter = async ( + args: [tEthereumAddress, tEthereumAddress], + verify?: boolean ) => withSaveAndVerify( await new ParaSwapLiquiditySwapAdapterFactory(await getFirstSigner()).deploy(...args), diff --git a/helpers/contracts-getters.ts b/helpers/contracts-getters.ts index d7281291..82b3df5b 100644 --- a/helpers/contracts-getters.ts +++ b/helpers/contracts-getters.ts @@ -19,6 +19,7 @@ import { MockVariableDebtTokenFactory, MockUniswapV2Router02Factory, MockParaSwapAugustusFactory, + MockParaSwapAugustusRegistryFactory, ParaSwapLiquiditySwapAdapterFactory, PriceOracleFactory, ReserveLogicFactory, @@ -374,6 +375,14 @@ export const getMockParaSwapAugustus = async (address?: tEthereumAddress) => await getFirstSigner() ); +export const getMockParaSwapAugustusRegistry = async (address?: tEthereumAddress) => + await MockParaSwapAugustusRegistryFactory.connect( + address || + (await getDb().get(`${eContractid.MockParaSwapAugustusRegistry}.${DRE.network.name}`).value()) + .address, + await getFirstSigner() + ); + export const getParaSwapLiquiditySwapAdapter = async (address?: tEthereumAddress) => await ParaSwapLiquiditySwapAdapterFactory.connect( address || diff --git a/helpers/types.ts b/helpers/types.ts index 38bcebd0..1feac1ed 100644 --- a/helpers/types.ts +++ b/helpers/types.ts @@ -88,6 +88,7 @@ export enum eContractid { UniswapRepayAdapter = 'UniswapRepayAdapter', FlashLiquidationAdapter = 'FlashLiquidationAdapter', MockParaSwapAugustus = 'MockParaSwapAugustus', + MockParaSwapAugustusRegistry = 'MockParaSwapAugustusRegistry', ParaSwapLiquiditySwapAdapter = 'ParaSwapLiquiditySwapAdapter', } diff --git a/package.json b/package.json index 1105b4b7..46c04270 100644 --- a/package.json +++ b/package.json @@ -67,7 +67,7 @@ "dev:UniswapLiquiditySwapAdapter": "hardhat --network kovan deploy-UniswapLiquiditySwapAdapter --provider 0x88757f2f99175387aB4C6a4b3067c77A695b0349 --router 0xfcd87315f0e4067070ade8682fcdbc3006631441 --weth 0xd0a1e359811322d97991e03f863a0c30c2cf029c", "main:deployUniswapRepayAdapter": "hardhat --network main deploy-UniswapRepayAdapter --provider 0xB53C1a33016B2DC2fF3653530bfF1848a515c8c5 --router 0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D --weth 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2", "main:UniswapLiquiditySwapAdapter": "hardhat --network main deploy-UniswapLiquiditySwapAdapter --provider 0xB53C1a33016B2DC2fF3653530bfF1848a515c8c5 --router 0x7a250d5630B4cF539739dF2C5dAcb4c659F2488D --weth 0xC02aaA39b223FE8D0A0e5C4F27eAD9083C756Cc2", - "main:ParaSwapLiquiditySwapAdapter": "hardhat --network main deploy-ParaSwapLiquiditySwapAdapter --provider 0xB53C1a33016B2DC2fF3653530bfF1848a515c8c5", + "main:ParaSwapLiquiditySwapAdapter": "hardhat --network main deploy-ParaSwapLiquiditySwapAdapter --provider 0xB53C1a33016B2DC2fF3653530bfF1848a515c8c5 --augustusRegistry 0x0000000000000000000000000000000000000000", "kovan:verify": "npm run hardhat:kovan verify:general -- --all --pool Aave", "ropsten:verify": "npm run hardhat:ropsten verify:general -- --all --pool Aave", "mainnet:verify": "npm run hardhat:main verify:general -- --all --pool Aave", diff --git a/tasks/deployments/deploy-ParaSwapLiquiditySwapAdapter.ts b/tasks/deployments/deploy-ParaSwapLiquiditySwapAdapter.ts index 1ee0720f..1322645b 100644 --- a/tasks/deployments/deploy-ParaSwapLiquiditySwapAdapter.ts +++ b/tasks/deployments/deploy-ParaSwapLiquiditySwapAdapter.ts @@ -8,8 +8,9 @@ const CONTRACT_NAME = 'ParaSwapLiquiditySwapAdapter'; task(`deploy-${CONTRACT_NAME}`, `Deploys the ${CONTRACT_NAME} contract`) .addParam('provider', 'Address of the LendingPoolAddressesProvider') + .addParam('augustusRegistry', 'Address of ParaSwap AugustusRegistry') .addFlag('verify', `Verify ${CONTRACT_NAME} contract via Etherscan API.`) - .setAction(async ({ provider, verify }, localBRE) => { + .setAction(async ({ provider, augustusRegistry, verify }, localBRE) => { await localBRE.run('set-DRE'); if (!localBRE.network.config.chainId) { @@ -19,10 +20,10 @@ task(`deploy-${CONTRACT_NAME}`, `Deploys the ${CONTRACT_NAME} contract`) console.log(`\n- ${CONTRACT_NAME} deployment`); const adapter = await new ParaSwapLiquiditySwapAdapterFactory( await getFirstSigner() - ).deploy(provider); + ).deploy(provider, augustusRegistry); await adapter.deployTransaction.wait(); console.log(`${CONTRACT_NAME}.address`, adapter.address); - await verifyContract(adapter.address, [provider]); + await verifyContract(adapter.address, [provider, augustusRegistry]); console.log(`\tFinished ${CONTRACT_NAME} deployment`); }); diff --git a/test-suites/test-aave/__setup.spec.ts b/test-suites/test-aave/__setup.spec.ts index 9e4422ca..7dd73c15 100644 --- a/test-suites/test-aave/__setup.spec.ts +++ b/test-suites/test-aave/__setup.spec.ts @@ -27,6 +27,7 @@ import { deployUniswapRepayAdapter, deployFlashLiquidationAdapter, deployMockParaSwapAugustus, + deployMockParaSwapAugustusRegistry, deployParaSwapLiquiditySwapAdapter, authorizeWETHGateway, } from '../../helpers/contracts-deployments'; @@ -286,9 +287,11 @@ const buildTestEnv = async (deployer: Signer, secondaryWallet: Signer) => { await deployUniswapRepayAdapter(adapterParams); await deployFlashLiquidationAdapter(adapterParams); - await deployMockParaSwapAugustus(); + const augustus = await deployMockParaSwapAugustus(); - await deployParaSwapLiquiditySwapAdapter([addressesProvider.address]); + const augustusRegistry = await deployMockParaSwapAugustusRegistry([augustus.address]); + + await deployParaSwapLiquiditySwapAdapter([addressesProvider.address, augustusRegistry.address]); await deployWalletBalancerProvider(); diff --git a/test-suites/test-aave/paraswapAdapters.liquiditySwap.spec.ts b/test-suites/test-aave/paraswapAdapters.liquiditySwap.spec.ts index 415bb474..1a4a424b 100644 --- a/test-suites/test-aave/paraswapAdapters.liquiditySwap.spec.ts +++ b/test-suites/test-aave/paraswapAdapters.liquiditySwap.spec.ts @@ -6,9 +6,13 @@ import { getSignatureFromTypedData, buildParaSwapLiquiditySwapParams, } from '../../helpers/contracts-helpers'; -import { getMockParaSwapAugustus } from '../../helpers/contracts-getters'; +import { + getMockParaSwapAugustus, + getMockParaSwapAugustusRegistry, +} from '../../helpers/contracts-getters'; import { deployParaSwapLiquiditySwapAdapter } from '../../helpers/contracts-deployments'; import { MockParaSwapAugustus } from '../../types/MockParaSwapAugustus'; +import { MockParaSwapAugustusRegistry } from '../../types/MockParaSwapAugustusRegistry'; import { Zero } from '@ethersproject/constants'; import BigNumber from 'bignumber.js'; import { DRE, evmRevert, evmSnapshot } from '../../helpers/misc-utils'; @@ -23,10 +27,12 @@ const { expect } = require('chai'); makeSuite('ParaSwap adapters', (testEnv: TestEnv) => { let mockAugustus: MockParaSwapAugustus; + let mockAugustusRegistry: MockParaSwapAugustusRegistry; let evmSnapshotId: string; before(async () => { mockAugustus = await getMockParaSwapAugustus(); + mockAugustusRegistry = await getMockParaSwapAugustusRegistry(); }); beforeEach(async () => { @@ -43,13 +49,25 @@ makeSuite('ParaSwap adapters', (testEnv: TestEnv) => { const { addressesProvider } = testEnv; await deployParaSwapLiquiditySwapAdapter([ addressesProvider.address, + mockAugustusRegistry.address, ]); }); it('should revert if not valid addresses provider', async () => { await expect( deployParaSwapLiquiditySwapAdapter([ - mockAugustus.address, + mockAugustus.address, // any invalid contract can be used here + mockAugustusRegistry.address, + ]) + ).to.be.reverted; + }); + + it('should revert if not valid augustus registry', async () => { + const { addressesProvider } = testEnv; + await expect( + deployParaSwapLiquiditySwapAdapter([ + addressesProvider.address, + mockAugustus.address, // any invalid contract can be used here ]) ).to.be.reverted; }); @@ -1637,6 +1655,49 @@ makeSuite('ParaSwap adapters', (testEnv: TestEnv) => { ).to.be.revertedWith('MIN_AMOUNT_EXCEEDS_MAX_SLIPPAGE'); }); + it('should revert if wrong address used for Augustus', async () => { + const { users, weth, oracle, dai, aWETH, paraswapLiquiditySwapAdapter } = testEnv; + const user = users[0].signer; + const userAddress = users[0].address; + + const amountWETHtoSwap = await convertToCurrencyDecimals(weth.address, '10'); + + const daiPrice = await oracle.getAssetPrice(dai.address); + const expectedDaiAmount = await convertToCurrencyDecimals( + dai.address, + new BigNumber(amountWETHtoSwap.toString()).div(daiPrice.toString()).toFixed(0) + ); + + await mockAugustus.expectSwap(weth.address, dai.address, amountWETHtoSwap, amountWETHtoSwap, expectedDaiAmount); + + // User will swap liquidity aEth to aDai + await aWETH.connect(user).approve(paraswapLiquiditySwapAdapter.address, amountWETHtoSwap); + + const mockAugustusCalldata = mockAugustus.interface.encodeFunctionData( + 'swap', + [weth.address, dai.address, amountWETHtoSwap, expectedDaiAmount] + ); + + await expect( + paraswapLiquiditySwapAdapter.connect(user).swapAndDeposit( + weth.address, + dai.address, + amountWETHtoSwap, + expectedDaiAmount, + 0, + mockAugustusCalldata, + oracle.address, // using arbitrary contract instead of mock Augustus + { + amount: 0, + deadline: 0, + v: 0, + r: '0x0000000000000000000000000000000000000000000000000000000000000000', + s: '0x0000000000000000000000000000000000000000000000000000000000000000', + } + ) + ).to.be.revertedWith('INVALID_AUGUSTUS'); + }); + it('should bubble up errors from Augustus', async () => { const { users, weth, oracle, dai, aWETH, paraswapLiquiditySwapAdapter } = testEnv; const user = users[0].signer;