Skip to content

Commit

Permalink
RPC Error Revert Reason Cleanup (#3087)
Browse files Browse the repository at this point in the history 
* refactor error handling

* Cleanup, add comments, fix tests

* Newsfrag

* Fix tests

* Ditch jsonschema for more performant validation

* Fix tests

* Refactor exceptions

* drop noqa

* Updates per feedback, naming improvements

* Revert logic for error with valid id
  • Loading branch information
@reedsa
reedsa authored Sep 14, 2023
1 parent 1b72768 commit ffec086
Show file tree
Hide file tree
Showing 5 changed files with 289 additions and 93 deletions.
1 change: 1 addition & 0 deletions newsfragments/3053.feature.rst
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improved error messaging for exceptions from malformed JSON-RPC responses.
118 changes: 116 additions & 2 deletions tests/core/manager/test_response_formatters.py
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
import re

from eth_utils.toolz import (
identity,
Expand All @@ -25,9 +26,36 @@
"because the ancient block sync is still in progress.",
},
}
ERROR_RESPONSE_WITH_NONE_ID = {
"id": None,
"jsonrpc": "2.0",
"error": {
"code": -32000,
"message": "Requested block number is in a range that is not available yet, "
"because the ancient block sync is still in progress.",
},
}
ERROR_RESPONSE_WITH_VALID_ID = {
"id": 1,
"jsonrpc": "2.0",
"error": {
"code": -32000,
"message": "Requested block number is in a range that is not available yet, "
"because the ancient block sync is still in progress.",
},
}
NONE_RESPONSE = {"jsonrpc": "2.0", "id": 1, "result": None}
ZERO_X_RESPONSE = {"jsonrpc": "2.0", "id": 1, "result": "0x"}
INVALID_JSONRPC_RESP_FORMAT = {
"jsonrpc": "999",
"error": {
"code": -32000,
"message": "Requested block number is in a range that is not available yet, "
"because the ancient block sync is still in progress.",
},
}
UNEXPECTED_RESPONSE_FORMAT = {"jsonrpc": "2.0", "id": 1}
UNEXPECTED_RESPONSE_FORMAT_NONE_ID = {"jsonrpc": "2.0", "id": None}
ANOTHER_UNEXPECTED_RESP_FORMAT = {
"name": "LimitError",
"message": "You cannot query logs for more than 10000 blocks at once.",
Expand All @@ -41,6 +69,28 @@
"available",
},
}
INVALID_CODE_RESP_FORMAT = {
"jsonrpc": "2.0",
"error": {
"code": "-32601",
"message": "the method eth_getTransactionByHash does not exist/is not "
"available",
},
}
MISSING_CODE_RESP_FORMAT = {
"jsonrpc": "2.0",
"error": {
"message": "the method eth_getTransactionByHash does not exist/is not "
"available",
},
}
INVALID_MESSAGE_RESP_FORMAT = {
"jsonrpc": "2.0",
"error": {
"code": -32000,
"message": {},
},
}
ETH_TESTER_METHOD_NOT_FOUND_RESP_FORMAT = {
"error": "the method eth_getTransactionByHash does not exist/is not available",
}
Expand Down Expand Up @@ -77,6 +127,22 @@ def raise_contract_logic_error(response):
raise_block_not_found,
ValueError,
),
(
# Error response with no result formatters raises a ValueError
ERROR_RESPONSE_WITH_NONE_ID,
(),
identity,
raise_block_not_found,
ValueError,
),
(
# Error response with no result formatters raises a ValueError
ERROR_RESPONSE_WITH_VALID_ID,
(),
identity,
raise_block_not_found,
ValueError,
),
(
# None result raises error if there is a null_result_formatter
NONE_RESPONSE,
Expand Down Expand Up @@ -169,21 +235,69 @@ def test_formatted_response_raises_errors(
TransactionNotFound,
"Transaction with hash: '0x01' not found.",
),
(
INVALID_JSONRPC_RESP_FORMAT,
(),
identity,
identity,
BadResponseFormat,
f"The response was in an unexpected format and unable to be parsed. "
f'The "jsonrpc" field must be present with a value of "2.0". '
f"The raw response is: {INVALID_JSONRPC_RESP_FORMAT}",
),
(
UNEXPECTED_RESPONSE_FORMAT,
(),
identity,
identity,
BadResponseFormat,
f"The response was in an unexpected format and unable to be parsed. The raw response is: {UNEXPECTED_RESPONSE_FORMAT}", # noqa: E501
f"The response was in an unexpected format and unable to be parsed. "
f"The raw response is: {UNEXPECTED_RESPONSE_FORMAT}",
),
(
ANOTHER_UNEXPECTED_RESP_FORMAT,
(),
identity,
identity,
BadResponseFormat,
f"The response was in an unexpected format and unable to be parsed. The raw response is: {ANOTHER_UNEXPECTED_RESP_FORMAT}", # noqa: E501
f"The response was in an unexpected format and unable to be parsed. "
f"The raw response is: {ANOTHER_UNEXPECTED_RESP_FORMAT}",
),
(
INVALID_CODE_RESP_FORMAT,
(),
identity,
identity,
BadResponseFormat,
re.escape(
f"The response was in an unexpected format and unable to be parsed. "
f"error['code'] must be an integer. "
f"The raw response is: {INVALID_CODE_RESP_FORMAT}"
),
),
(
MISSING_CODE_RESP_FORMAT,
(),
identity,
identity,
BadResponseFormat,
re.escape(
f"The response was in an unexpected format and unable to be parsed. "
f"error['code'] must be an integer. "
f"The raw response is: {MISSING_CODE_RESP_FORMAT}"
),
),
(
INVALID_MESSAGE_RESP_FORMAT,
(),
identity,
identity,
BadResponseFormat,
re.escape(
f"The response was in an unexpected format and unable to be parsed. "
f"error['message'] must be a string. "
f"The raw response is: {INVALID_MESSAGE_RESP_FORMAT}"
),
),
],
)
Expand Down
158 changes: 90 additions & 68 deletions web3/_utils/contract_error_handling.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,96 +49,118 @@
"function type.",
}

MISSING_DATA = "no data"

def raise_contract_logic_error_on_revert(response: RPCResponse) -> RPCResponse:
"""
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 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
def _parse_error_with_reverted_prefix(data: str) -> str:
"""
if not isinstance(response["error"], dict):
raise ValueError("Error expected to be a dict")

data = response["error"].get("data", "")
message = response["error"].get("message", "")

message_present = message is not None and message != ""
Parse errors from the data string which begin with the "Reverted" prefix.
"Reverted", function selector and offset are always the same for revert errors
"""
prefix = f"Reverted {SOLIDITY_ERROR_FUNC_SELECTOR}"
data_offset = ("00" * 31) + "20" # 0x0000...0020 (32 bytes)
revert_pattern = prefix + data_offset
error = data

if data.startswith(revert_pattern):
# if common revert pattern
string_length = int(data[len(revert_pattern) : len(revert_pattern) + 64], 16)
error = data[
len(revert_pattern) + 64 : len(revert_pattern) + 64 + string_length * 2
]
elif data.startswith("Reverted 0x"):
# Special case for this form: 'Reverted 0x...'
error = data.split(" ")[1][2:]

try:
error = bytes.fromhex(error).decode("utf8")
except UnicodeDecodeError:
warnings.warn("Could not decode revert reason as UTF-8", RuntimeWarning)
raise ContractLogicError("execution reverted", data=data)

if data is None:
if message_present:
raise ContractLogicError(message)
elif not message_present:
raise ContractLogicError("execution reverted")
else:
raise Exception("Unreachable")
return error

# Ganache case:
if isinstance(data, dict) and message_present:
raise ContractLogicError(f"execution reverted: {message}", data=data)

# Parity/OpenEthereum case:
if data.startswith("Reverted "):
# "Reverted", function selector and offset are always the same for revert errors
prefix = f"Reverted {SOLIDITY_ERROR_FUNC_SELECTOR}"
data_offset = ("00" * 31) + "20" # 0x0000...0020 (32 bytes)
revert_pattern = prefix + data_offset

if data.startswith(revert_pattern):
# if common revert pattern
string_length = int(
data[len(revert_pattern) : len(revert_pattern) + 64], 16
)
reason_as_hex = data[
len(revert_pattern) + 64 : len(revert_pattern) + 64 + string_length * 2
]
elif data.startswith("Reverted 0x"):
# Special case for this form: 'Reverted 0x...'
reason_as_hex = data.split(" ")[1][2:]
else:
raise ContractLogicError("execution reverted", data=data)
def _raise_contract_error(response_error_data: str) -> None:
"""
Decode response error from data string and raise appropriate exception.
try:
reason_string = bytes.fromhex(reason_as_hex).decode("utf8")
raise ContractLogicError(f"execution reverted: {reason_string}", data=data)
except UnicodeDecodeError:
warnings.warn("Could not decode revert reason as UTF-8", RuntimeWarning)
raise ContractLogicError("execution reverted", data=data)
"Reverted " (prefix may be present in `data`)
Function selector for Error(string): 08c379a (4 bytes)
Data offset: 32 (32 bytes)
String length (32 bytes)
Reason string (padded, use string length from above to get meaningful part)
"""
if response_error_data.startswith("Reverted "):
reason_string = _parse_error_with_reverted_prefix(response_error_data)
raise ContractLogicError(
f"execution reverted: {reason_string}", data=response_error_data
)

# --- EIP-3668 | CCIP Read --- #
if data[:10] == OFFCHAIN_LOOKUP_FUNC_SELECTOR:
parsed_data_as_bytes = to_bytes(hexstr=data[10:])
elif response_error_data[:10] == OFFCHAIN_LOOKUP_FUNC_SELECTOR:
# --- EIP-3668 | CCIP read error --- #
parsed_data_as_bytes = to_bytes(hexstr=response_error_data[10:])
abi_decoded_data = abi.decode(
list(OFFCHAIN_LOOKUP_FIELDS.values()), parsed_data_as_bytes
)
offchain_lookup_payload = dict(
zip(OFFCHAIN_LOOKUP_FIELDS.keys(), abi_decoded_data)
)
raise OffchainLookup(offchain_lookup_payload, data=data)
raise OffchainLookup(offchain_lookup_payload, data=response_error_data)

# --- Solidity Panic Error --- #
if data[:10] == PANIC_ERROR_FUNC_SELECTOR:
panic_error_code = data[-2:]
raise ContractPanicError(PANIC_ERROR_CODES[panic_error_code], data=data)
elif response_error_data[:10] == PANIC_ERROR_FUNC_SELECTOR:
# --- Solidity Panic Error --- #
panic_error_code = response_error_data[-2:]
raise ContractPanicError(
PANIC_ERROR_CODES[panic_error_code], data=response_error_data
)

# Solidity 0.8.4 introduced custom error messages that allow args to
# be passed in (or not). See:
# https://blog.soliditylang.org/2021/04/21/custom-errors/
if len(data) >= 10 and not data[:10] == SOLIDITY_ERROR_FUNC_SELECTOR:
elif (
len(response_error_data) >= 10
and not response_error_data[:10] == SOLIDITY_ERROR_FUNC_SELECTOR
):
# Raise with data as both the message and the data for backwards
# compatibility and so that data can be accessed via 'data' attribute
# on the ContractCustomError exception
raise ContractCustomError(data, data=data)
raise ContractCustomError(response_error_data, data=response_error_data)

# Geth case:
if message_present and response["error"].get("code", "") == 3:
raise ContractLogicError(message, data=data)
# Geth Revert without error message case:
if message_present and "execution reverted" in message:
raise ContractLogicError("execution reverted", data=data)

def raise_contract_logic_error_on_revert(response: RPCResponse) -> RPCResponse:
"""
Revert responses contain an error with the following optional attributes:
`code` - in this context, used for an unknown edge case when code = '3'
`message` - error message is passed to the raised exception
`data` - response error details (str, dict, None)
See also https://solidity.readthedocs.io/en/v0.6.3/control-structures.html#revert
"""
error = response.get("error")
if error is None or isinstance(error, str):
raise ValueError(error)

message = error.get("message")
message_present = message is not None and message != ""
data = error.get("data", MISSING_DATA)

if data is None:
if message_present:
raise ContractLogicError(message, data=data)
elif not message_present:
raise ContractLogicError("execution reverted", data=data)
elif isinstance(data, dict) and message_present:
raise ContractLogicError(f"execution reverted: {message}", data=data)
elif isinstance(data, str):
_raise_contract_error(data)

if message_present:
# Geth Revert with error message and code 3 case:
if error.get("code") == 3:
raise ContractLogicError(message, data=data)
# Geth Revert without error message case:
elif "execution reverted" in message:
raise ContractLogicError("execution reverted", data=data)

return response
Loading

0 comments on commit ffec086

Please sign in to comment.