Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

DeclareV2 class hash fix #788

Merged
merged 2 commits into from
Jul 7, 2023

Conversation

kkovaacs
Copy link
Contributor

@kkovaacs kkovaacs commented Jul 7, 2023

DeclareV2 should declare the class with the correct class hash

Description

DeclareV2 code used the transaction hash (maybe we could use the Rust type system to make sure no such problems are there in the code?) instead of the class hash when adding the declared class to storage.

kkovaacs added 2 commits July 7, 2023 15:06
We'll need this value when we're adding the declared class to storage
so this change moves the computation from the transaction hash calculation
function into DeclareV2::new().
The code used the _transaction hash_ instead of the Sierra class hash
to add the declared class to storage. This change fixes that to
use the class hash instead.
@codecov-commenter
Copy link

Codecov Report

Merging #788 (7f7af88) into main (a8650ac) will increase coverage by 0.63%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #788      +/-   ##
==========================================
+ Coverage   92.53%   93.17%   +0.63%     
==========================================
  Files          52       52              
  Lines       12119    12127       +8     
==========================================
+ Hits        11214    11299      +85     
+ Misses        905      828      -77     
Impacted Files Coverage Δ
src/core/transaction_hash/mod.rs 80.64% <0.00%> (+1.28%) ⬆️
src/transaction/declare_v2.rs 87.27% <77.77%> (-0.05%) ⬇️
src/lib.rs 98.71% <100.00%> (+<0.01%) ⬆️

... and 1 file with indirect coverage changes

@juanbono juanbono added this pull request to the merge queue Jul 7, 2023
Merged via the queue into lambdaclass:main with commit 0c9978a Jul 7, 2023
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.

4 participants