Skip to content

Commitment: rehash code only with asserts#14536

Merged
AskAlexSharov merged 7 commits into
mainfrom
trie-no-code-rehashing
Apr 12, 2025
Merged

Commitment: rehash code only with asserts#14536
AskAlexSharov merged 7 commits into
mainfrom
trie-no-code-rehashing

Conversation

@awskii

@awskii awskii commented Apr 10, 2025

Copy link
Copy Markdown
Member

disable contract code reading and hashing when asserts are off and pick code hash from ecnoded account

@awskii awskii requested review from antonis19 and shotasilagadze and removed request for antonis19 April 10, 2025 06:17
@awskii awskii requested a review from antonis19 April 10, 2025 06:36
@@ -1027,23 +1027,22 @@ func (sdc *SharedDomainsCommitmentContext) Account(plainKey []byte) (u *commitme
u.Flags |= commitment.BalanceUpdate

@antonis19 antonis19 Apr 11, 2025

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 don't know why we automatically consider this to be a balance update? What if the balance wasn't updated?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

how you gonna check? asking history is not worth the effort, easier to mark balance as updated anyways, so balance will not be ignored in the middle of something. Account() happens only for keys we really know need to load (no memoized hash | key was directly given through Updates list as being touched). Balance is used along with Nonce during hashing so only makes sense to load them both or neither.

if ch := acc.CodeHash.Bytes(); len(ch) > 0 {
u.Flags |= commitment.CodeUpdate
copy(u.CodeHash[:], ch)
copy(u.CodeHash[:], acc.CodeHash.Bytes())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what difference?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

diff is below, so we do not fetch code and hash it if we see not empty code hash

@AskAlexSharov AskAlexSharov enabled auto-merge (squash) April 12, 2025 04:18
@AskAlexSharov AskAlexSharov merged commit 94dd521 into main Apr 12, 2025
@AskAlexSharov AskAlexSharov deleted the trie-no-code-rehashing branch April 12, 2025 04:46
@awskii awskii mentioned this pull request Apr 17, 2025
15 tasks
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.

3 participants