Skip to content

feat: mappings from bytecode to contract name#429

Merged
daejunpark merged 3 commits into
mainfrom
feat/bytecode-contract-mapping
Jan 7, 2025
Merged

feat: mappings from bytecode to contract name#429
daejunpark merged 3 commits into
mainfrom
feat/bytecode-contract-mapping

Conversation

@daejunpark

Copy link
Copy Markdown
Collaborator

No description provided.

@daejunpark daejunpark requested a review from 0xkarmacoma January 6, 2025 18:23
Comment thread src/halmos/mapper.py
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
Copy Markdown
Contributor

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
Copy Markdown
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
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ah weird 🤨

Comment thread src/halmos/mapper.py
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
Copy Markdown
Contributor

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$__

@daejunpark daejunpark Jan 7, 2025

Copy link
Copy Markdown
Collaborator Author

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
Copy Markdown
Contributor

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
Copy Markdown
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
@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