magic-admin-python

magic-admin-python copied to clipboard

✅ Prerequisites

• [x] Did you perform a cursory search of open issues? Is this bug already reported elsewhere?
• [x] Are you running the latest SDK version?
• [x] Are you reporting to the correct repository (magic-admin-python)?

🐛 Description

Currently magic-admin depends on web3 < 6, which has issues with Python 3.11 (because of getargspec from inspect). It seems this issue was fixed with web3 6.10. So in practice we can't use magic with Python 3.11 and are restricted to 3.10. This means that the docs are imprecise (only works 3.7 - 3.10) and with 3.12 being release next week and 3.11 being stable, it would make sense to handle the different web3 versions.

Quickly looking through the code, it seems the only place this is relevant is the recoverHash function from web3. It seems this was deprecated a while ago https://github.com/ethereum/eth-account/pull/195/files and recover_message is the method that replaced it.

This method looks fairly straight forward: https://github.com/ethereum/eth-account/blob/8249380b085e6e09dd895c4d6239612eee4a781c/eth_account/account.py#L462-L463

Where: https://github.com/ethereum/eth-account/blob/8249380b085e6e09dd895c4d6239612eee4a781c/eth_account/messages.py#L73-L82

And finally https://github.com/ethereum/eth-account/blob/45911155c71770217917ceabd3589c5710d2b67d/eth_utils/crypto.py#L8-L11

Where keccak seems to be SHA3, and I think hashlib has this native, so I wonder if the dependency would even be necessary (given that web3 and eth_account have had some API stability issues in the past).

That all being said, I think it could probably make sense to isolate the web3 interaction to a new file something like:

class Web3:
    @classmethod
    def recover_address(cls, claim, proof):
        pass

And and that a class attribute to ResourceComponent. That would isolate the interaction with web3 like defunct_hash_message and recover_message and others, also a lot easier to mock (vs mocking the whole library now).

The last thing I can think of, would be to add a conftest with some fixtures with autouse=True so to avoid the fairly complex fixture chaining in the TestTokenValidate like:

https://github.com/magiclabs/magic-admin-python/blob/44907faa49135c6da23c6fe79a62b4ad71325e09/tests/unit/resources/token_test.py#L171-L203

hey @ccrvlh. Or anyone that needs this still. I've been having the same issues so I have uploaded a revisited version of this package on pypi that supports python 3.9,3.10, 3.11. https://github.com/luisgj/magic-admin-python-v2.

You can install it with the name pip install cool-magic-admin

I've opened PR #92 with the fixes but the team hasn't been maintaining the package for a while 🤷🏻