From 34d4b05245b70cdf0808e29e2289fb909dbb1470 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 1 Oct 2023 15:08:31 +0300 Subject: [PATCH 01/11] Use different selectors for various registries --- contracts/interfaces/ICurveRegistry.sol | 12 ++++ contracts/oracles/CurveOracle.sol | 96 +++++++++++++++++++++++-- test/helpers.js | 19 +++++ test/oracles/CurveOracle.js | 4 +- 4 files changed, 122 insertions(+), 9 deletions(-) diff --git a/contracts/interfaces/ICurveRegistry.sol b/contracts/interfaces/ICurveRegistry.sol index e81fc4c1..e5bacd2c 100644 --- a/contracts/interfaces/ICurveRegistry.sol +++ b/contracts/interfaces/ICurveRegistry.sol @@ -5,8 +5,20 @@ pragma solidity 0.8.19; // solhint-disable func-name-mixedcase interface ICurveRegistry { + // MAIN_REGISTRY, METAPOOL_FACTORY, CRYPTOSWAP_REGISTRY, CRYPTOPOOL_FACTORY, METAREGISTRY, CRVUSD_PLAIN_POOLS, CURVE_TRICRYPTO_FACTORY function find_pool_for_coins(address _srcToken, address _dstToken, uint256 _index) external view returns (address); + + // MAIN_REGISTRY, METAPOOL_FACTORY, METAREGISTRY, CRVUSD_PLAIN_POOLS function get_coin_indices(address _pool, address _srcToken, address _dstToken) external view returns (int128, int128, bool); + // CRYPTOSWAP_REGISTRY, CRYPTOPOOL_FACTORY, CURVE_TRICRYPTO_FACTORY - returns (uint256,uint256); + + // MAIN_REGISTRY, CRYPTOSWAP_REGISTRY, METAREGISTRY function get_balances(address _pool) external view returns (uint256[8] memory); + // METAPOOL_FACTORY, CRVUSD_PLAIN_POOLS - returns (uint256[4]); + // CURVE_TRICRYPTO_FACTORY - returns (uint256[3]); + // CRYPTOPOOL_FACTORY - returns (uint256[2]); + + // MAIN_REGISTRY, METAPOOL_FACTORY, METAREGISTRY, CRVUSD_PLAIN_POOLS function get_underlying_balances(address _pool) external view returns (uint256[8] memory); + // CRYPTOSWAP_REGISTRY, CRYPTOPOOL_FACTORY - NO METHOD } diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index 8b845e16..92c9583c 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -14,8 +14,18 @@ contract CurveOracle is IOracle { using OraclePrices for OraclePrices.Data; using Math for uint256; + enum CurveRegistryType { + MAIN_REGISTRY, + METAPOOL_FACTORY, + CRYPTOSWAP_REGISTRY, + CRYPTOPOOL_FACTORY, + METAREGISTRY, + CRVUSD_PLAIN_POOLS, + CURVE_TRICRYPTO_FACTORY + } + struct FunctionInfo { - function (address) external view returns (uint256[8] memory) balanceFunc; + bytes4 balanceFuncSelector; bytes4 dyFuncInt128Selector; function (uint256, uint256, uint256) external view returns (uint256) dyFuncUint256; } @@ -25,13 +35,15 @@ contract CurveOracle is IOracle { uint256 public immutable MAX_POOLS; uint256 public immutable REGISTRIES_COUNT; ICurveRegistry[11] public registries; + CurveRegistryType[11] public registryTypes; - constructor(ICurveProvider _addressProvider, uint256 _maxPools, uint256[] memory _registryIds) { + constructor(ICurveProvider _addressProvider, uint256 _maxPools, uint256[] memory _registryIds, CurveRegistryType[] memory _registryTypes) { MAX_POOLS = _maxPools; REGISTRIES_COUNT = _registryIds.length; unchecked { for (uint256 i = 0; i < REGISTRIES_COUNT; i++) { registries[i] = ICurveRegistry(_addressProvider.get_address(_registryIds[i])); + registryTypes[i] = _registryTypes[i]; } } } @@ -47,28 +59,98 @@ contract CurveOracle is IOracle { address pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), registryIndex); while (pool != address(0) && index < MAX_POOLS) { index++; - (int128 srcTokenIndex, int128 dstTokenIndex, bool isUnderlying) = registries[i].get_coin_indices(pool, address(srcToken), address(dstToken)); + // call `get_coin_indices` and set (srcTokenIndex, dstTokenIndex, isUnderlying) variables + bool isUnderlying; + int128 srcTokenIndex; + int128 dstTokenIndex; + (bool success, bytes memory data) = address(registries[i]).staticcall(abi.encodeWithSelector(ICurveRegistry.get_coin_indices.selector, pool, address(srcToken), address(dstToken))); + if (success && data.length >= 64) { + // TODO: here we can do only 3 checks from `else if` below + if ( + registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || + registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || + registryTypes[i] == CurveRegistryType.METAREGISTRY || + registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS + ) { + (srcTokenIndex, dstTokenIndex, isUnderlying) = abi.decode(data, (int128, int128, bool)); + } else if ( + registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || + registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY || + registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY + ) { + (uint256 _srcTokenIndex, uint256 _dstTokenIndex) = abi.decode(data, (uint256, uint256)); + (srcTokenIndex, dstTokenIndex) = (int128(int256(_srcTokenIndex)), int128(int256(_dstTokenIndex))); + } else { + pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); + continue; + } + } else { + pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); + continue; + } + if (!isUnderlying) { info = FunctionInfo({ - balanceFunc: registries[i].get_balances, + balanceFuncSelector: ICurveRegistry.get_balances.selector, dyFuncInt128Selector: ICurveSwap.get_dy.selector, dyFuncUint256: ICurveSwapNew(pool).get_dy }); } else { info = FunctionInfo({ - balanceFunc: registries[i].get_underlying_balances, + balanceFuncSelector: ICurveRegistry.get_underlying_balances.selector, dyFuncInt128Selector: ICurveSwap.get_dy_underlying.selector, dyFuncUint256: ICurveSwapNew(pool).get_dy_underlying }); } - uint256[8] memory balances = info.balanceFunc(pool); + // call `balanceFunc` (`get_balances` or `get_underlying_balances`) and decode results + uint256[8] memory balances; + (success, data) = address(registries[i]).staticcall(abi.encodeWithSelector(info.balanceFuncSelector, pool)); + if (success && data.length >= 64) { + // TODO: here we can do only 4 checks from `else if` conditions below + if ( + registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || + registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || + registryTypes[i] == CurveRegistryType.METAREGISTRY + ) { + balances = abi.decode(data, (uint256[8])); + } else if ( + registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || + registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS + ) { + uint256[4] memory _balances = abi.decode(data, (uint256[4])); + for (uint256 j = 0; j < 4; j++) { + balances[j] = _balances[j]; + } + } else if ( + registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY + ) { + uint256[3] memory _balances = abi.decode(data, (uint256[3])); + for (uint256 j = 0; j < 3; j++) { + balances[j] = _balances[j]; + } + } else if ( + registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY + ) { + uint256[2] memory _balances = abi.decode(data, (uint256[2])); + for (uint256 j = 0; j < 2; j++) { + balances[j] = _balances[j]; + } + } else { + pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); + continue; + } + } else { + pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); + continue; + } + uint256 w = (balances[uint128(srcTokenIndex)] * balances[uint128(dstTokenIndex)]).sqrt(); uint256 b0 = balances[uint128(srcTokenIndex)] / 10000; if (b0 != 0) { uint256 b1; - (bool success, bytes memory data) = pool.staticcall(abi.encodeWithSelector(info.dyFuncInt128Selector, srcTokenIndex, dstTokenIndex, b0)); + (success, data) = pool.staticcall(abi.encodeWithSelector(info.dyFuncInt128Selector, srcTokenIndex, dstTokenIndex, b0)); if (success && data.length == 32) { b1 = abi.decode(data, (uint256)); } else { diff --git a/test/helpers.js b/test/helpers.js index b184825f..86bd790f 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -58,6 +58,16 @@ const contracts = { chaiPot: '0x197E90f9FAD81970bA7976f33CbD77088E5D7cf7', }; +const CurveRegistryType = { + MAIN_REGISTRY: 0, + METAPOOL_FACTORY: 1, + CRYPTOSWAP_REGISTRY: 2, + CRYPTOPOOL_FACTORY: 3, + METAREGISTRY: 4, + CRVUSD_PLAIN_POOLS: 5, + CURVE_TRICRYPTO_FACTORY: 6, +}; + const deployParams = { AaveWrapperV2: { lendingPool: '0x7d2768dE32b0b80b7a3454c06BdAc94A69DDc7A9', @@ -79,6 +89,15 @@ const deployParams = { provider: '0x0000000022D53366457F9d5E68Ec105046FC4383', maxPools: 100, registryIds: [0, 3, 5, 6, 7, 8, 11], + registryTypes: [ + CurveRegistryType.MAIN_REGISTRY, + CurveRegistryType.METAPOOL_FACTORY, + CurveRegistryType.CRYPTOSWAP_REGISTRY, + CurveRegistryType.CRYPTOPOOL_FACTORY, + CurveRegistryType.METAREGISTRY, + CurveRegistryType.CRVUSD_PLAIN_POOLS, + CurveRegistryType.CURVE_TRICRYPTO_FACTORY, + ], }, Dodo: { dodoZoo: '0x3A97247DF274a17C59A3bd12735ea3FcDFb49950', diff --git a/test/oracles/CurveOracle.js b/test/oracles/CurveOracle.js index 97c24999..4b7eafa1 100644 --- a/test/oracles/CurveOracle.js +++ b/test/oracles/CurveOracle.js @@ -9,7 +9,7 @@ const { describe('CurveOracle', function () { async function initContracts () { - const curveOracle = await deployContract('CurveOracle', [Curve.provider, Curve.maxPools, Curve.registryIds]); + const curveOracle = await deployContract('CurveOracle', [Curve.provider, Curve.maxPools, Curve.registryIds, Curve.registryTypes]); const uniswapV3Oracle = await deployContract('UniswapV3LikeOracle', [UniswapV3.factory, UniswapV3.initcodeHash, UniswapV3.fees]); return { curveOracle, uniswapV3Oracle }; } @@ -41,7 +41,7 @@ describe('CurveOracle doesn\'t ruin rates', function () { const deployer = await ethers.getSigner(); const uniswapV2LikeOracle = await deployContract('UniswapV2LikeOracle', [UniswapV2.factory, UniswapV2.initcodeHash]); - const curveOracle = await deployContract('CurveOracle', [Curve.provider, Curve.maxPools, Curve.registryIds]); + const curveOracle = await deployContract('CurveOracle', [Curve.provider, Curve.maxPools, Curve.registryIds, Curve.registryTypes]); const uniswapOracle = await deployContract('UniswapOracle', [Uniswap.factory]); const mooniswapOracle = await deployContract('MooniswapOracle', [tokens.oneInchLP1]); const wethWrapper = await deployContract('BaseCoinWrapper', [tokens.ETH, tokens.WETH]); From 4de588adf61905920bbcb02ef50c16c3acac4a1b Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 1 Oct 2023 15:14:00 +0300 Subject: [PATCH 02/11] Refactor CurveSwap interfaces naming --- contracts/interfaces/ICurveSwap.sol | 4 ++-- contracts/oracles/CurveOracle.sol | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/contracts/interfaces/ICurveSwap.sol b/contracts/interfaces/ICurveSwap.sol index 433d9efa..40781a92 100644 --- a/contracts/interfaces/ICurveSwap.sol +++ b/contracts/interfaces/ICurveSwap.sol @@ -4,12 +4,12 @@ pragma solidity 0.8.19; // solhint-disable func-name-mixedcase -interface ICurveSwap { +interface ICurveSwapInt128 { function get_dy(int128 _from, int128 _to, uint256 _amount) external view returns (uint256); function get_dy_underlying(int128 _from, int128 _to, uint256 _amount) external view returns (uint256); } -interface ICurveSwapNew { +interface ICurveSwapUint256 { function get_dy(uint256 _from, uint256 _to, uint256 _amount) external view returns (uint256); function get_dy_underlying(uint256 _from, uint256 _to, uint256 _amount) external view returns (uint256); } diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index 92c9583c..8b381e75 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -92,14 +92,14 @@ contract CurveOracle is IOracle { if (!isUnderlying) { info = FunctionInfo({ balanceFuncSelector: ICurveRegistry.get_balances.selector, - dyFuncInt128Selector: ICurveSwap.get_dy.selector, - dyFuncUint256: ICurveSwapNew(pool).get_dy + dyFuncInt128Selector: ICurveSwapInt128.get_dy.selector, + dyFuncUint256: ICurveSwapUint256(pool).get_dy }); } else { info = FunctionInfo({ balanceFuncSelector: ICurveRegistry.get_underlying_balances.selector, - dyFuncInt128Selector: ICurveSwap.get_dy_underlying.selector, - dyFuncUint256: ICurveSwapNew(pool).get_dy_underlying + dyFuncInt128Selector: ICurveSwapInt128.get_dy_underlying.selector, + dyFuncUint256: ICurveSwapUint256(pool).get_dy_underlying }); } From 884fe747adf5cc1b6e2751c930920a556a0f700f Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 1 Oct 2023 19:09:50 +0300 Subject: [PATCH 03/11] Fix specific Curve case and add test --- contracts/oracles/CurveOracle.sol | 2 +- test/helpers.js | 2 ++ test/oracles/CurveOracle.js | 8 +++++++- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index 8b381e75..e44dfdcb 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -151,7 +151,7 @@ contract CurveOracle is IOracle { if (b0 != 0) { uint256 b1; (success, data) = pool.staticcall(abi.encodeWithSelector(info.dyFuncInt128Selector, srcTokenIndex, dstTokenIndex, b0)); - if (success && data.length == 32) { + if (success && data.length >= 32) { // vyper could return redundant bytes b1 = abi.decode(data, (uint256)); } else { b1 = info.dyFuncUint256(uint128(srcTokenIndex), uint128(dstTokenIndex), b0); diff --git a/test/helpers.js b/test/helpers.js index 86bd790f..4798d84c 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -45,6 +45,8 @@ const tokens = { yvWBTC: '0xA696a63cc78DfFa1a63E9E50587C197387FF6C7E', crvUSD: '0xf939E0A03FB07F59A73314E73794Be0E57ac1b4E', wstETH: '0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0', + BEAN: '0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab', + '3CRV': '0x6c3F90f043a72FA612cbac8115EE7e52BDe6E490', base: { DAI: '0x50c5725949A6F0c72E6C4a641F24049A917DB0Cb', WETH: '0x4200000000000000000000000000000000000006', diff --git a/test/oracles/CurveOracle.js b/test/oracles/CurveOracle.js index 4b7eafa1..96366b5d 100644 --- a/test/oracles/CurveOracle.js +++ b/test/oracles/CurveOracle.js @@ -1,6 +1,6 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const { ethers } = require('hardhat'); -const { assertRoughlyEqualValues, deployContract } = require('@1inch/solidity-utils'); +const { expect, assertRoughlyEqualValues, deployContract } = require('@1inch/solidity-utils'); const { tokens, deployParams: { AaveWrapperV2, Curve, Uniswap, UniswapV2, UniswapV3 }, @@ -34,6 +34,12 @@ describe('CurveOracle', function () { const rate = await curveOracle.getRate(tokens.WBTC, tokens.WETH, tokens.NONE, thresholdFilter); assertRoughlyEqualValues(rate.rate.toString(), expectedRate.rate.toString(), '0.05'); }); + + it('should use correct `get_dy` selector when vyper return redundant bytes', async function () { + const { curveOracle } = await loadFixture(initContracts); + const rate = await curveOracle.getRate(tokens.BEAN, tokens['3CRV'], tokens.NONE, thresholdFilter); + expect(rate.rate).to.gt('0'); + }); }); describe('CurveOracle doesn\'t ruin rates', function () { From 960c72f5125318455d437715417757f44522f1b6 Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 Oct 2023 00:14:54 +0300 Subject: [PATCH 04/11] Add tests various registries doesn't ruin with different selectors --- contracts/interfaces/ICurveRegistry.sol | 3 + test/oracles/CurveOracle.js | 80 ++++++++++++++++++++++++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/contracts/interfaces/ICurveRegistry.sol b/contracts/interfaces/ICurveRegistry.sol index e5bacd2c..91497fa4 100644 --- a/contracts/interfaces/ICurveRegistry.sol +++ b/contracts/interfaces/ICurveRegistry.sol @@ -5,6 +5,9 @@ pragma solidity 0.8.19; // solhint-disable func-name-mixedcase interface ICurveRegistry { + function pool_count() external view returns (uint256); + function pool_list(uint256 index) external view returns (address); + // MAIN_REGISTRY, METAPOOL_FACTORY, CRYPTOSWAP_REGISTRY, CRYPTOPOOL_FACTORY, METAREGISTRY, CRVUSD_PLAIN_POOLS, CURVE_TRICRYPTO_FACTORY function find_pool_for_coins(address _srcToken, address _dstToken, uint256 _index) external view returns (address); diff --git a/test/oracles/CurveOracle.js b/test/oracles/CurveOracle.js index 4b7eafa1..0c3fb52b 100644 --- a/test/oracles/CurveOracle.js +++ b/test/oracles/CurveOracle.js @@ -1,6 +1,6 @@ const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); const { ethers } = require('hardhat'); -const { assertRoughlyEqualValues, deployContract } = require('@1inch/solidity-utils'); +const { expect, assertRoughlyEqualValues, deployContract } = require('@1inch/solidity-utils'); const { tokens, deployParams: { AaveWrapperV2, Curve, Uniswap, UniswapV2, UniswapV3 }, @@ -34,6 +34,84 @@ describe('CurveOracle', function () { const rate = await curveOracle.getRate(tokens.WBTC, tokens.WETH, tokens.NONE, thresholdFilter); assertRoughlyEqualValues(rate.rate.toString(), expectedRate.rate.toString(), '0.05'); }); + + describe('doesn\'t ruin various registry with different selectors', function () { + it('Main Registry', async function () { + await testNotRuins(0, 2); + }); + + it('Metapool Factory', async function () { + await testNotRuins(1, 2); + }); + + it('Cryptoswap Registry', async function () { + await testNotRuins(2, 2); + }); + + it('Cryptopool Factory', async function () { + await testNotRuins(3, 2); + }); + + it('Metaregistry', async function () { + await testNotRuins(4, 2); + }); + + it('crvUSD Plain Pools', async function () { + await testNotRuins(5, 2); + }); + + it('Curve Tricrypto Factory', async function () { + await testNotRuins(6, 2); + }); + + async function testNotRuins (registryIndex, testPoolsAmount) { + const poolAbiUint256 = [ + { + name: 'coins', + type: 'function', + inputs: [{ type: 'uint256', name: 'arg0' }], + outputs: [{ type: 'address', name: 'value' }], + stateMutability: 'view', + }, + ]; + const poolAbiInt128 = [ + { + name: 'coins', + type: 'function', + inputs: [{ type: 'int128', name: 'arg0' }], + outputs: [{ type: 'address', name: 'value' }], + stateMutability: 'view', + }, + ]; + + const curveOracle = await deployContract('CurveOracle', [Curve.provider, Curve.maxPools, [Curve.registryIds[registryIndex]], [Curve.registryTypes[registryIndex]]]); + const curveProvider = await ethers.getContractAt('ICurveProvider', Curve.provider); + const registryAddress = await curveProvider.get_address(Curve.registryIds[registryIndex]); + const registry = await ethers.getContractAt('ICurveRegistry', registryAddress); + + const poolCount = await registry.pool_count(); + + // we check only `testPoolsAmount` random pools from the registry to save time + for (let i = 0; i < poolCount; i += Math.ceil(poolCount / testPoolsAmount)) { + const poolAddress = await registry.pool_list(i); + let token0, token1; + try { + const poolUint256 = await ethers.getContractAt(poolAbiUint256, poolAddress); + token0 = await poolUint256.coins(0); + token1 = await poolUint256.coins(1); + } catch (e) { + try { + const poolInt128 = await ethers.getContractAt(poolAbiInt128, poolAddress); + token0 = await poolInt128.coins(0); + token1 = await poolInt128.coins(1); + } catch (e) { + expect.fail(`pool ${i} ${poolAddress} doesn't work with uint256 and int128 selectors of \`coins\` method`); + } + } + await curveOracle.getRate(token0, token1, tokens.NONE, thresholdFilter); + } + } + }); }); describe('CurveOracle doesn\'t ruin rates', function () { From cfc8bf18fe4a649b4f6d19bbee1a3c6ce34db029 Mon Sep 17 00:00:00 2001 From: Denis Date: Tue, 3 Oct 2023 00:44:13 +0300 Subject: [PATCH 05/11] Increase mocha timeout --- hardhat.config.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hardhat.config.js b/hardhat.config.js index b049edae..84b7be9b 100644 --- a/hardhat.config.js +++ b/hardhat.config.js @@ -36,7 +36,7 @@ module.exports = { deploy: 'deploy/commands', }, mocha: { - timeout: 90000, + timeout: 120000, }, tracer: { enableAllOpcodes: true, From 5b781507202907dacbebc7a871add24b75fea109 Mon Sep 17 00:00:00 2001 From: zZoMROT Date: Tue, 10 Oct 2023 11:20:27 +0300 Subject: [PATCH 06/11] Apply suggestions from code review Co-authored-by: Mikhail Melnik --- contracts/oracles/CurveOracle.sol | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index e44dfdcb..26b64cdc 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -78,8 +78,7 @@ contract CurveOracle is IOracle { registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY || registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY ) { - (uint256 _srcTokenIndex, uint256 _dstTokenIndex) = abi.decode(data, (uint256, uint256)); - (srcTokenIndex, dstTokenIndex) = (int128(int256(_srcTokenIndex)), int128(int256(_dstTokenIndex))); + (srcTokenIndex, dstTokenIndex) = abi.decode(data, (int128, int128)); } else { pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); continue; From 74ba7a016a98e8aea4fb48b0dd6cb94e0ce884a8 Mon Sep 17 00:00:00 2001 From: Denis Date: Thu, 12 Oct 2023 22:39:13 +0300 Subject: [PATCH 07/11] Fix using get_dy in pools --- contracts/oracles/CurveOracle.sol | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index 26b64cdc..68ce6410 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -146,9 +146,9 @@ contract CurveOracle is IOracle { uint256 w = (balances[uint128(srcTokenIndex)] * balances[uint128(dstTokenIndex)]).sqrt(); uint256 b0 = balances[uint128(srcTokenIndex)] / 10000; + uint256 b1 = balances[uint128(dstTokenIndex)] / 10000; - if (b0 != 0) { - uint256 b1; + if (b0 != 0 && b1 != 0) { (success, data) = pool.staticcall(abi.encodeWithSelector(info.dyFuncInt128Selector, srcTokenIndex, dstTokenIndex, b0)); if (success && data.length >= 32) { // vyper could return redundant bytes b1 = abi.decode(data, (uint256)); From 7e3c7661331574ad411dcae66e36816e70163307 Mon Sep 17 00:00:00 2001 From: Denis Date: Thu, 12 Oct 2023 22:49:55 +0300 Subject: [PATCH 08/11] Refactor ifs for decode result of get_coin_indices and get_balances methods --- contracts/oracles/CurveOracle.sol | 39 ++++++++++--------------------- 1 file changed, 12 insertions(+), 27 deletions(-) diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index 68ce6410..b0a918a2 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -65,23 +65,18 @@ contract CurveOracle is IOracle { int128 dstTokenIndex; (bool success, bytes memory data) = address(registries[i]).staticcall(abi.encodeWithSelector(ICurveRegistry.get_coin_indices.selector, pool, address(srcToken), address(dstToken))); if (success && data.length >= 64) { - // TODO: here we can do only 3 checks from `else if` below if ( - registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || - registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || - registryTypes[i] == CurveRegistryType.METAREGISTRY || - registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS - ) { - (srcTokenIndex, dstTokenIndex, isUnderlying) = abi.decode(data, (int128, int128, bool)); - } else if ( registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY || registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY ) { (srcTokenIndex, dstTokenIndex) = abi.decode(data, (int128, int128)); } else { - pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); - continue; + // registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || + // registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || + // registryTypes[i] == CurveRegistryType.METAREGISTRY || + // registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS + (srcTokenIndex, dstTokenIndex, isUnderlying) = abi.decode(data, (int128, int128, bool)); } } else { pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); @@ -106,38 +101,28 @@ contract CurveOracle is IOracle { uint256[8] memory balances; (success, data) = address(registries[i]).staticcall(abi.encodeWithSelector(info.balanceFuncSelector, pool)); if (success && data.length >= 64) { - // TODO: here we can do only 4 checks from `else if` conditions below - if ( - registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || - registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || - registryTypes[i] == CurveRegistryType.METAREGISTRY - ) { - balances = abi.decode(data, (uint256[8])); - } else if ( - registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || + if (registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS ) { uint256[4] memory _balances = abi.decode(data, (uint256[4])); for (uint256 j = 0; j < 4; j++) { balances[j] = _balances[j]; } - } else if ( - registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY - ) { + } else if (registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY) { uint256[3] memory _balances = abi.decode(data, (uint256[3])); for (uint256 j = 0; j < 3; j++) { balances[j] = _balances[j]; } - } else if ( - registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY - ) { + } else if (registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY) { uint256[2] memory _balances = abi.decode(data, (uint256[2])); for (uint256 j = 0; j < 2; j++) { balances[j] = _balances[j]; } } else { - pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); - continue; + // registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || + // registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || + // registryTypes[i] == CurveRegistryType.METAREGISTRY + balances = abi.decode(data, (uint256[8])); } } else { pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); From b27dd4914c7b07712f736b2269ff72c535388ade Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 15 Oct 2023 00:36:29 +0300 Subject: [PATCH 09/11] Add memory-safe in assembly --- contracts/OffchainOracle.sol | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contracts/OffchainOracle.sol b/contracts/OffchainOracle.sol index ac68905a..a0af4744 100644 --- a/contracts/OffchainOracle.sol +++ b/contracts/OffchainOracle.sol @@ -430,7 +430,7 @@ contract OffchainOracle is Ownable { uint256 prod0; // Least significant 256 bits of the product uint256 prod1; // Most significant 256 bits of the product // solhint-disable-next-line no-inline-assembly - assembly ("memory-safe") { + assembly ("memory-safe") { // solhint-disable-line no-inline-assembly let mm := mulmod(x, y, not(0)) prod0 := mul(x, y) prod1 := sub(sub(mm, prod0), lt(mm, prod0)) From c747ba73c6ee235e7fad957bfa50a2fc12cf5851 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 15 Oct 2023 00:41:11 +0300 Subject: [PATCH 10/11] Optimise decoding of balances --- contracts/oracles/CurveOracle.sol | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index b0a918a2..25efb274 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -98,31 +98,30 @@ contract CurveOracle is IOracle { } // call `balanceFunc` (`get_balances` or `get_underlying_balances`) and decode results - uint256[8] memory balances; + uint256[] memory balances; (success, data) = address(registries[i]).staticcall(abi.encodeWithSelector(info.balanceFuncSelector, pool)); if (success && data.length >= 64) { + assembly ("memory-safe") { // solhint-disable-line no-inline-assembly + balances := add(data, 0x20) + } + + uint256 length; if (registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || - registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS - ) { - uint256[4] memory _balances = abi.decode(data, (uint256[4])); - for (uint256 j = 0; j < 4; j++) { - balances[j] = _balances[j]; - } + registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS) { + length = 4; } else if (registryTypes[i] == CurveRegistryType.CURVE_TRICRYPTO_FACTORY) { - uint256[3] memory _balances = abi.decode(data, (uint256[3])); - for (uint256 j = 0; j < 3; j++) { - balances[j] = _balances[j]; - } + length = 3; } else if (registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY) { - uint256[2] memory _balances = abi.decode(data, (uint256[2])); - for (uint256 j = 0; j < 2; j++) { - balances[j] = _balances[j]; - } + length = 2; } else { // registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || // registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || // registryTypes[i] == CurveRegistryType.METAREGISTRY - balances = abi.decode(data, (uint256[8])); + length = 8; + } + + assembly ("memory-safe") { // solhint-disable-line no-inline-assembly + mstore(balances, length) } } else { pool = registries[i].find_pool_for_coins(address(srcToken), address(dstToken), ++registryIndex); From f8be0d1d5a793dc63cf9a35311212abe7e2667d6 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 15 Oct 2023 19:59:20 +0300 Subject: [PATCH 11/11] Fix and micro optimisation --- contracts/oracles/CurveOracle.sol | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/contracts/oracles/CurveOracle.sol b/contracts/oracles/CurveOracle.sol index 25efb274..22ff17cf 100644 --- a/contracts/oracles/CurveOracle.sol +++ b/contracts/oracles/CurveOracle.sol @@ -101,11 +101,10 @@ contract CurveOracle is IOracle { uint256[] memory balances; (success, data) = address(registries[i]).staticcall(abi.encodeWithSelector(info.balanceFuncSelector, pool)); if (success && data.length >= 64) { - assembly ("memory-safe") { // solhint-disable-line no-inline-assembly - balances := add(data, 0x20) - } - - uint256 length; + // registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || + // registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || + // registryTypes[i] == CurveRegistryType.METAREGISTRY + uint256 length = 8; if (registryTypes[i] == CurveRegistryType.METAPOOL_FACTORY || registryTypes[i] == CurveRegistryType.CRVUSD_PLAIN_POOLS) { length = 4; @@ -113,14 +112,10 @@ contract CurveOracle is IOracle { length = 3; } else if (registryTypes[i] == CurveRegistryType.CRYPTOPOOL_FACTORY) { length = 2; - } else { - // registryTypes[i] == CurveRegistryType.MAIN_REGISTRY || - // registryTypes[i] == CurveRegistryType.CRYPTOSWAP_REGISTRY || - // registryTypes[i] == CurveRegistryType.METAREGISTRY - length = 8; } assembly ("memory-safe") { // solhint-disable-line no-inline-assembly + balances := data mstore(balances, length) } } else {