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

Replaced deploy_contract() in favor of set_class_hash_at() plus an availability check #850

Closed
wants to merge 10 commits into from

Conversation

xqft
Copy link
Contributor

@xqft xqft commented Jul 19, 2023

Replaced deploy_contract() in favor of set_class_hash_at() plus an availability check

Description

Closes #826

Checklist

  • Linked to Github Issue
  • Unit tests added

@codecov-commenter
Copy link

codecov-commenter commented Jul 19, 2023

Codecov Report

Merging #850 (27b6900) into main (3bbcf53) will decrease coverage by 0.14%.
The diff coverage is 66.21%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #850      +/-   ##
==========================================
- Coverage   91.74%   91.60%   -0.14%     
==========================================
  Files          54       54              
  Lines       12842    12880      +38     
==========================================
+ Hits        11782    11799      +17     
- Misses       1060     1081      +21     
Impacted Files Coverage Δ
...calls/deprecated_business_logic_syscall_handler.rs 90.23% <50.00%> (-0.47%) ⬇️
src/syscalls/business_logic_syscall_handler.rs 77.57% <60.00%> (-0.39%) ⬇️
src/state/cached_state.rs 88.00% <67.44%> (-1.52%) ⬇️
src/transaction/deploy.rs 90.32% <71.42%> (-0.29%) ⬇️
src/transaction/deploy_account.rs 88.91% <83.33%> (-0.09%) ⬇️

Comment on lines +763 to +773
let state = &mut self.starknet_storage_state.state;
match state.get_class_hash_at(&contract_address) {
Ok(x) if x == [0; 32] => {}
Ok(_) => {
return Err(
StateError::ContractAddressUnavailable(contract_address.clone()).into(),
)
}
_ => {}
}
state.set_class_hash_at(contract_address.clone(), class_hash_bytes)
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks a lot like something that could be its own function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it was deploy_contract(). We could talk with @juanbono about the decision of removing it.

@fmoletta fmoletta closed this Jan 26, 2024
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.

Remove deploy_contract from State trait and use set_class_hash_at instead.
4 participants