Exchange & Proxy Integration Tests - ERC1155 #1673

hysz · 2019-03-06T00:53:41Z

Description

Integration tests for the ERC1155 proxy with the Mutli-Asset Proxy and Exchange.

Testing instructions

Types of changes

Checklist:

Prefix PR title with [WIP] if necessary.
Add tests to cover changes as needed.
Update documentation as needed.
Add new entries to the relevant CHANGELOG.jsons.

fabioberger

Didn't review the actual Solidity and someone else should make sure the tests actually test what they intend to test, but I reviewed the dev tooling changes and they look solid.

fabioberger · 2019-03-14T11:22:25Z

contracts/asset-proxy/contracts/src/ERC1155Proxy.sol

@@ -0,0 +1,268 @@
+/*


How this already been reviewed elsewhere? Are we duplicating Solidity source code within the monorepo?

Yup! Been reviewed in #1661. Just had to do a cheeky rebase.

fabioberger · 2019-03-14T11:43:55Z

packages/types/CHANGELOG.json

+                "pr": 1657
+            }
+        ]
+    },


These should be one CHANGELOG entry.

fabioberger · 2019-03-14T11:45:49Z

Eventually we'll also need to add ERC1155 support to OrderWatcher.

coveralls · 2019-03-15T04:53:54Z

Coverage increased (+0.08%) to 85.194% when pulling 98c6fa1 on feature/contracts/erc1155ProxyIntegrationTests-2 into 3fff3d9 on development.

abandeali1

Few small changes, then LGTM!

abandeali1 · 2019-03-18T19:55:15Z

contracts/asset-proxy/CHANGELOG.json

            }
        ]
    },
    {
        "timestamp": 1551479279,
-        "version": "1.0.9",
+        "version": "1.å.9",


abandeali1 · 2019-03-18T19:59:53Z

contracts/asset-proxy/test/proxies.ts

+                const tokensToTransfer = erc1155FungibleTokens.slice(0, 1);
+                const valuesToTransfer = [new BigNumber(25)];
+                const valueMultiplier = new BigNumber(23);
+                const receiverCallbackData = '0x0102030405';


This data is just being ignored since both parties are EOAs, correct? It might be good to explicitly test for when the callback data is null as well.

abandeali1 · 2019-03-18T20:03:03Z

contracts/exchange/test/core.ts

+        });
+        it('should allow an order exchanging erc1155 assets to be partially filled', async () => {
+            // NOTICE:
+            // As-per the eip1155 standard, there is no way to distinguish between a fungible or non-fungible erc1155 assets.


Shouldn't a partial transfer of a non-fungible still fail with TRANSFER_FAILED? Whatever the behavior is, we should add a test for it.

We chatted about this offline.

Partial fills on non-fungible assets should be prohibited by the maker by setting the amount to 1. The proxy cannot distinguish between fungible and non-fungible assets, so it cannot force partial fills to fail in this scenario. If the amount is not set to 1 and an asset is non-fungible then the behavior depends on the implementation of the ERC1155 contract.

…coding

…ULLL (0x)

probot-autolabeler bot added the contracts label Mar 6, 2019

hysz force-pushed the feature/contracts/erc1155Proxy-4 branch from c0eef21 to 49ccb98 Compare March 7, 2019 22:40

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from 403a815 to 06d591c Compare March 7, 2019 23:00

hysz requested review from fabioberger and LogvinovLeon as code owners March 7, 2019 23:48

probot-autolabeler bot added the @0x/order-utils label Mar 7, 2019

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from faba871 to b2f9b07 Compare March 8, 2019 01:09

hysz changed the title ~~[WIP] Exchange & Proxy Integration Tests - ERC1155~~ Exchange & Proxy Integration Tests - ERC1155 Mar 11, 2019

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from 4e72b93 to 5e62bde Compare March 12, 2019 01:24

hysz force-pushed the feature/contracts/erc1155Proxy-4 branch 2 times, most recently from 12eeed4 to 441b379 Compare March 12, 2019 21:38

fabioberger approved these changes Mar 14, 2019

View reviewed changes

hysz force-pushed the feature/contracts/erc1155Proxy-4 branch from 9b2197f to 32b0b0b Compare March 14, 2019 21:21

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from 5e62bde to fefd068 Compare March 14, 2019 23:23

hysz requested a review from abandeali1 as a code owner March 14, 2019 23:23

hysz force-pushed the feature/contracts/erc1155Proxy-4 branch from 32b0b0b to 5cc46b7 Compare March 15, 2019 03:48

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch 2 times, most recently from 4fa27ec to ce6ff86 Compare March 15, 2019 04:42

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch 3 times, most recently from 75cc676 to 44db7d3 Compare March 15, 2019 05:56

hysz force-pushed the feature/contracts/erc1155Proxy-4 branch from a33da79 to 22bc1fb Compare March 15, 2019 22:06

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from 24859f2 to ecd604f Compare March 16, 2019 01:46

hysz requested review from BMillman19, fragosti and steveklebanoff as code owners March 16, 2019 01:46

probot-autolabeler bot added the @0x/website label Mar 16, 2019

hysz changed the base branch from feature/contracts/erc1155Proxy-4 to development March 16, 2019 01:52

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from ecd604f to d540e4b Compare March 16, 2019 04:34

hysz assigned abandeali1 and dekz Mar 16, 2019

abandeali1 suggested changes Mar 18, 2019

View reviewed changes

hysz added 10 commits March 19, 2019 09:36

ERC1155 Asset Data tests + types

5063c17

Exchange Integration Tests

82b6dad

More integration tests for ERC1155 <> Multi Asset Proxy encoding / de…

31dbca7

…coding

ERC1155 integration tests with MultiAssetProxy

6957e6e

Ran prettier & linter

6992bff

rebased against development

c0260bc

Added ERC1155 artifacts to log decoder for exchange wrapper

29ebed9

updated changelogs

7024f29

Updated changelog and yarn.lock

30d0bde

Added 1explicit ERC1555Poxy testsfor cwhen receiver allback data is N…

98c6fa1

…ULLL (0x)

hysz force-pushed the feature/contracts/erc1155ProxyIntegrationTests-2 branch from 8dff829 to 98c6fa1 Compare March 19, 2019 16:38

abandeali1 approved these changes Mar 19, 2019

View reviewed changes

hysz merged commit e8be5a0 into development Mar 19, 2019

This was referenced Nov 17, 2020

[Snyk] Security upgrade yargs from 10.1.2 to 16.0.0 larrycameron80/0x-monorepo#106

Open

[Snyk] Security upgrade yargs from 10.1.2 to 16.0.0 larrycameron80/0x-monorepo#107

Open

[Snyk] Fix for 1 vulnerabilities larrycameron80/0x-monorepo#108

Open

This was referenced Aug 2, 2022

[Snyk] Fix for 5 vulnerabilities adamlaska/0x-monorepo#67

Closed

[Snyk] Fix for 5 vulnerabilities adamlaska/0x-monorepo#68

Closed

adamlaska mentioned this pull request Aug 3, 2022

[Snyk] Fix for 5 vulnerabilities adamlaska/0x-monorepo#70

Closed

snyk-bot mentioned this pull request Aug 20, 2022

[Snyk] Fix for 7 vulnerabilities adamlaska/0x-monorepo#117

Closed

snyk-bot mentioned this pull request Sep 24, 2022

[Snyk] Fix for 7 vulnerabilities adamlaska/0x-monorepo#132

Open

adamlaska mentioned this pull request Mar 10, 2023

[Snyk] Security upgrade typedoc from 0.15.8 to 0.21.10 adamlaska/0x-monorepo#204

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Exchange & Proxy Integration Tests - ERC1155 #1673

Exchange & Proxy Integration Tests - ERC1155 #1673

hysz commented Mar 6, 2019 •

edited

Loading

fabioberger left a comment

fabioberger Mar 14, 2019

hysz Mar 14, 2019

fabioberger Mar 14, 2019

fabioberger commented Mar 14, 2019

coveralls commented Mar 15, 2019 •

edited

Loading

abandeali1 left a comment

abandeali1 Mar 18, 2019

abandeali1 Mar 18, 2019

abandeali1 Mar 18, 2019

hysz Mar 19, 2019

Exchange & Proxy Integration Tests - ERC1155 #1673

Exchange & Proxy Integration Tests - ERC1155 #1673

Conversation

hysz commented Mar 6, 2019 • edited Loading

Description

Testing instructions

Types of changes

Checklist:

fabioberger left a comment

Choose a reason for hiding this comment

fabioberger Mar 14, 2019

Choose a reason for hiding this comment

hysz Mar 14, 2019

Choose a reason for hiding this comment

fabioberger Mar 14, 2019

Choose a reason for hiding this comment

fabioberger commented Mar 14, 2019

coveralls commented Mar 15, 2019 • edited Loading

abandeali1 left a comment

Choose a reason for hiding this comment

abandeali1 Mar 18, 2019

Choose a reason for hiding this comment

abandeali1 Mar 18, 2019

Choose a reason for hiding this comment

abandeali1 Mar 18, 2019

Choose a reason for hiding this comment

hysz Mar 19, 2019

Choose a reason for hiding this comment

hysz commented Mar 6, 2019 •

edited

Loading

coveralls commented Mar 15, 2019 •

edited

Loading