Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: mappings from bytecode to contract name #429

Merged
merged 3 commits into from
Jan 7, 2025

Conversation

daejunpark
Copy link
Collaborator

No description provided.

@daejunpark daejunpark requested a review from 0xkarmacoma January 6, 2025 18:23
code_data = (hexcode, placeholders, contract_name, filename)
self._build_out_map_code[size].append(code_data)

def get_by_code(self, bytecode: ForwardRef("ByteVec")) -> tuple:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the options are:

  1. bytecode: ForwardRef["ByteVec"]
  2. bytecode: "ByteVec"
  3. bytecode: ByteVec (provided we from .bytevec import ByteVec)

My preference would be option (3.), I don't see a problem with importing bytevec from this module -- and going to the definition of ByteVec or unwrap() would work

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

if i remember correctly, (2) was complained by ruff, and (3) failed due to circular dependency.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah weird 🤨

def eq_except_placeholders(hexcode: str, placeholders):
last = 0
for start, end in placeholders:
if not eq_bytes(bytecode[last:start], hexcode[2 * last : 2 * start]):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, we have to do the indexing dance because for immutables placeholders look like 0000...0000 but for libraries they look like __$72fd9f18565b1bf49af679aa1eb458ccdd$__

Copy link
Collaborator Author

@daejunpark daejunpark Jan 7, 2025

Choose a reason for hiding this comment

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

yes exactly. library placeholders could be matched using regex, but immutable placeholders are complicated due to ambiguity, especially when surrounded by additional zeros. so i just used indexes for a unified approach.

@0xkarmacoma
Copy link
Collaborator

looks very clean to me! this more or less deprecates the existing Mapper.add_mapping(self, mapping: ContractMappingInfo), right? (since each contract now has its own mapping as it is deployed)

@daejunpark
Copy link
Collaborator Author

this more or less deprecates the existing Mapper.add_mapping(self, mapping: ContractMappingInfo), right? (since each contract now has its own mapping as it is deployed)

it's correct that this is related to the existing Mapper mechanism, and some functionality overlaps, e.g., get_by_bytecode() used in render_trace() for create txs, which is better to be replaced by the new get_by_code().

@daejunpark daejunpark enabled auto-merge (squash) January 7, 2025 20:36
@daejunpark daejunpark merged commit c705e37 into main Jan 7, 2025
45 checks passed
@daejunpark daejunpark deleted the feat/bytecode-contract-mapping branch January 7, 2025 20:51
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