Replace eth_abi.abi.process_type with eth_abi.grammar.parse #977

dylanjw · 2018-07-30T18:42:50Z

What was wrong?

How was it fixed?

Replace process_type with grammar.parse

Cute Animal Picture

carver · 2018-07-30T19:29:51Z

Maybe better to replace with a local process_type utility function. Then it can detect which version of eth-abi is in use and use the appropriate option.

dylanjw · 2018-07-30T21:06:56Z

web3/utils/abi.py

+        collapse_type,
+    )
+
+


@carver Something like this?

Yup, although I think it should be >= v1 in the if test. It highlights the breaking change at the major version. (Also, 0.5.0 is already released, and we could theoretically release an 0.6.0 on the v0/stable branch).

carver

Let's add a core test in tox with eth-abi constrained <1 to catch these things.

carver · 2018-07-30T21:33:43Z

web3/utils/abi.py

+        collapse_type,
+    )
+
+


Yup, although I think it should be >= v1 in the if test. It highlights the breaking change at the major version. (Also, 0.5.0 is already released, and we could theoretically release an 0.6.0 on the v0/stable branch).

carver · 2018-07-30T21:34:20Z

web3/utils/abi.py

-from eth_abi.abi import (
-    collapse_type,
-    process_type,
+from eth_abi.grammar import (


This would have to move into the if test.

carver · 2018-07-31T00:12:40Z

.circleci/config.yml

@@ -356,7 +385,8 @@ workflows:
      - py35-integration-parity-ws
      - py35-integration-ethtester-pyethereum
      - py35-integration-ethtester-pyevm
-      - py36-core
+      - py36-core-eth_abi0
+      - py36-core-eth_abi1


Can we start testing against eth-abi v2 yet, or is the alpha broken?

Yep, I started to add it, but then needed to take a minute to figure out how to get pre-releases installed. Im still hitting a snag getting it working correctly for v0 and v1. The eth_abi0 env is still installing v1.1.1. Will have a look tomorrow.

dylanjw · 2018-07-31T18:15:30Z

@carver eth_abi < v1 is failing due to incompatibility with the other dependencies. Because the latest web3 is incompatible with eth_abi < 1, should I remove the conditional import of the collapse_type and process_type functions?

carver · 2018-07-31T23:49:01Z

Because the latest web3 is incompatible with eth_abi < 1, should I remove the conditional import of the collapse_type and process_type functions?

Actually, I can't see why we'd test <1 at all. The web3.py requirement is >1.1.1: https://github.com/ethereum/web3.py/blob/master/setup.py#L22

But we still need to conditionally import, because they won't be there anymore by the time v2 leaves alpha.

dylanjw · 2018-08-02T20:59:06Z

@carver Because eth-abi 2.0.0a1 has some failing tests I will put it in a separate pull request, where the failing tests can be sorted out.

EDIT: And I suppose that PR will need to be merged first, since we need to see that the import conditional isn't broken w/ eth-abi 2.

dylanjw · 2018-08-02T21:17:08Z

Will need to rebase once #974 is merged

carver · 2018-08-02T21:46:38Z

Both core tests are running against eth ABI 1.1.1 - it probably makes sense to release the next eth abi v2 alpha with process_type removed, and then pin one of these tests to >= that v2 version.

dylanjw · 2018-08-22T21:55:33Z

Got a error with the caching middleware:

=================================== FAILURES ===================================
_____________ test_latest_block_based_cache_middleware_busts_cache _____________

w3 = <web3.main.Web3 object at 0x7fab1353eda0>
mocker = <pytest_mock.MockFixture object at 0x7fab1353e160>

    def test_latest_block_based_cache_middleware_busts_cache(w3, mocker):
        result = w3.manager.request_blocking('fake_endpoint', [])
    
        assert w3.manager.request_blocking('fake_endpoint', []) == result
        w3.testing.mine()
    
        # should still be cached for at least 1 second.  This also verifies that
        # the middleware caches the latest block based on the block time.
>       assert w3.manager.request_blocking('fake_endpoint', []) == result
E       AssertionError: assert '2adffae3-7f2...-5411065fd33d' == 'e2b2f6df-7c20...-c35710a47abe'
E         - 2adffae3-7f2a-4e24-b04c-5411065fd33d
E         + e2b2f6df-7c20-44d7-a12b-c35710a47abe

mocker     = <pytest_mock.MockFixture object at 0x7fab1353e160>
result     = 'e2b2f6df-7c20-44d7-a12b-c35710a47abe'
w3         = <web3.main.Web3 object at 0x7fab1353eda0>

tests/core/middleware/test_latest_block_based_cache_middleware.py:189: AssertionError

carver · 2018-08-23T21:06:15Z

Got a error with the caching middleware:

Was this resolved and then rebased? I don't see any failing CI run.

dylanjw · 2018-08-23T23:20:29Z

I restarted the tests, and they passed. Maybe the test execution had a 1 second delay causing the cache middleware to refresh.

dylanjw · 2018-08-23T23:22:17Z

I would like to hold off on merging this until an eth-abi alpha is released with process_type removed.

carver · 2018-09-18T17:32:48Z

web3/utils/abi.py

+            type_str_repr = '{} (normalized to {})'.format(
+                type_str_repr,
+                repr(normalized_type_str),
+            )


Looks like all this type_str work can go inside the TupleType check.

eth-abi v2 drops support for types formatted as tuples. e.g.: eth_abi.is_encodable(('address', '', []), '0x1111111111111111111111111111111111111111')

dylanjw force-pushed the remove_process_type branch from 9638b8c to 6898014 Compare July 30, 2018 21:05

dylanjw commented Jul 30, 2018

View reviewed changes

carver requested changes Jul 30, 2018

View reviewed changes

dylanjw force-pushed the remove_process_type branch 3 times, most recently from 45277e1 to 67de2c7 Compare July 30, 2018 22:20

carver approved these changes Jul 31, 2018

View reviewed changes

dylanjw force-pushed the remove_process_type branch from e4a711e to 10a9dd8 Compare July 31, 2018 18:15

dylanjw force-pushed the remove_process_type branch 4 times, most recently from f509617 to 2b6d7de Compare August 2, 2018 20:47

dylanjw force-pushed the remove_process_type branch from 2b6d7de to 644a0bd Compare August 2, 2018 21:01

dylanjw force-pushed the remove_process_type branch 3 times, most recently from 03d6d5a to 14313ae Compare August 22, 2018 21:17

dylanjw force-pushed the remove_process_type branch from 14313ae to 1faa393 Compare August 22, 2018 23:00

carver added the No Merge label Aug 24, 2018

dylanjw added 2 commits September 18, 2018 09:57

Replace eth_abi.abi.process_type with eth_abi.grammar.parse

4ae4ba3

Add eth-abi 0,1,2 to tox ini

43ce58f

dylanjw added 3 commits September 18, 2018 09:57

Add eth_abi version tox envs to circleci config

7635f39

Use tox commands to install eth-abi versions

b76f52f

Test against eth-abi v2

9016832

dylanjw force-pushed the remove_process_type branch from 1faa393 to 9016832 Compare September 18, 2018 16:57

dylanjw removed the No Merge label Sep 18, 2018

carver approved these changes Sep 18, 2018

View reviewed changes

dylanjw force-pushed the remove_process_type branch from 94d35b5 to 03d16af Compare September 18, 2018 18:59

Test against eth-abi 2.0.0b1

0a5ffb8

dylanjw force-pushed the remove_process_type branch from 03d16af to 0a5ffb8 Compare September 18, 2018 19:03

Support eth_abi v2 is_encodable

be9c4c5

eth-abi v2 drops support for types formatted as tuples. e.g.: eth_abi.is_encodable(('address', '', []), '0x1111111111111111111111111111111111111111')

dylanjw force-pushed the remove_process_type branch from 3547ed0 to be9c4c5 Compare September 18, 2018 22:01

dylanjw merged commit 154f43e into ethereum:master Sep 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace eth_abi.abi.process_type with eth_abi.grammar.parse #977

Replace eth_abi.abi.process_type with eth_abi.grammar.parse #977

dylanjw commented Jul 30, 2018

carver commented Jul 30, 2018

dylanjw Jul 30, 2018

carver Jul 30, 2018

carver left a comment

carver Jul 30, 2018

carver Jul 30, 2018

carver Jul 31, 2018

dylanjw Jul 31, 2018

dylanjw commented Jul 31, 2018

carver commented Jul 31, 2018

dylanjw commented Aug 2, 2018 •

edited

Loading

dylanjw commented Aug 2, 2018

carver commented Aug 2, 2018

dylanjw commented Aug 22, 2018

carver commented Aug 23, 2018

dylanjw commented Aug 23, 2018

dylanjw commented Aug 23, 2018

carver Sep 18, 2018

Replace eth_abi.abi.process_type with eth_abi.grammar.parse #977

Replace eth_abi.abi.process_type with eth_abi.grammar.parse #977

Conversation

dylanjw commented Jul 30, 2018

What was wrong?

How was it fixed?

Cute Animal Picture

carver commented Jul 30, 2018

dylanjw Jul 30, 2018

Choose a reason for hiding this comment

carver Jul 30, 2018

Choose a reason for hiding this comment

carver left a comment

Choose a reason for hiding this comment

carver Jul 30, 2018

Choose a reason for hiding this comment

carver Jul 30, 2018

Choose a reason for hiding this comment

carver Jul 31, 2018

Choose a reason for hiding this comment

dylanjw Jul 31, 2018

Choose a reason for hiding this comment

dylanjw commented Jul 31, 2018

carver commented Jul 31, 2018

dylanjw commented Aug 2, 2018 • edited Loading

dylanjw commented Aug 2, 2018

carver commented Aug 2, 2018

dylanjw commented Aug 22, 2018

carver commented Aug 23, 2018

dylanjw commented Aug 23, 2018

dylanjw commented Aug 23, 2018

carver Sep 18, 2018

Choose a reason for hiding this comment

dylanjw commented Aug 2, 2018 •

edited

Loading