Skip to content

Conversation

TheDZhon
Copy link
Contributor

@TheDZhon TheDZhon commented Oct 15, 2025

  • Add scan summary
  • Refactor the code (streamline complexity)
  • Binary comparison default changed: opt-out instead of opt-in
  • Non-zero exit status when diffs detected
  • Allow some concrete contract diffs explicitly

@TheDZhon TheDZhon requested a review from a team as a code owner October 15, 2025 20:06
@tamtamchik tamtamchik requested a review from Copilot October 15, 2025 22:23
Copy link
Contributor

@Copilot 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 comprehensive type hints and documentation across the codebase, implements a summary feature for scan results, and refactors code for improved maintainability. The changes focus on improving code quality through better error handling, cleaner abstractions, and enhanced observability.

Key Changes:

  • Added type hints to function signatures throughout the codebase
  • Added docstrings to document function purposes, parameters, returns, and exceptions
  • Implemented a final summary feature that aggregates and displays source and bytecode comparison statistics
  • Refactored error handling to raise exceptions instead of calling sys.exit(), improving testability
  • Extracted helper functions to reduce complexity in long functions

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
diffyscan/diffyscan.py Added summary statistics tracking and final summary printing; refactored config processing with helper functions; updated function signatures with type hints
diffyscan/utils/binary_verifier.py Fixed bytecode slicing bug; refactored deep_match_bytecode by extracting helper functions; added return value to indicate match status
diffyscan/utils/calldata.py Added type hints and comprehensive docstrings; extracted validation logic into separate function
diffyscan/utils/common.py Converted error handling from sys.exit() to exceptions; refactored HTTP request handling with decorator; improved temp file handling in prettify_solidity
diffyscan/utils/compiler.py Added type hints and docstrings to all compiler-related functions
diffyscan/utils/custom_types.py Fixed typo in class name; updated TypedDict definitions with NotRequired for optional fields
diffyscan/utils/encoder.py Fixed tuple encoding logic to properly handle dynamic parts; added type hints
diffyscan/utils/explorer.py Refactored explorer fetching with helper functions; improved error handling; added validation helper; updated type hints
diffyscan/utils/github.py Added type hints to function signatures; improved documentation
diffyscan/utils/hardhat.py Added TIMEOUT_FOR_CONNECT_SEC constant
diffyscan/utils/helpers.py Added type hints and docstrings
diffyscan/utils/node_handler.py Added comprehensive docstrings and type hints

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@TheDZhon TheDZhon requested a review from a team as a code owner October 16, 2025 07:20
@TheDZhon TheDZhon linked an issue Oct 20, 2025 that may be closed by this pull request
Copy link
Member

@tamtamchik tamtamchik left a comment

Choose a reason for hiding this comment

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

👏 LGTM, only adding REMOTE_RPC_URL="..." and LOCAL_RPC_URL="..." every time is annoying, maybe add some default value

Comment on lines +337 to +338
local_rpc_url = load_env("LOCAL_RPC_URL", masked=False, required=True)
remote_rpc_url = load_env("REMOTE_RPC_URL", masked=True, required=True)
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add some default at least for LOCAL_RPC_URL here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will introduce in a separate PR

@tamtamchik tamtamchik merged commit ee76f48 into main Oct 20, 2025
4 checks passed
@tamtamchik tamtamchik deleted the feat/summary branch October 20, 2025 22:23
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.

Return exit code > 0 if some diffs were off

2 participants