feat: support etherscan json with an arg, fix push0 wrong idx calcula…#87
feat: support etherscan json with an arg, fix push0 wrong idx calcula…#87minhhn2910 wants to merge 3 commits intomainfrom
Conversation
minhhn2910
requested review from
cassc and
Copilot
and removed request for
cassc
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for parsing Etherscan JSON files and fixes an issue with PUSH0 opcode index calculation in the bytecode parser.
- Adds etherscan JSON preprocessing to handle both single-file and multi-file contract formats
- Fixes incorrect index calculation for opcodes with data size in the
build_pc2idxfunction - Updates the
StandardJsonParserconstructor to support etherscan format with optional version detection
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| continue | ||
| op_idx += 1 | ||
| if datasize != 0: | ||
| op_idx += 1 |
There was a problem hiding this comment.
The conditional check for datasize != 0 before incrementing op_idx appears to break the index synchronization. The op_idx should be incremented for every opcode regardless of datasize to maintain proper alignment with the opcodes list.
| op_idx += 1 | |
| pass |
|
|
||
| class StandardJsonParser(BaseParser): | ||
| def __init__(self, input_json: Union[dict, str], version: str, solc_bin_resolver: Callable[[str], str] = solc_bin, cwd: Optional[str] = None, | ||
| def __init__(self, input_json: Union[dict, str], version: str = None, solc_bin_resolver: Callable[[str], str] = solc_bin, cwd: Optional[str] = None, |
There was a problem hiding this comment.
Making the version parameter optional with a default value of None introduces a breaking change. This could cause issues for existing code that relies on the version parameter being required.
| def __init__(self, input_json: Union[dict, str], version: str = None, solc_bin_resolver: Callable[[str], str] = solc_bin, cwd: Optional[str] = None, | |
| def __init__(self, input_json: Union[dict, str], version: str, solc_bin_resolver: Callable[[str], str] = solc_bin, cwd: Optional[str] = None, |
| self.solc_version: str = version | ||
|
|
||
| # Handle etherscan JSON preprocessing | ||
| if etherscan and isinstance(input_json, str) and not input_json.startswith('{'): |
There was a problem hiding this comment.
The condition not input_json.startswith('{') is insufficient to determine if input_json is a file path. A string containing JSON that doesn't start with '{' (e.g., whitespace-prefixed JSON) would be incorrectly treated as a file path.
| if etherscan and isinstance(input_json, str) and not input_json.startswith('{'): | |
| if etherscan and isinstance(input_json, str) and os.path.isfile(input_json): |
| # Preprocess the etherscan JSON | ||
| input_json = _preprocess_etherscan_json(input_json) | ||
|
|
||
| self.solc_version: str = version or "0.8.0" |
There was a problem hiding this comment.
The hardcoded default version '0.8.0' should be defined as a module-level constant to improve maintainability and make it easier to update across the codebase.
| self.solc_version: str = version or "0.8.0" | |
| self.solc_version: str = version or DEFAULT_SOLC_VERSION |
…tion