Skip to content
This repository has been archived by the owner on Jan 9, 2025. It is now read-only.

overwrite fix to codehash #1654

Merged
merged 5 commits into from
Dec 4, 2024
Merged

overwrite fix to codehash #1654

merged 5 commits into from
Dec 4, 2024

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Dec 3, 2024

Overwrite fix to codehash done in #1644

Motivation:

I prefer a solution that avoids having invalid states altogether.

  • All accounts have a code_hash set to keccak256("") when deployed
  • Any modification to the account's code will also modify it's code_hash
  • extcodehash simply check the account_exists = has_code_or_nonce or has_balance condition to know whether to return 0 or code_hash

which is more inline with the spec. The codehash of an account is never zero, the extcodehash is.

Copy link

codecov bot commented Dec 3, 2024

Codecov Report

Attention: Patch coverage is 87.50000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 63.9%. Comparing base (71e205d) to head (32d43c4).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cairo_zero/backend/starknet.cairo 0.0% 1 Missing ⚠️
cairo_zero/kakarot/interfaces/interfaces.cairo 0.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1654     +/-   ##
=======================================
- Coverage   64.2%   63.9%   -0.3%     
=======================================
  Files         44      44             
  Lines       8340    8337      -3     
=======================================
- Hits        5355    5331     -24     
- Misses      2985    3006     +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@enitrat enitrat requested a review from ClementWalter December 4, 2024 07:44
@ClementWalter ClementWalter merged commit 07070b4 into main Dec 4, 2024
16 checks passed
@ClementWalter ClementWalter deleted the fix/codehash branch December 4, 2024 08:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants