more pr review

tests/core/utilities/test_method_formatters.py

@@ -2,7 +2,7 @@
 
 from web3._utils.method_formatters import (
     get_error_formatters,
-    get_revert_reason,
+    raise_if_revert_reason,
 )
 from web3._utils.rpc_abi import (
     RPC,
@@ -50,10 +50,23 @@
 })
 
 
-def test_get_revert_reason() -> None:
-    assert get_revert_reason(REVERT_WITH_MSG) == 'execution reverted: not allowed to monitor'
-    assert get_revert_reason(REVERT_WITHOUT_MSG) == ''
-    assert get_revert_reason(OTHER_ERROR) is None
+@pytest.mark.parametrize(
+    "response,expected",
+    (
+        (REVERT_WITH_MSG, 'execution reverted: not allowed to monitor'),
+        (REVERT_WITHOUT_MSG, ''),
+    ),
+    ids=[
+        'test-get-revert-reason-with-msg',
+        'test-get-revert-reason-without-msg',
+    ])
+def test_get_revert_reason(response, expected) -> None:
+    with pytest.raises(SolidityError):
+        assert raise_if_revert_reason(response) == expected
+
+
+def test_get_revert_reason_other_error() -> None:
+    assert raise_if_revert_reason(OTHER_ERROR) is None
 
 
 def test_get_error_formatters() -> None:

tests/integration/generate_fixtures/common.py

@@ -36,8 +36,6 @@
         "homesteadBlock": 0,
         "byzantiumBlock": 0,
         "constantinopleBlock": 0,
-        #  "petersburgBlock": 0,
-        #  "istanbulBlock": 0,
         "eip150Block": 0,
         "eip155Block": 0,
         "eip158Block": 0,
@@ -53,9 +51,6 @@
         "0000000000000000000000000000000000000004": {"balance": "1"},
         "0000000000000000000000000000000000000005": {"balance": "1"},
         "0000000000000000000000000000000000000006": {"balance": "1"},
-        #  "0000000000000000000000000000000000000007": {"balance": "1"},
-        #  "0000000000000000000000000000000000000008": {"balance": "1"},
-        #  "0000000000000000000000000000000000000009": {"balance": "1"},
     },
     "timestamp": "0x00",
     "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000",

tests/integration/generate_fixtures/parity.py

@@ -29,7 +29,7 @@
                 "minimumDifficulty": "0x020000",
                 "difficultyBoundDivisor": "0x0800",
                 "durationLimit": "0x0d",
-                "blockReward": "0x1bc16d674ec80000",  # constantinople
+                "blockReward": "0x1bc16d674ec80000",
                 "difficultyBombDelays": {
                     "0x0": "0x1e8480",
                 },
@@ -58,9 +58,9 @@
         "eip1014Transition": 0,
         "eip1052Transition": 0,
         "eip1283Transition": 0,
-        #  Petersburg
+        #  TODO: Petersburg
         #  "eip1283DisableTransition": 0,
-        #  Istanbul
+        #  TODO: Istanbul
         #  "eip1283ReenableTransition": 0,
         #  "eip1344Transition": 0,
         #  "eip1884Transition": 0,
@@ -132,9 +132,6 @@
                 "pricing": {"linear": {"base": 500, "word": 0}}
             }
         },
-        #  "0000000000000000000000000000000000000007": {},
-        #  "0000000000000000000000000000000000000008": {},
-        #  "0000000000000000000000000000000000000009": {},
     }
 }
 

web3/_utils/method_formatters.py

@@ -487,50 +487,40 @@ def apply_list_to_array_formatter(formatter: Any) -> Callable[..., Any]:
 ABI_REQUEST_FORMATTERS = abi_request_formatters(STANDARD_NORMALIZERS, RPC_ABIS)
 
 
-def get_revert_reason(response: RPCResponse) -> str:
+def raise_if_revert_reason(response: RPCResponse) -> None:
     """
-    Parse revert reason from response, return None if no revert happened.
-
-    If a revert happened, but no message has been given, return an empty string.
-
     Reverts contain a `data` attribute with the following layout:
         "Reverted "
         Function selector for Error(string): 08c379a (4 bytes)
         Data offset: 32 (32 bytes)
         String length (32 bytes)
-        Reason strong (padded, use string length from above to get meaningful part)
+        Reason string (padded, use string length from above to get meaningful part)
 
     See also https://solidity.readthedocs.io/en/v0.6.3/control-structures.html#revert
     """
     if not isinstance(response['error'], dict):
-        return None
+        raise ValueError('Error expected to be a dict')
 
     # Parity/OpenEthereum case:
     data = response['error'].get('data', '')
     if data.startswith('Reverted '):
         # "Reverted", function selector and offset are always the same for revert errors
         prefix = 'Reverted 0x08c379a00000000000000000000000000000000000000000000000000000000000000020'  # noqa: 501
         if not data.startswith(prefix):
-            return ''
+            raise SolidityError()
 
         reason_length = int(data[len(prefix):len(prefix) + 64], 16)
         reason = data[len(prefix) + 64:len(prefix) + 64 + reason_length * 2]
-        return f'execution reverted: {bytes.fromhex(reason).decode("utf8")}'
+        raise SolidityError(f'execution reverted: {bytes.fromhex(reason).decode("utf8")}')
 
     # Geth case:
     if 'message' in response['error'] and response['error'].get('code', '') == 3:
-        return response['error']['message']
-
-    return None
+        raise SolidityError(response['error']['message'])
 
 
 def raise_solidity_error_on_revert(response: RPCResponse) -> RPCResponse:
-    revert_reason = get_revert_reason(response)
-    if revert_reason is None:
-        return response
-    if revert_reason == '':
-        raise SolidityError()
-    raise SolidityError(revert_reason)
+    raise_if_revert_reason(response)
+    return response
 
 
 ERROR_FORMATTERS: Dict[RPCEndpoint, Callable[..., Any]] = {

web3/_utils/module_testing/eth_module.py

@@ -748,7 +748,7 @@ def test_eth_call_revert_with_msg(
         revert_contract: "Contract",
         unlocked_account: ChecksumAddress,
     ) -> None:
-        with pytest.raises(SolidityError, match='execution reverted'):
+        with pytest.raises(SolidityError, match='execution reverted: Function has been reverted'):
             txn_params = revert_contract._prepare_transaction(
                 fn_name="revertWithMessage",
                 transaction={
@@ -764,7 +764,7 @@ def test_eth_estimateGas_revert_with_msg(
         revert_contract: "Contract",
         unlocked_account: ChecksumAddress,
     ) -> None:
-        with pytest.raises(SolidityError, match='execution reverted'):
+        with pytest.raises(SolidityError, match='execution reverted: Function has been reverted'):
             txn_params = revert_contract._prepare_transaction(
                 fn_name="revertWithMessage",
                 transaction={

web3/providers/eth_tester/main.py

@@ -6,6 +6,10 @@
     Optional,
 )
 
+from eth_abi import (
+    decode_single,
+)
+
 from web3._utils.compat import (
     Literal,
 )
@@ -102,8 +106,11 @@ def make_request(self, method: RPCEndpoint, params: Any) -> RPCResponse:
                 "error": "RPC Endpoint has not been implemented: {0}".format(method),
             })
         except TransactionFailed as e:
-            # Mirroring Geth: 'execution reverted: <revert message>'
-            raise SolidityError(f'execution reverted: {e.args[0]}')
+            if type(e.args[0]) == str:
+                reason = e.args[0]
+            else:
+                reason = decode_single('(string)', e.args[0].args[0][4:])[0]
+            raise SolidityError(f'execution reverted: {reason}')
         else:
             return {
                 'result': response,