Skip to content

Conversation

@minhhn2910
Copy link
Collaborator

…tion

@minhhn2910 minhhn2910 requested review from cassc and Copilot and removed request for cassc August 15, 2025 08:13
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_pc2idx function
  • Updates the StandardJsonParser constructor 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
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
op_idx += 1
pass

Copilot uses AI. Check for mistakes.

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,
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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,

Copilot uses AI. Check for mistakes.
self.solc_version: str = version

# Handle etherscan JSON preprocessing
if etherscan and isinstance(input_json, str) and not input_json.startswith('{'):
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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):

Copilot uses AI. Check for mistakes.
# Preprocess the etherscan JSON
input_json = _preprocess_etherscan_json(input_json)

self.solc_version: str = version or "0.8.0"
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
self.solc_version: str = version or "0.8.0"
self.solc_version: str = version or DEFAULT_SOLC_VERSION

Copilot uses AI. Check for mistakes.
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