diff options
26 files changed, 688 insertions, 45 deletions
diff --git a/CODEOWNERS b/CODEOWNERS index 278234aff..ba40af31a 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -26,3 +26,4 @@ packages/subproviders/ @fabioberger @dekz packages/connect/ @fragosti packages/monorepo-scripts/ @fabioberger packages/order-utils/ @fabioberger @LogvinovLeon +python-packages/ @feuGeneA
\ No newline at end of file diff --git a/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol b/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol index fea9a53c2..4991c0ea5 100644 --- a/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol +++ b/packages/contracts/contracts/extensions/Forwarder/MixinExchangeWrapper.sol @@ -155,8 +155,10 @@ contract MixinExchangeWrapper is uint256 remainingMakerAssetFillAmount = safeSub(makerAssetFillAmount, totalFillResults.makerAssetFilledAmount); // Convert the remaining amount of makerAsset to buy into remaining amount - // of takerAsset to sell, assuming entire amount can be sold in the current order - uint256 remainingTakerAssetFillAmount = getPartialAmountFloor( + // of takerAsset to sell, assuming entire amount can be sold in the current order. + // We round up because the exchange rate computed by fillOrder rounds in favor + // of the Maker. In this case we want to overestimate the amount of takerAsset. + uint256 remainingTakerAssetFillAmount = getPartialAmountCeil( orders[i].takerAssetAmount, orders[i].makerAssetAmount, remainingMakerAssetFillAmount @@ -224,7 +226,9 @@ contract MixinExchangeWrapper is // Convert the remaining amount of ZRX to buy into remaining amount // of WETH to sell, assuming entire amount can be sold in the current order. - uint256 remainingWethSellAmount = getPartialAmountFloor( + // We round up because the exchange rate computed by fillOrder rounds in favor + // of the Maker. In this case we want to overestimate the amount of takerAsset. + uint256 remainingWethSellAmount = getPartialAmountCeil( orders[i].takerAssetAmount, safeSub(orders[i].makerAssetAmount, orders[i].takerFee), // our exchange rate after fees remainingZrxBuyAmount @@ -233,7 +237,7 @@ contract MixinExchangeWrapper is // Attempt to sell the remaining amount of WETH. FillResults memory singleFillResult = fillOrderNoThrow( orders[i], - safeAdd(remainingWethSellAmount, 1), // we add 1 wei to the fill amount to make up for rounding errors + remainingWethSellAmount, signatures[i] ); diff --git a/packages/contracts/test/extensions/forwarder.ts b/packages/contracts/test/extensions/forwarder.ts index b76624fa9..c006be0fe 100644 --- a/packages/contracts/test/extensions/forwarder.ts +++ b/packages/contracts/test/extensions/forwarder.ts @@ -45,6 +45,7 @@ describe(ContractName.Forwarder, () => { let weth: DummyERC20TokenContract; let zrxToken: DummyERC20TokenContract; + let erc20TokenA: DummyERC20TokenContract; let erc721Token: DummyERC721TokenContract; let forwarderContract: ForwarderContract; let wethContract: WETH9Contract; @@ -77,7 +78,6 @@ describe(ContractName.Forwarder, () => { erc20Wrapper = new ERC20Wrapper(provider, usedAddresses, owner); const numDummyErc20ToDeploy = 3; - let erc20TokenA; [erc20TokenA, zrxToken] = await erc20Wrapper.deployDummyTokensAsync( numDummyErc20ToDeploy, constants.DUMMY_TOKEN_DECIMALS, @@ -902,6 +902,269 @@ describe(ContractName.Forwarder, () => { ); expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded', async () => { + // The 0x Protocol contracts round the exchange rate in favor of the Maker. + // In this case, the taker must round up how much they're going to spend, which + // in turn increases the amount of MakerAsset being purchased. + // Example: + // The taker wants to buy 5 units of the MakerAsset at a rate of 3M/2T. + // For every 2 units of TakerAsset, the taker will receive 3 units of MakerAsset. + // To purchase 5 units, the taker must spend 10/3 = 3.33 units of TakerAssset. + // However, the Taker can only spend whole units. + // Spending floor(10/3) = 3 units will yield a profit of Floor(3*3/2) = Floor(4.5) = 4 units of MakerAsset. + // Spending ceil(10/3) = 4 units will yield a profit of Floor(4*3/2) = 6 units of MakerAsset. + // + // The forwarding contract will opt for the second option, which overbuys, to ensure the taker + // receives at least the amount of MakerAsset they requested. + // + // Construct test case using values from example above + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('30'), + takerAssetAmount: new BigNumber('20'), + makerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenA.address), + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const desiredMakerAssetFillAmount = new BigNumber('5'); + const makerAssetFillAmount = new BigNumber('6'); + const ethValue = new BigNumber('4'); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded, and MakerAsset is ZRX', async () => { + // See the test case above for a detailed description of this case. + // The difference here is that the MakerAsset is ZRX. We expect the same result as above, + // but this tests a different code path. + // + // Construct test case using values from example above + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('30'), + takerAssetAmount: new BigNumber('20'), + makerAssetData: zrxAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const desiredMakerAssetFillAmount = new BigNumber('5'); + const makerAssetFillAmount = new BigNumber('6'); + const ethValue = new BigNumber('4'); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded (Regression Test)', async () => { + // Order taken from a transaction on mainnet that failed due to a rounding error. + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('268166666666666666666'), + takerAssetAmount: new BigNumber('219090625878836371'), + makerAssetData: assetDataUtils.encodeERC20AssetData(erc20TokenA.address), + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + // The taker will receive more than the desired amount of makerAsset due to rounding + const desiredMakerAssetFillAmount = new BigNumber('5000000000000000000'); + const ethValue = new BigNumber('4084971271824171'); + const makerAssetFillAmount = ethValue + .times(orderWithoutFee.makerAssetAmount) + .dividedToIntegerBy(orderWithoutFee.takerAssetAmount); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[makerAddress][defaultMakerAssetAddress].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][defaultMakerAssetAddress]).to.be.bignumber.equal( + erc20Balances[takerAddress][defaultMakerAssetAddress].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(newBalances[forwarderContract.address][defaultMakerAssetAddress]).to.be.bignumber.equal( + constants.ZERO_AMOUNT, + ); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy slightly greater MakerAsset when exchange rate is rounded, and MakerAsset is ZRX (Regression Test)', async () => { + // Order taken from a transaction on mainnet that failed due to a rounding error. + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('268166666666666666666'), + takerAssetAmount: new BigNumber('219090625878836371'), + makerAssetData: zrxAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + // The taker will receive more than the desired amount of makerAsset due to rounding + const desiredMakerAssetFillAmount = new BigNumber('5000000000000000000'); + const ethValue = new BigNumber('4084971271824171'); + const makerAssetFillAmount = ethValue + .times(orderWithoutFee.makerAssetAmount) + .dividedToIntegerBy(orderWithoutFee.takerAssetAmount); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync( + ordersWithoutFee, + feeOrders, + desiredMakerAssetFillAmount, + { + value: ethValue, + from: takerAddress, + }, + ); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(makerAssetFillAmount).to.be.bignumber.greaterThan(desiredMakerAssetFillAmount); + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); + it('Should buy correct MakerAsset when exchange rate is NOT rounded, and MakerAsset is ZRX (Regression Test)', async () => { + // An extra unit of TakerAsset was sent to the exchange contract to account for rounding errors, in Forwarder v1. + // Specifically, the takerFillAmount was calculated using Floor(desiredMakerAmount * exchangeRate) + 1 + // We have since changed this to be Ceil(desiredMakerAmount * exchangeRate) + // These calculations produce different results when `desiredMakerAmount * exchangeRate` is an integer. + // + // This test verifies that `ceil` is sufficient: + // Let TakerAssetAmount = MakerAssetAmount * 2 + // -> exchangeRate = TakerAssetAmount / MakerAssetAmount = (2*MakerAssetAmount)/MakerAssetAmount = 2 + // .: desiredMakerAmount * exchangeRate is an integer. + // + // Construct test case using values from example above + orderWithoutFee = await orderFactory.newSignedOrderAsync({ + makerAssetAmount: new BigNumber('30'), + takerAssetAmount: new BigNumber('60'), + makerAssetData: zrxAssetData, + takerAssetData: assetDataUtils.encodeERC20AssetData(weth.address), + makerFee: new BigNumber(0), + takerFee: new BigNumber(0), + }); + const ordersWithoutFee = [orderWithoutFee]; + const feeOrders: SignedOrder[] = []; + const makerAssetFillAmount = new BigNumber('5'); + const ethValue = new BigNumber('10'); + // Execute test case + tx = await forwarderWrapper.marketBuyOrdersWithEthAsync(ordersWithoutFee, feeOrders, makerAssetFillAmount, { + value: ethValue, + from: takerAddress, + }); + // Fetch end balances and construct expected outputs + const takerEthBalanceAfter = await web3Wrapper.getBalanceInWeiAsync(takerAddress); + const forwarderEthBalance = await web3Wrapper.getBalanceInWeiAsync(forwarderContract.address); + const newBalances = await erc20Wrapper.getBalancesAsync(); + const primaryTakerAssetFillAmount = ethValue; + const totalEthSpent = primaryTakerAssetFillAmount.plus(gasPrice.times(tx.gasUsed)); + // Validate test case + expect(takerEthBalanceAfter).to.be.bignumber.equal(takerEthBalanceBefore.minus(totalEthSpent)); + expect(newBalances[makerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][zrxToken.address].minus(makerAssetFillAmount), + ); + expect(newBalances[takerAddress][zrxToken.address]).to.be.bignumber.equal( + erc20Balances[takerAddress][zrxToken.address].plus(makerAssetFillAmount), + ); + expect(newBalances[makerAddress][weth.address]).to.be.bignumber.equal( + erc20Balances[makerAddress][weth.address].plus(primaryTakerAssetFillAmount), + ); + expect(newBalances[forwarderContract.address][weth.address]).to.be.bignumber.equal(constants.ZERO_AMOUNT); + expect(forwarderEthBalance).to.be.bignumber.equal(constants.ZERO_AMOUNT); + }); }); describe('marketBuyOrdersWithEth with extra fees', () => { it('should buy an asset and send fee to feeRecipient', async () => { diff --git a/packages/contracts/test/utils/forwarder_wrapper.ts b/packages/contracts/test/utils/forwarder_wrapper.ts index f1a64d47d..a0bfcfe1d 100644 --- a/packages/contracts/test/utils/forwarder_wrapper.ts +++ b/packages/contracts/test/utils/forwarder_wrapper.ts @@ -26,9 +26,12 @@ export class ForwarderWrapper { _.forEach(feeOrders, feeOrder => { const feeAvailable = feeOrder.makerAssetAmount.minus(feeOrder.takerFee); if (!remainingFeeAmount.isZero() && feeAvailable.gt(remainingFeeAmount)) { - wethAmount = wethAmount - .plus(feeOrder.takerAssetAmount.times(remainingFeeAmount).dividedToIntegerBy(feeAvailable)) - .plus(1); + wethAmount = wethAmount.plus( + feeOrder.takerAssetAmount + .times(remainingFeeAmount) + .dividedBy(feeAvailable) + .ceil(), + ); remainingFeeAmount = new BigNumber(0); } else if (!remainingFeeAmount.isZero()) { wethAmount = wethAmount.plus(feeOrder.takerAssetAmount); diff --git a/packages/order-utils/test/asset_data_utils_test.ts b/packages/order-utils/test/asset_data_utils_test.ts new file mode 100644 index 000000000..f8b850604 --- /dev/null +++ b/packages/order-utils/test/asset_data_utils_test.ts @@ -0,0 +1,31 @@ +import * as chai from 'chai'; + +import { ERC20AssetData } from '@0x/types'; + +import { assetDataUtils } from '../src/asset_data_utils'; + +import { chaiSetup } from './utils/chai_setup'; + +chaiSetup.configure(); +const expect = chai.expect; + +const KNOWN_ENCODINGS = [ + { + address: '0x1dc4c1cefef38a777b15aa20260a54e584b16c48', + assetData: '0xf47261b00000000000000000000000001dc4c1cefef38a777b15aa20260a54e584b16c48', + }, +]; + +const ERC20_ASSET_PROXY_ID = '0xf47261b0'; + +describe('assetDataUtils', () => { + it('should encode', () => { + const assetData = assetDataUtils.encodeERC20AssetData(KNOWN_ENCODINGS[0].address); + expect(assetData).to.equal(KNOWN_ENCODINGS[0].assetData); + }); + it('should decode', () => { + const assetData: ERC20AssetData = assetDataUtils.decodeERC20AssetData(KNOWN_ENCODINGS[0].assetData); + expect(assetData.tokenAddress).to.equal(KNOWN_ENCODINGS[0].address); + expect(assetData.assetProxyId).to.equal(ERC20_ASSET_PROXY_ID); + }); +}); diff --git a/python-packages/order_utils/setup.py b/python-packages/order_utils/setup.py index a76d724aa..1a094cfe1 100644..100755 --- a/python-packages/order_utils/setup.py +++ b/python-packages/order_utils/setup.py @@ -1,13 +1,16 @@ +#!/usr/bin/env python + """setuptools module for order_utils package.""" import subprocess # nosec from shutil import rmtree -from os import path, remove, walk +from os import environ, path, remove, walk +from sys import argv -from distutils.command.clean import clean # type: ignore -from setuptools import setup # type: ignore -import setuptools.command.build_py # type: ignore -from setuptools.command.test import test as TestCommand # type: ignore +from distutils.command.clean import clean +import distutils.command.build_py +from setuptools import setup +from setuptools.command.test import test as TestCommand class TestCommandExtension(TestCommand): @@ -15,13 +18,13 @@ class TestCommandExtension(TestCommand): def run_tests(self): """Invoke pytest.""" - import pytest # type: ignore + import pytest pytest.main() # pylint: disable=too-many-ancestors -class LintCommand(setuptools.command.build_py.build_py): +class LintCommand(distutils.command.build_py.build_py): """Custom setuptools command class for running linters.""" def run(self): @@ -34,7 +37,7 @@ class LintCommand(setuptools.command.build_py.build_py): # docstring style checker: "pydocstyle src test setup.py".split(), # static type checker: - "mypy src setup.py".split(), + "mypy src test setup.py".split(), # security issue checker: "bandit -r src ./setup.py".split(), # general linter: @@ -42,6 +45,21 @@ class LintCommand(setuptools.command.build_py.build_py): # pylint takes relatively long to run, so it runs last, to enable # fast failures. ] + + # tell mypy where to find interface stubs for 3rd party libs + environ["MYPYPATH"] = path.join( + path.dirname(path.realpath(argv[0])), "stubs" + ) + + # HACK(gene): until eth_abi releases + # https://github.com/ethereum/eth-abi/pull/107 , we need to simply + # create an empty file `py.typed` in the eth_abi package directory. + import eth_abi + + eth_abi_dir = path.dirname(path.realpath(eth_abi.__file__)) + with open(path.join(eth_abi_dir, "py.typed"), "a"): + pass + for lint_command in lint_commands: print( "Running lint command `", " ".join(lint_command).strip(), "`" @@ -79,7 +97,7 @@ setup( "test": TestCommandExtension, }, include_package_data=True, - install_requires=["web3"], + install_requires=["eth-abi", "web3"], extras_require={ "dev": [ "bandit", @@ -87,6 +105,7 @@ setup( "coverage", "coveralls", "mypy", + "mypy_extensions", "pycodestyle", "pydocstyle", "pylint", @@ -118,7 +137,7 @@ setup( "Topic :: Software Development :: Libraries", "Topic :: Utilities", ], - zip_safe=False, + zip_safe=False, # required per mypy command_options={ "build_sphinx": { "source_dir": ("setup.py", "src"), diff --git a/python-packages/order_utils/src/conf.py b/python-packages/order_utils/src/conf.py index f3f15967c..e74a29d00 100644 --- a/python-packages/order_utils/src/conf.py +++ b/python-packages/order_utils/src/conf.py @@ -2,6 +2,9 @@ # Reference: http://www.sphinx-doc.org/en/master/config +from typing import List + + # pylint: disable=invalid-name # because these variables are not named in upper case, as globals should be. @@ -29,7 +32,7 @@ master_doc = "index" # The master toctree document. language = None -exclude_patterns = [] # type: ignore +exclude_patterns: List[str] = [] # The name of the Pygments (syntax highlighting) style to use. pygments_style = None diff --git a/python-packages/order_utils/src/index.rst b/python-packages/order_utils/src/index.rst index cbc4c8409..e09abcca2 100644 --- a/python-packages/order_utils/src/index.rst +++ b/python-packages/order_utils/src/index.rst @@ -10,9 +10,12 @@ order_utils.py .. automodule:: zero_ex.order_utils :members: -.. automodule:: zero_ex.order_utils.signature_utils +.. automodule:: zero_ex.order_utils.asset_data_utils :members: +.. autoclass:: zero_ex.order_utils.asset_data_utils.ERC20AssetData + +See source for properties. Sphinx does not easily generate class property docs; pull requests welcome. Indices and tables ================== diff --git a/python-packages/order_utils/src/zero_ex/dev_utils/__init__.py b/python-packages/order_utils/src/zero_ex/dev_utils/__init__.py new file mode 100644 index 000000000..b6a224d2c --- /dev/null +++ b/python-packages/order_utils/src/zero_ex/dev_utils/__init__.py @@ -0,0 +1 @@ +"""Dev utils to be shared across 0x projects and packages.""" diff --git a/python-packages/order_utils/src/zero_ex/dev_utils/abi_utils.py b/python-packages/order_utils/src/zero_ex/dev_utils/abi_utils.py new file mode 100644 index 000000000..71b6128ca --- /dev/null +++ b/python-packages/order_utils/src/zero_ex/dev_utils/abi_utils.py @@ -0,0 +1,102 @@ +"""Ethereum ABI utilities. + +Builds on the eth-abi package, adding some convenience methods like those found +in npmjs.com/package/ethereumjs-abi. Ideally, all of this code should be +pushed upstream into eth-abi. +""" + +import re +from typing import Any, List + +from mypy_extensions import TypedDict + +from eth_abi import encode_abi +from web3 import Web3 + +from .type_assertions import assert_is_string, assert_is_list + + +class MethodSignature(TypedDict, total=False): + """Object interface to an ABI method signature.""" + + method: str + args: List[str] + + +def parse_signature(signature: str) -> MethodSignature: + """Parse a method signature into its constituent parts. + + >>> parse_signature("ERC20Token(address)") + {'method': 'ERC20Token', 'args': ['address']} + """ + assert_is_string(signature, "signature") + + matches = re.match(r"^(\w+)\((.+)\)$", signature) + if matches is None: + raise ValueError(f"Invalid method signature {signature}") + return {"method": matches[1], "args": matches[2].split(",")} + + +def elementary_name(name: str) -> str: + """Convert from short to canonical names; barely implemented. + + Modeled after ethereumjs-abi's ABI.elementaryName(), but only implemented + to support our particular use case and a few other simple ones. + + >>> elementary_name("address") + 'address' + >>> elementary_name("uint") + 'uint256' + """ + assert_is_string(name, "name") + + return { + "int": "int256", + "uint": "uint256", + "fixed": "fixed128x128", + "ufixed": "ufixed128x128", + }.get(name, name) + + +def event_id(name: str, types: List[str]) -> str: + """Return the Keccak-256 hash of the given method. + + >>> event_id("ERC20Token", ["address"]) + '0xf47261b06eedbfce68afd46d0f3c27c60b03faad319eaf33103611cf8f6456ad' + """ + assert_is_string(name, "name") + assert_is_list(types, "types") + + signature = f"{name}({','.join(list(map(elementary_name, types)))})" + return Web3.sha3(text=signature).hex() + + +def method_id(name: str, types: List[str]) -> str: + """Return the 4-byte method identifier. + + >>> method_id("ERC20Token", ["address"]) + '0xf47261b0' + """ + assert_is_string(name, "name") + assert_is_list(types, "types") + + return event_id(name, types)[0:10] + + +def simple_encode(method: str, *args: Any) -> bytes: + # docstring considered all one line by pylint: disable=line-too-long + r"""Encode a method ABI. + + >>> simple_encode("ERC20Token(address)", "0x1dc4c1cefef38a777b15aa20260a54e584b16c48") + b'\xf4ra\xb0\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x1d\xc4\xc1\xce\xfe\xf3\x8aw{\x15\xaa &\nT\xe5\x84\xb1lH' + """ # noqa: E501 (line too long) + assert_is_string(method, "method") + + signature: MethodSignature = parse_signature(method) + + return bytes.fromhex( + ( + method_id(signature["method"], signature["args"]) + + encode_abi(signature["args"], args).hex() + )[2:] + ) diff --git a/python-packages/order_utils/src/zero_ex/dev_utils/type_assertions.py b/python-packages/order_utils/src/zero_ex/dev_utils/type_assertions.py new file mode 100644 index 000000000..745d014e6 --- /dev/null +++ b/python-packages/order_utils/src/zero_ex/dev_utils/type_assertions.py @@ -0,0 +1,33 @@ +"""Assertions for runtime type checking of function arguments.""" + +from typing import Any + + +def assert_is_string(value: Any, name: str) -> None: + """If :param value: isn't of type str, raise a TypeError. + + >>> try: assert_is_string(123, 'var') + ... except TypeError as type_error: print(str(type_error)) + ... + expected variable 'var', with value 123, to have type 'str', not 'int' + """ + if not isinstance(value, str): + raise TypeError( + f"expected variable '{name}', with value {str(value)}, to have" + + f" type 'str', not '{type(value).__name__}'" + ) + + +def assert_is_list(value: Any, name: str) -> None: + """If :param value: isn't of type list, raise a TypeError. + + >>> try: assert_is_list(123, 'var') + ... except TypeError as type_error: print(str(type_error)) + ... + expected variable 'var', with value 123, to have type 'list', not 'int' + """ + if not isinstance(value, list): + raise TypeError( + f"expected variable '{name}', with value {str(value)}, to have" + + f" type 'list', not '{type(value).__name__}'" + ) diff --git a/python-packages/order_utils/src/zero_ex/order_utils/asset_data_utils.py b/python-packages/order_utils/src/zero_ex/order_utils/asset_data_utils.py new file mode 100644 index 000000000..451de39af --- /dev/null +++ b/python-packages/order_utils/src/zero_ex/order_utils/asset_data_utils.py @@ -0,0 +1,72 @@ +"""Asset data encoding and decoding utilities.""" + +from mypy_extensions import TypedDict + +import eth_abi + +from zero_ex.dev_utils import abi_utils +from zero_ex.dev_utils.type_assertions import assert_is_string + + +ERC20_ASSET_DATA_BYTE_LENGTH = 36 +SELECTOR_LENGTH = 10 + + +class ERC20AssetData(TypedDict): + """Object interface to ERC20 asset data.""" + + asset_proxy_id: str + token_address: str + + +def encode_erc20_asset_data(token_address: str) -> str: + """Encode an ERC20 token address into an asset data string. + + :param token_address: the ERC20 token's contract address. + :rtype: hex encoded asset data string, usable in the makerAssetData or + takerAssetData fields in a 0x order. + + >>> encode_erc20_asset_data('0x1dc4c1cefef38a777b15aa20260a54e584b16c48') + '0xf47261b00000000000000000000000001dc4c1cefef38a777b15aa20260a54e584b16c48' + """ + assert_is_string(token_address, "token_address") + + return ( + "0x" + + abi_utils.simple_encode("ERC20Token(address)", token_address).hex() + ) + + +def decode_erc20_asset_data(asset_data: str) -> ERC20AssetData: + # docstring considered all one line by pylint: disable=line-too-long + """Decode an ERC20 assetData hex string. + + :param asset_data: String produced by prior call to encode_erc20_asset_data() + + >>> decode_erc20_asset_data("0xf47261b00000000000000000000000001dc4c1cefef38a777b15aa20260a54e584b16c48") + {'asset_proxy_id': '0xf47261b0', 'token_address': '0x1dc4c1cefef38a777b15aa20260a54e584b16c48'} + """ # noqa: E501 (line too long) + assert_is_string(asset_data, "asset_data") + + if len(asset_data) < ERC20_ASSET_DATA_BYTE_LENGTH: + raise ValueError( + "Could not decode ERC20 Proxy Data. Expected length of encoded" + + f" data to be at least {str(ERC20_ASSET_DATA_BYTE_LENGTH)}." + + f" Got {str(len(asset_data))}." + ) + + asset_proxy_id: str = asset_data[0:10] + if asset_proxy_id != abi_utils.method_id("ERC20Token", ["address"]): + raise ValueError( + "Could not decode ERC20 Proxy Data. Expected Asset Proxy Id to be" + + f" ERC20 ({abi_utils.method_id('ERC20Token', ['address'])})" + + f" but got {asset_proxy_id}." + ) + + # workaround for https://github.com/PyCQA/pylint/issues/1498 + # pylint: disable=unsubscriptable-object + token_address = eth_abi.decode_abi( + ["address"], bytes.fromhex(asset_data[SELECTOR_LENGTH:]) + )[0] + + return {"asset_proxy_id": asset_proxy_id, "token_address": token_address} diff --git a/python-packages/order_utils/src/zero_ex/order_utils/signature_utils.py b/python-packages/order_utils/src/zero_ex/order_utils/signature_utils.py deleted file mode 100644 index 7f4697106..000000000 --- a/python-packages/order_utils/src/zero_ex/order_utils/signature_utils.py +++ /dev/null @@ -1,13 +0,0 @@ -"""Signature utilities.""" - - -def ec_sign_order_hash(): - """Signs an orderHash. - - Returns its elliptic curve signature and signature type. This method - currently supports TestRPC, Geth, and Parity above and below v1.6.6. - - >>> ec_sign_order_hash() - 'stub return value' - """ - return "stub return value" diff --git a/python-packages/order_utils/stubs/distutils/__init__.pyi b/python-packages/order_utils/stubs/distutils/__init__.pyi new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/python-packages/order_utils/stubs/distutils/__init__.pyi diff --git a/python-packages/order_utils/stubs/distutils/command/__init__.pyi b/python-packages/order_utils/stubs/distutils/command/__init__.pyi new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/python-packages/order_utils/stubs/distutils/command/__init__.pyi diff --git a/python-packages/order_utils/stubs/distutils/command/clean.pyi b/python-packages/order_utils/stubs/distutils/command/clean.pyi new file mode 100644 index 000000000..46a42ddb1 --- /dev/null +++ b/python-packages/order_utils/stubs/distutils/command/clean.pyi @@ -0,0 +1,7 @@ +from distutils.core import Command + +class clean(Command): + def initialize_options(self: clean) -> None: ... + def finalize_options(self: clean) -> None: ... + def run(self: clean) -> None: ... + ... diff --git a/python-packages/order_utils/stubs/pytest/__init__.pyi b/python-packages/order_utils/stubs/pytest/__init__.pyi new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/python-packages/order_utils/stubs/pytest/__init__.pyi diff --git a/python-packages/order_utils/stubs/pytest/raises.pyi b/python-packages/order_utils/stubs/pytest/raises.pyi new file mode 100644 index 000000000..2e3b29f3d --- /dev/null +++ b/python-packages/order_utils/stubs/pytest/raises.pyi @@ -0,0 +1 @@ +def raises(exception: Exception) -> ExceptionInfo: ... diff --git a/python-packages/order_utils/stubs/setuptools/__init__.pyi b/python-packages/order_utils/stubs/setuptools/__init__.pyi new file mode 100644 index 000000000..baa349d70 --- /dev/null +++ b/python-packages/order_utils/stubs/setuptools/__init__.pyi @@ -0,0 +1,6 @@ +from distutils.dist import Distribution +from typing import Any + +def setup(**attrs: Any) -> Distribution: ... + +class Command: ... diff --git a/python-packages/order_utils/stubs/setuptools/command/__init__.pyi b/python-packages/order_utils/stubs/setuptools/command/__init__.pyi new file mode 100644 index 000000000..e69de29bb --- /dev/null +++ b/python-packages/order_utils/stubs/setuptools/command/__init__.pyi diff --git a/python-packages/order_utils/stubs/setuptools/command/test.pyi b/python-packages/order_utils/stubs/setuptools/command/test.pyi new file mode 100644 index 000000000..c5ec770ad --- /dev/null +++ b/python-packages/order_utils/stubs/setuptools/command/test.pyi @@ -0,0 +1,3 @@ +from setuptools import Command + +class test(Command): ... diff --git a/python-packages/order_utils/stubs/web3/__init__.pyi b/python-packages/order_utils/stubs/web3/__init__.pyi new file mode 100644 index 000000000..c6f357009 --- /dev/null +++ b/python-packages/order_utils/stubs/web3/__init__.pyi @@ -0,0 +1,10 @@ +from typing import Optional, Union + +class Web3: + @staticmethod + def sha3( + primitive: Optional[Union[bytes, int, None]] = None, + text: Optional[str] = None, + hexstr: Optional[str] = None + ) -> bytes: ... + ... diff --git a/python-packages/order_utils/test/test_abi_utils.py b/python-packages/order_utils/test/test_abi_utils.py new file mode 100644 index 000000000..49a2a4f20 --- /dev/null +++ b/python-packages/order_utils/test/test_abi_utils.py @@ -0,0 +1,53 @@ +"""Tests of 0x.abi_utils.""" + +import pytest + +from zero_ex.dev_utils.abi_utils import ( + elementary_name, + event_id, + method_id, + parse_signature, + simple_encode, +) + + +def test_parse_signature_type_error(): + """Test that passing in wrong types raises TypeError.""" + with pytest.raises(TypeError): + parse_signature(123) + + +def test_parse_signature_bad_input(): + """Test that passing a non-signature string raises a ValueError.""" + with pytest.raises(ValueError): + parse_signature("a string that's not even close to a signature") + + +def test_elementary_name_type_error(): + """Test that passing in wrong types raises TypeError.""" + with pytest.raises(TypeError): + elementary_name(123) + + +def test_event_id_type_error(): + """Test that passing in wrong types raises TypeError.""" + with pytest.raises(TypeError): + event_id(123, []) + + with pytest.raises(TypeError): + event_id("valid string", 123) + + +def test_method_id_type_error(): + """Test that passing in wrong types raises TypeError.""" + with pytest.raises(TypeError): + method_id(123, []) + + with pytest.raises(TypeError): + method_id("ERC20Token", 123) + + +def test_simple_encode_type_error(): + """Test that passing in wrong types raises TypeError.""" + with pytest.raises(TypeError): + simple_encode(123) diff --git a/python-packages/order_utils/test/test_asset_data_utils.py b/python-packages/order_utils/test/test_asset_data_utils.py new file mode 100644 index 000000000..eeada5873 --- /dev/null +++ b/python-packages/order_utils/test/test_asset_data_utils.py @@ -0,0 +1,35 @@ +"""Tests of 0x.order_utils.asset_data_utils.""" + +import pytest + +from zero_ex.order_utils.asset_data_utils import ( + encode_erc20_asset_data, + decode_erc20_asset_data, + ERC20_ASSET_DATA_BYTE_LENGTH, +) + + +def test_encode_erc20_asset_data_type_error(): + """Test that passing in a non-string raises a TypeError.""" + with pytest.raises(TypeError): + encode_erc20_asset_data(123) + + +def test_decode_erc20_asset_data_type_error(): + """Test that passing in a non-string raises a TypeError.""" + with pytest.raises(TypeError): + decode_erc20_asset_data(123) + + +def test_decode_erc20_asset_data_too_short(): + """Test that passing an insufficiently long string raises a ValueError.""" + with pytest.raises(ValueError): + decode_erc20_asset_data(" " * (ERC20_ASSET_DATA_BYTE_LENGTH - 1)) + + +def test_decode_erc20_asset_data_invalid_proxy_id(): + """Test that passing data with an invalid proxy ID raises a ValueError.""" + with pytest.raises(ValueError): + decode_erc20_asset_data( + "0xffffffff" + (" " * ERC20_ASSET_DATA_BYTE_LENGTH) + ) diff --git a/python-packages/order_utils/test/test_doctest.py b/python-packages/order_utils/test/test_doctest.py index a0e61f84a..ba5da5418 100644 --- a/python-packages/order_utils/test/test_doctest.py +++ b/python-packages/order_utils/test/test_doctest.py @@ -1,10 +1,24 @@ """Exercise doctests for order_utils module.""" from doctest import testmod -from zero_ex.order_utils import signature_utils +from zero_ex.dev_utils import abi_utils, type_assertions +from zero_ex.order_utils import asset_data_utils -def test_doctest(): - """Invoke doctest on the module.""" - (failure_count, _) = testmod(signature_utils) + +def test_doctest_asset_data_utils(): + """Invoke doctest on the asset_data_utils module.""" + (failure_count, _) = testmod(asset_data_utils) + assert failure_count == 0 + + +def test_doctest_abi_utils(): + """Invoke doctest on the abi_utils module.""" + (failure_count, _) = testmod(abi_utils) + assert failure_count == 0 + + +def test_doctest_type_assertions(): + """Invoke doctest on the type_assertions module.""" + (failure_count, _) = testmod(type_assertions) assert failure_count == 0 diff --git a/python-packages/order_utils/test/test_signature_utils.py b/python-packages/order_utils/test/test_signature_utils.py deleted file mode 100644 index 7e830f9f8..000000000 --- a/python-packages/order_utils/test/test_signature_utils.py +++ /dev/null @@ -1,8 +0,0 @@ -"""Tests of 0x.order_utils.signature_utils.*.""" - -from zero_ex.order_utils.signature_utils import ec_sign_order_hash - - -def test_ec_sign_order_hash(): - """Test the signing of order hashes.""" - assert ec_sign_order_hash() == "stub return value" |