Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

slither-read-storage native POA support #1843

Merged
merged 26 commits into from
Jun 2, 2023

Conversation

webthethird
Copy link
Contributor

@webthethird webthethird commented Apr 17, 2023

Adds a new RpcInfo class that holds self.rpc, self.web3 and self.block, which are removed from SlitherReadStorage. SlitherReadStorage gets new self._rpc_info: Optional[RpcInfo] which is required for using Web3.

In RpcInfo, automatically detect if the network is POA when setting self._block. web3.eth.get_block raises an ExtraDataLengthError when using an RPC for a POA network, so if we catch one of those after trying to set self._block, we know it is a POA network and therefore inject the appropriate middleware.

In SlitherReadStorage, self.rpc_info: Optional[RpcInfo]
replaces self.rpc, self._block, self._web3
@webthethird webthethird requested a review from 0xalpharush April 28, 2023 13:30
["latest", "earliest", "pending", "safe", "finalized"]
@webthethird webthethird requested a review from 0xalpharush May 4, 2023 17:18
@webthethird
Copy link
Contributor Author

I'm running into an error in web3.eth.get_block when passing in an enum element, even though its value is a string.

    def test_read_storage(web3, ganache) -> None:
        assert web3.is_connected()
        bin_path = Path(TEST_DATA_DIR, "StorageLayout.bin").as_posix()
        abi_path = Path(TEST_DATA_DIR, "StorageLayout.abi").as_posix()
        bytecode = get_source_file(bin_path)
        abi = get_source_file(abi_path)
        contract = deploy_contract(web3, ganache, bytecode, abi)
        contract.functions.store().transact({"from": ganache.eth_address})
        address = contract.address
    
        sl = Slither(Path(TEST_DATA_DIR, "storage_layout-0.8.10.sol").as_posix())
        contracts = sl.contracts
    
>       rpc_info: RpcInfo = RpcInfo(ganache.provider, BlockTag.LATEST)

test_read_storage.py:106: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../slither/tools/read_storage/read_storage.py:73: in __init__
    self._block: int = self.web3.eth.get_block(block)["number"]
../../../../../.local/lib/python3.10/site-packages/web3/eth/eth.py:389: in get_block
    return self._get_block(block_identifier, full_transactions)
../../../../../.local/lib/python3.10/site-packages/web3/module.py:58: in caller
    (method_str, params), response_formatters = method.process_params(
../../../../../.local/lib/python3.10/site-packages/web3/method.py:214: in process_params
    self.json_rpc_method = self.method_choice_depends_on_args(value=params[0])
cytoolz/functoolz.pyx:253: in cytoolz.functoolz.curry.__call__
    ???
cytoolz/functoolz.pyx:249: in cytoolz.functoolz.curry.__call__
    ???
../../../../../.local/lib/python3.10/site-packages/web3/_utils/blocks.py:62: in select_method_for_block_identifier
    if is_predefined_block_number(value):
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

value = <BlockTag.LATEST: 'latest'>

    def is_predefined_block_number(value: Any) -> bool:
        if is_text(value):
            value_text = value
        elif is_bytes(value):
            # `value` could either be random bytes or the utf-8 encoding of
            # one of the words in: {"latest", "pending", "earliest", "safe", "finalized"}
            # We cannot decode the bytes as utf8, because random bytes likely won't be
            # valid. So we speculatively decode as 'latin-1', which cannot fail.
            value_text = value.decode("latin-1")
        elif is_integer(value):
            return False
        else:
>           raise TypeError(f"unrecognized block reference: {value!r}")
E           TypeError: unrecognized block reference: <BlockTag.LATEST: 'latest'>

I'm thinking it might be better to use the pre-existing web3.types.BlockParams instead of the new BlockTag enum:
BlockParams = Literal["latest", "earliest", "pending", "safe", "finalized"]

@webthethird
Copy link
Contributor Author

test_read_storage is passing on my machine, but for some reason the assert web3.is_connected() is failing here

=========================== short test summary info ===========================
FAILED tests/tools/read-storage/test_read_storage.py::test_read_storage - assert False
 +  where False = <bound method Web3.is_connected of <web3.main.Web3 object at 0x0000020F37819AF0>>()
 +    where <bound method Web3.is_connected of <web3.main.Web3 object at 0x0000020F37819AF0>> = <web3.main.Web3 object at 0x0000020F37819AF0>.is_connected

@webthethird webthethird requested a review from 0xalpharush May 4, 2023 18:28
@webthethird
Copy link
Contributor Author

The more I look at it, the more it seems like introducing the BlockTag enum just over-complicates the code.
Looking at web3._utils, this is similar to what we were doing before introducing that enum:

def parse_block_identifier(
    w3: "Web3", block_identifier: BlockIdentifier
) -> BlockIdentifier:
    if block_identifier is None:
        return w3.eth.default_block
    if isinstance(block_identifier, int):
        return parse_block_identifier_int(w3, block_identifier)
    elif block_identifier in ["latest", "earliest", "pending", "safe", "finalized"]:
        return block_identifier
    elif isinstance(block_identifier, bytes) or is_hex_encoded_block_hash(
        block_identifier
    ):
        return w3.eth.get_block(block_identifier)["number"]
    else:
        raise BlockNumberOutofRange

I am leaning toward going back to this and removing BlockTag:

class RpcInfo:
    def __init__(self, rpc_url: str, block: BlockIdentifier = "latest") -> None:
        assert isinstance(block, int) or block in ["latest", "earliest", "pending", "safe", "finalized"]
        ...

@webthethird
Copy link
Contributor Author

I am leaning toward going back to this and removing BlockTag

@0xalpharush I went ahead and changed it back

Better, cleaner python

Co-authored-by: alpharush <[email protected]>
@webthethird webthethird requested a review from 0xalpharush May 12, 2023 13:14
@0xalpharush
Copy link
Contributor

I think we can squash and merge this

@0xalpharush 0xalpharush merged commit 00461aa into crytic:dev Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants