From 74ee39d79f57094284b81a00a4669d6ef87f8af5 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Thu, 18 Jul 2024 22:23:30 +0530 Subject: [PATCH 01/17] refactor: make `build_kind_from_tags` a method on `BaseManifest` --- crates/dojo-world/src/manifest/mod.rs | 39 ++++++++++++++------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/crates/dojo-world/src/manifest/mod.rs b/crates/dojo-world/src/manifest/mod.rs index 4d5a052941..af6705123a 100644 --- a/crates/dojo-world/src/manifest/mod.rs +++ b/crates/dojo-world/src/manifest/mod.rs @@ -132,6 +132,24 @@ impl BaseManifest { self.models.retain(|model| !tags.contains(&model.inner.tag)); } + /// Generates a map of `tag -> ManifestKind` + pub fn build_kind_from_tags(&self) -> HashMap { + let mut kind_from_tags = HashMap::::new(); + + kind_from_tags.insert(WORLD_CONTRACT_TAG.to_string(), ManifestKind::WorldClass); + kind_from_tags.insert(BASE_CONTRACT_TAG.to_string(), ManifestKind::BaseClass); + + for model in self.models.as_slice() { + kind_from_tags.insert(model.inner.tag.clone(), ManifestKind::Model); + } + + for contract in self.contracts.as_slice() { + kind_from_tags.insert(contract.inner.tag.clone(), ManifestKind::Contract); + } + + kind_from_tags + } + pub fn merge(&mut self, overlay: OverlayManifest) { let mut base_map = HashMap::new(); @@ -161,7 +179,7 @@ impl BaseManifest { } #[derive(Clone, Debug, Copy)] -enum ManifestKind { +pub enum ManifestKind { BaseClass, WorldClass, Contract, @@ -169,23 +187,6 @@ enum ManifestKind { } impl OverlayManifest { - fn build_kind_from_tags(base_manifest: &BaseManifest) -> HashMap { - let mut kind_from_tags = HashMap::::new(); - - kind_from_tags.insert(WORLD_CONTRACT_TAG.to_string(), ManifestKind::WorldClass); - kind_from_tags.insert(BASE_CONTRACT_TAG.to_string(), ManifestKind::BaseClass); - - for model in base_manifest.models.as_slice() { - kind_from_tags.insert(model.inner.tag.clone(), ManifestKind::Model); - } - - for contract in base_manifest.contracts.as_slice() { - kind_from_tags.insert(contract.inner.tag.clone(), ManifestKind::Contract); - } - - kind_from_tags - } - fn load_overlay( path: &PathBuf, kind: ManifestKind, @@ -219,7 +220,7 @@ impl OverlayManifest { ) -> Result { fs::create_dir_all(path)?; - let kind_from_tags = Self::build_kind_from_tags(base_manifest); + let kind_from_tags = base_manifest.build_kind_from_tags(); let mut loaded_tags = HashMap::::new(); let mut overlays = OverlayManifest::default(); From 9256b9a660bed2528b07168a5ffb2c72259e4f08 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Sun, 21 Jul 2024 16:02:41 +0530 Subject: [PATCH 02/17] initial implementation --- crates/dojo-world/src/contracts/naming.rs | 12 ++++ crates/dojo-world/src/manifest/mod.rs | 36 ++++++++++- crates/dojo-world/src/manifest/types.rs | 9 +++ crates/sozo/ops/src/migration/auto_auth.rs | 19 +++--- crates/sozo/ops/src/migration/migrate.rs | 58 ++++++++++++++--- crates/sozo/ops/src/migration/mod.rs | 63 +++++++++++-------- .../manifests/dev/deployment/metadata.toml | 5 ++ 7 files changed, 155 insertions(+), 47 deletions(-) create mode 100644 examples/spawn-and-move/manifests/dev/deployment/metadata.toml diff --git a/crates/dojo-world/src/contracts/naming.rs b/crates/dojo-world/src/contracts/naming.rs index 93bd561b3a..5acdfe1189 100644 --- a/crates/dojo-world/src/contracts/naming.rs +++ b/crates/dojo-world/src/contracts/naming.rs @@ -68,6 +68,18 @@ pub fn get_filename_from_tag(tag: &str) -> String { format!("{tag}{TAG_SEPARATOR}{selector}") } +pub fn get_tag_from_filename(filename: &str) -> Result { + let parts: Vec<&str> = filename.split(TAG_SEPARATOR).collect(); + if parts.len() != 3 { + return Err(anyhow!( + "Unexpected filename. Expected format: \ + {TAG_SEPARATOR}{TAG_SEPARATOR}" + )); + } + + Ok(format!("{}{TAG_SEPARATOR}{}", parts[0], parts[1])) +} + pub fn compute_bytearray_hash(value: &str) -> Felt { let ba = ByteArray::from_string(value).unwrap(); poseidon_hash_many(&ByteArray::cairo_serialize(&ba)) diff --git a/crates/dojo-world/src/manifest/mod.rs b/crates/dojo-world/src/manifest/mod.rs index af6705123a..c5e295cca4 100644 --- a/crates/dojo-world/src/manifest/mod.rs +++ b/crates/dojo-world/src/manifest/mod.rs @@ -29,9 +29,10 @@ mod test; mod types; pub use types::{ - AbiFormat, BaseManifest, Class, ComputedValueEntrypoint, DeploymentManifest, DojoContract, - DojoModel, Manifest, ManifestMethods, Member, OverlayClass, OverlayContract, - OverlayDojoContract, OverlayDojoModel, OverlayManifest, WorldContract, WorldMetadata, + AbiFormat, BaseManifest, Class, ComputedValueEntrypoint, DeploymentManifest, + DeploymentMetadata, DojoContract, DojoModel, Manifest, ManifestMethods, Member, OverlayClass, + OverlayContract, OverlayDojoContract, OverlayDojoModel, OverlayManifest, WorldContract, + WorldMetadata, }; pub const WORLD_CONTRACT_TAG: &str = "dojo-world"; @@ -361,6 +362,7 @@ impl DeploymentManifest { Ok(()) } + // Writes the Deployment manifest in JSON format, with ABIs embedded. pub fn write_to_path_json(&self, path: &Utf8PathBuf, root_dir: &Utf8PathBuf) -> Result<()> { fs::create_dir_all(path.parent().unwrap())?; @@ -443,6 +445,34 @@ impl DeploymentManifest { } } +impl DeploymentMetadata { + pub fn load_from_path(path: &Utf8PathBuf) -> Result { + let manifest: Self = toml::from_str(&fs::read_to_string(path)?).unwrap(); + + Ok(manifest) + } + + pub fn write_to_path_toml(&self, path: &Utf8PathBuf) -> Result<()> { + fs::create_dir_all(path.parent().unwrap())?; + + let deployed_manifest = toml::to_string_pretty(&self)?; + fs::write(path, deployed_manifest)?; + + Ok(()) + } + + // adds any missing contracts to the metadata + // this is required so we add any newly added contract to the metadata + pub fn add_missing(&mut self, manifest: &BaseManifest) { + for contract in manifest.contracts.iter() { + let name = naming::get_tag_from_filename(&contract.manifest_name).unwrap(); + if !self.contracts.contains_key(&name) { + self.contracts.insert(name, true); + } + } + } +} + // TODO: currently implementing this method using trait is causing lifetime issue due to // `async_trait` macro which is hard to debug. So moved it as a async method on type itself. // #[async_trait] diff --git a/crates/dojo-world/src/manifest/types.rs b/crates/dojo-world/src/manifest/types.rs index f9d51b8326..58500338fb 100644 --- a/crates/dojo-world/src/manifest/types.rs +++ b/crates/dojo-world/src/manifest/types.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::fs; use camino::Utf8PathBuf; @@ -36,6 +37,14 @@ pub struct DeploymentManifest { pub models: Vec>, } +// bool represents authorization has been done for that contract +#[derive(Default, Clone, Debug, Serialize, Deserialize)] +#[cfg_attr(test, derive(PartialEq))] +pub struct DeploymentMetadata { + // manifest_name -> bool + pub contracts: HashMap, +} + #[derive(Default, Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub struct OverlayManifest { diff --git a/crates/sozo/ops/src/migration/auto_auth.rs b/crates/sozo/ops/src/migration/auto_auth.rs index 54eb7af458..10327f4b83 100644 --- a/crates/sozo/ops/src/migration/auto_auth.rs +++ b/crates/sozo/ops/src/migration/auto_auth.rs @@ -9,7 +9,6 @@ use scarb_ui::Ui; use starknet::accounts::ConnectedAccount; use super::ui::MigrationUi; -use super::MigrationOutput; use crate::auth::{grant_writer, ResourceWriter}; pub async fn auto_authorize( @@ -17,8 +16,8 @@ pub async fn auto_authorize( world: &WorldContract, txn_config: &TxnConfig, local_manifest: &BaseManifest, - migration_output: &MigrationOutput, default_namespace: &str, + work: &Vec, ) -> Result<()> where A: ConnectedAccount + Sync + Send, @@ -29,25 +28,23 @@ where ui.print(" "); ui.print_step(6, "🖋️", "Authorizing systems based on overlay..."); ui.print(" "); - let new_writers = compute_writers(&ui, local_manifest, migration_output)?; + let new_writers = create_writers(&ui, local_manifest, work)?; grant_writer(&ui, world, new_writers, *txn_config, default_namespace).await } -pub fn compute_writers( +pub fn create_writers( ui: &Ui, local_manifest: &BaseManifest, - migration_output: &MigrationOutput, + tags: &Vec, ) -> Result> { let mut res = vec![]; let local_contracts = &local_manifest.contracts; // From all the contracts that were migrated successfully. - for migrated_contract in migration_output.contracts.iter().flatten() { + for tag in tags { // Find that contract from local_manifest based on its tag. - let contract = local_contracts - .iter() - .find(|c| migrated_contract.tag == c.inner.tag) - .expect("we know this contract exists"); + let contract = + local_contracts.iter().find(|c| tag == &c.inner.tag).expect("unexpected tag found"); if !contract.inner.writes.is_empty() { ui.print_sub(format!( @@ -64,7 +61,7 @@ pub fn compute_writers( format!("m:{}", tag_with_prefix) }; - let resource = format!("{},{}", resource_type, migrated_contract.tag); + let resource = format!("{},{}", resource_type, tag); res.push(ResourceWriter::from_str(&resource)?); } } diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 5cb9bf5b6a..e8f15b41ba 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -9,9 +9,9 @@ use dojo_world::contracts::naming::{ }; use dojo_world::contracts::{cairo_utils, WorldContract}; use dojo_world::manifest::{ - AbiFormat, BaseManifest, Class, DeploymentManifest, DojoContract, DojoModel, Manifest, - ManifestMethods, WorldContract as ManifestWorldContract, WorldMetadata, ABIS_DIR, BASE_DIR, - DEPLOYMENT_DIR, MANIFESTS_DIR, + AbiFormat, BaseManifest, Class, DeploymentManifest, DeploymentMetadata, DojoContract, + DojoModel, Manifest, ManifestMethods, WorldContract as ManifestWorldContract, WorldMetadata, + ABIS_DIR, BASE_DIR, DEPLOYMENT_DIR, MANIFESTS_DIR, }; use dojo_world::metadata::{dojo_metadata_from_workspace, ResourceMetadata}; use dojo_world::migration::class::ClassMigration; @@ -803,7 +803,7 @@ pub async fn update_manifests_and_abis( world_address: Felt, migration_output: Option, salt: &str, -) -> Result<()> { +) -> Result> { let ui = ws.config().ui(); ui.print_step(5, "✨", "Updating manifests..."); @@ -811,6 +811,19 @@ pub async fn update_manifests_and_abis( let deployed_path = deployment_dir.join("manifest").with_extension("toml"); let deployed_path_json = deployment_dir.join("manifest").with_extension("json"); + let deployment_metadata_path = deployment_dir.join("metadata").with_extension("toml"); + + let mut deployment_metadata = if deployment_metadata_path.exists() { + DeploymentMetadata::load_from_path(&deployment_metadata_path)? + } else { + DeploymentMetadata::default() + }; + + deployment_metadata.add_missing(&local_manifest); + let work = calculate(migration_output.as_ref(), &mut deployment_metadata); + deployment_metadata + .write_to_path_toml(&deployment_metadata_path) + .with_context(|| "Failed to write deployment metadata")?; let mut local_manifest: DeploymentManifest = local_manifest.into(); @@ -830,6 +843,8 @@ pub async fn update_manifests_and_abis( // when the migration has not been applied because in `plan` mode or because of an error, // the `migration_output` is empty. if let Some(migration_output) = migration_output { + // update world deployment transaction hash or block number if they are present in the + // migration output if migration_output.world_tx_hash.is_some() { local_manifest.world.inner.transaction_hash = migration_output.world_tx_hash; } @@ -840,7 +855,7 @@ pub async fn update_manifests_and_abis( migration_output.contracts.iter().for_each(|contract_output| { // ignore failed migration which are represented by None if let Some(output) = contract_output { - // find the contract in local manifest and update its address and base class hash + // find the contract in local manifest and update its base class hash let local = local_manifest .contracts .iter_mut() @@ -852,6 +867,8 @@ pub async fn update_manifests_and_abis( }); } + // compute contract addresses and update them in the manifest for contracts + // that have a base class hash set. local_manifest.contracts.iter_mut().for_each(|contract| { if contract.inner.base_class_hash != Felt::ZERO { let salt = generate_salt(&get_name_from_tag(&contract.inner.tag)); @@ -864,8 +881,6 @@ pub async fn update_manifests_and_abis( } }); - // copy abi files from `base/abi` to `deployment/abi` and update abi path in - // local_manifest update_manifest_abis(&mut local_manifest, manifest_dir, profile_name).await; local_manifest @@ -879,9 +894,36 @@ pub async fn update_manifests_and_abis( .with_context(|| "Failed to write json manifest")?; ui.print("\n✨ Done."); - Ok(()) + Ok(work) +} + +fn calculate( + migration_output: Option<&MigrationOutput>, + deployment_metadata: &mut DeploymentMetadata, +) -> Vec { + if let Some(migration_output) = migration_output { + migration_output.contracts.iter().flatten().for_each(|contract_output| { + let name = contract_output.tag.clone(); + + // overwrite existing information, since we now need to do the migration + deployment_metadata.contracts.insert(name, true); + }); + } + + // given deployment_metadata.contracts return only the ones that are true + let contracts = + deployment_metadata.contracts.iter().filter(|(_, &v)| v == true).collect::>(); + + let mut work = vec![]; + for contract in contracts { + work.push(contract.0.clone()); + } + + work } +// copy abi files from `base/abi` to `deployment/abi` and update abi path in +// local_manifest async fn update_manifest_abis( local_manifest: &mut DeploymentManifest, manifest_dir: &Utf8PathBuf, diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index fc420d8b76..9150c91bf1 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -2,7 +2,9 @@ use std::sync::Arc; use anyhow::{anyhow, bail, Result}; use dojo_world::contracts::WorldContract; -use dojo_world::manifest::{BASE_DIR, MANIFESTS_DIR, OVERLAYS_DIR}; +use dojo_world::manifest::{ + DeploymentMetadata, BASE_DIR, DEPLOYMENT_DIR, MANIFESTS_DIR, OVERLAYS_DIR, +}; use dojo_world::metadata::get_default_namespace_from_ws; use dojo_world::migration::world::WorldDiff; use dojo_world::migration::{DeployOutput, TxnConfig, UpgradeOutput}; @@ -116,9 +118,7 @@ where if total_diffs == 0 { ui.print("\n✨ No changes to be made. Remote World is already up to date!"); - return Ok(None); } - let mut strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; let world_address = strategy.world_address().expect("world address must exist"); strategy.resolve_variable(world_address)?; @@ -126,7 +126,7 @@ where if dry_run { print_strategy(&ui, account.provider(), &strategy, world_address).await; - update_manifests_and_abis( + let work = update_manifests_and_abis( ws, local_manifest, &manifest_dir, @@ -138,13 +138,15 @@ where ) .await?; + dbg!(&work); + Ok(None) } else { // Migrate according to the diff. let migration_output = match apply_diff(ws, &account, txn_config, &mut strategy).await { Ok(migration_output) => Some(migration_output), Err(e) => { - update_manifests_and_abis( + let _ = update_manifests_and_abis( ws, local_manifest, &manifest_dir, @@ -159,7 +161,7 @@ where } }; - update_manifests_and_abis( + let work = update_manifests_and_abis( ws, local_manifest.clone(), &manifest_dir, @@ -173,26 +175,37 @@ where let account = Arc::new(account); let world = WorldContract::new(world_address, account.clone()); - if let Some(migration_output) = &migration_output { - match auto_authorize( - ws, - &world, - &txn_config, - &local_manifest, - migration_output, - &default_namespace, - ) + + dbg!(&work); + match auto_authorize(ws, &world, &txn_config, &local_manifest, &default_namespace, &work) .await - { - Ok(()) => { - ui.print_sub("Auto authorize completed successfully"); - } - Err(e) => { - ui.print_sub(format!("Failed to auto authorize with error: {e}")); - } - }; - - // + { + Ok(()) => { + let deployment_dir = manifest_dir.join(DEPLOYMENT_DIR); + let deployment_metadata_path = + deployment_dir.join("metadata").with_extension("toml"); + + // read back metadata file and update it + let mut deployment_metadata = + DeploymentMetadata::load_from_path(&deployment_metadata_path)?; + + work.iter().for_each(|tag| { + let contract = + deployment_metadata.contracts.get_mut(tag).expect("unexpected tag found"); + + *contract = false; + }); + + deployment_metadata.write_to_path_toml(&deployment_metadata_path)?; + + ui.print_sub("Auto authorize completed successfully"); + } + Err(e) => { + ui.print_sub(format!("Failed to auto authorize with error: {e}")); + } + }; + + if let Some(migration_output) = &migration_output { if !ws.config().offline() { upload_metadata(ws, &account, migration_output.clone(), txn_config).await?; } diff --git a/examples/spawn-and-move/manifests/dev/deployment/metadata.toml b/examples/spawn-and-move/manifests/dev/deployment/metadata.toml new file mode 100644 index 0000000000..c3b7f4b53a --- /dev/null +++ b/examples/spawn-and-move/manifests/dev/deployment/metadata.toml @@ -0,0 +1,5 @@ +[contracts] +dojo_examples-mock_token = false +dojo_examples-others = false +dojo_examples-dungeon = false +dojo_examples-actions = false From 06ac5cbffae584af36fd63300a37f879e56a8786 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Mon, 22 Jul 2024 17:24:52 +0530 Subject: [PATCH 03/17] tmp --- crates/sozo/ops/src/migration/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index 9150c91bf1..cb0a7e18ca 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -46,6 +46,11 @@ pub struct ContractMigrationOutput { pub base_class_hash: Felt, } +// TODO: when diff is zero we still need to run auto authorize +// TODO: read deployment manifest to find diff in `writes` for auto authorize +// TODO: update method names to better reflect what they now do +// TODO: add tests +// TODO: general cleanup #[allow(clippy::too_many_arguments)] pub async fn migrate( ws: &Workspace<'_>, @@ -117,8 +122,9 @@ where ui.print_sub(format!("Total diffs found: {total_diffs}")); if total_diffs == 0 { - ui.print("\n✨ No changes to be made. Remote World is already up to date!"); + ui.print("\n✨ No diffs found. Remote World is already up to date!"); } + let mut strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; let world_address = strategy.world_address().expect("world address must exist"); strategy.resolve_variable(world_address)?; @@ -176,7 +182,6 @@ where let account = Arc::new(account); let world = WorldContract::new(world_address, account.clone()); - dbg!(&work); match auto_authorize(ws, &world, &txn_config, &local_manifest, &default_namespace, &work) .await { From b08b52932f6b5d8d53fba5f4531616e5bc12ca65 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Mon, 22 Jul 2024 17:54:06 +0530 Subject: [PATCH 04/17] refactor: update the order in `compute` itself --- crates/dojo-test-utils/src/migration.rs | 6 ++---- crates/dojo-world/src/contracts/world_test.rs | 3 +-- crates/dojo-world/src/manifest/manifest_test.rs | 2 +- crates/dojo-world/src/migration/world.rs | 11 +++++++++-- crates/dojo-world/src/migration/world_test.rs | 4 ++-- crates/sozo/ops/src/migration/mod.rs | 6 ++---- crates/sozo/ops/src/tests/migration.rs | 10 +++++----- 7 files changed, 22 insertions(+), 20 deletions(-) diff --git a/crates/dojo-test-utils/src/migration.rs b/crates/dojo-test-utils/src/migration.rs index fff72ec87c..a481e51553 100644 --- a/crates/dojo-test-utils/src/migration.rs +++ b/crates/dojo-test-utils/src/migration.rs @@ -32,8 +32,7 @@ pub fn prepare_migration( manifest.merge(overlay_manifest); } - let mut world = WorldDiff::compute(manifest, None); - world.update_order(default_namespace).unwrap(); + let world = WorldDiff::compute(manifest, None, default_namespace)?; let mut strat = prepare_for_migration(None, felt!("0x12345"), &target_dir, world).unwrap(); strat.resolve_variable(strat.world_address().unwrap()).unwrap(); @@ -62,8 +61,7 @@ pub fn prepare_migration_with_world_and_seed( manifest.merge(overlay_manifest); } - let mut world = WorldDiff::compute(manifest, None); - world.update_order(default_namespace).unwrap(); + let world = WorldDiff::compute(manifest, None, default_namespace)?; let seed = cairo_short_string_to_felt(seed).unwrap(); prepare_for_migration(world_address, seed, &target_dir, world) diff --git a/crates/dojo-world/src/contracts/world_test.rs b/crates/dojo-world/src/contracts/world_test.rs index 22aade32f3..9c18a4b05a 100644 --- a/crates/dojo-world/src/contracts/world_test.rs +++ b/crates/dojo-world/src/contracts/world_test.rs @@ -75,8 +75,7 @@ pub async fn deploy_world( manifest.merge(overlay_manifest); } - let mut world = WorldDiff::compute(manifest.clone(), None); - world.update_order(default_namespace).unwrap(); + let world = WorldDiff::compute(manifest.clone(), None, default_namespace).unwrap(); let account = sequencer.account(0); diff --git a/crates/dojo-world/src/manifest/manifest_test.rs b/crates/dojo-world/src/manifest/manifest_test.rs index f59b85063a..4eb5920bfe 100644 --- a/crates/dojo-world/src/manifest/manifest_test.rs +++ b/crates/dojo-world/src/manifest/manifest_test.rs @@ -357,7 +357,7 @@ fn fetch_remote_manifest() { // compute diff from local and remote manifest - let diff = WorldDiff::compute(local_manifest, Some(remote_manifest)); + let diff = WorldDiff::compute(local_manifest, Some(remote_manifest), &"dojo-test").unwrap(); assert_eq!(diff.count_diffs(), 0, "there should not be any diff"); } diff --git a/crates/dojo-world/src/migration/world.rs b/crates/dojo-world/src/migration/world.rs index 9a703f1338..e66314c781 100644 --- a/crates/dojo-world/src/migration/world.rs +++ b/crates/dojo-world/src/migration/world.rs @@ -28,7 +28,11 @@ pub struct WorldDiff { } impl WorldDiff { - pub fn compute(local: BaseManifest, remote: Option) -> WorldDiff { + pub fn compute( + local: BaseManifest, + remote: Option, + default_namespace: &str, + ) -> Result { let models = local .models .iter() @@ -90,7 +94,10 @@ impl WorldDiff { init_calldata: vec![], }; - WorldDiff { world, base, contracts, models } + let mut diff = WorldDiff { world, base, contracts, models }; + diff.update_order(&default_namespace)?; + + Ok(diff) } pub fn count_diffs(&self) -> usize { diff --git a/crates/dojo-world/src/migration/world_test.rs b/crates/dojo-world/src/migration/world_test.rs index 94c85a51ef..465e400741 100644 --- a/crates/dojo-world/src/migration/world_test.rs +++ b/crates/dojo-world/src/migration/world_test.rs @@ -32,7 +32,7 @@ fn no_diff_when_local_and_remote_are_equal() { let mut remote: DeploymentManifest = local.clone().into(); remote.models = remote_models; - let diff = WorldDiff::compute(local, Some(remote)); + let diff = WorldDiff::compute(local, Some(remote), &"dojo-test").unwrap(); assert_eq!(diff.count_diffs(), 0); } @@ -121,7 +121,7 @@ fn diff_when_local_and_remote_are_different() { remote.models[1].inner.class_hash = 33_u32.into(); remote.contracts[0].inner.class_hash = felt!("0x1112"); - let diff = WorldDiff::compute(local, Some(remote)); + let diff = WorldDiff::compute(local, Some(remote), &"dojo-test").unwrap(); assert_eq!(diff.count_diffs(), 3); assert!(diff.models.iter().any(|m| m.tag == get_tag("dojo_mock", "model2"))); diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index cb0a7e18ca..a46163d06d 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -115,8 +115,8 @@ where // Calculate diff between local and remote World manifests. ui.print_step(2, "🧰", "Evaluating Worlds diff..."); - let mut diff = WorldDiff::compute(local_manifest.clone(), remote_manifest.clone()); - diff.update_order(&default_namespace)?; + let diff = + WorldDiff::compute(local_manifest.clone(), remote_manifest.clone(), &default_namespace)?; let total_diffs = diff.count_diffs(); ui.print_sub(format!("Total diffs found: {total_diffs}")); @@ -144,8 +144,6 @@ where ) .await?; - dbg!(&work); - Ok(None) } else { // Migrate according to the diff. diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index ad061f59f7..b9a573b6bd 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -136,7 +136,7 @@ async fn metadata_calculated_properly() { manifest.merge(overlay_manifest); } - let world = WorldDiff::compute(manifest, None); + let world = WorldDiff::compute(manifest, None, &"dojo-test").unwrap(); let migration = prepare_for_migration( None, @@ -175,7 +175,7 @@ async fn migration_with_correct_calldata_second_time_work_as_expected() { ) .unwrap(); - let world = WorldDiff::compute(manifest.clone(), None); + let world = WorldDiff::compute(manifest.clone(), None, &"dojo-test").unwrap(); let migration = prepare_for_migration( None, @@ -207,8 +207,8 @@ async fn migration_with_correct_calldata_second_time_work_as_expected() { } let default_namespace = get_default_namespace_from_ws(&ws).unwrap(); - let mut world = WorldDiff::compute(manifest, Some(remote_manifest)); - world.update_order(&default_namespace).expect("Failed to update order"); + let world = WorldDiff::compute(manifest, Some(remote_manifest), &default_namespace) + .expect("failed to update order"); let mut migration = prepare_for_migration( Some(world_address), @@ -243,7 +243,7 @@ async fn migration_from_remote() { ) .unwrap(); - let world = WorldDiff::compute(manifest, None); + let world = WorldDiff::compute(manifest, None, &"dojo-test").unwrap(); let migration = prepare_for_migration( None, From 9cd77478e84fd93792a544f9267e9570ef5dd1e0 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Tue, 23 Jul 2024 20:05:07 +0530 Subject: [PATCH 05/17] refactor: resolve variables in `prepare_for_migration` itself --- bin/sozo/tests/register_test.rs | 2 +- crates/dojo-test-utils/src/migration.rs | 3 +- crates/dojo-world/src/contracts/world_test.rs | 3 +- crates/dojo-world/src/migration/strategy.rs | 20 ++--- crates/sozo/ops/src/migration/migrate.rs | 74 ++++++++----------- crates/sozo/ops/src/migration/mod.rs | 16 ++-- crates/sozo/ops/src/tests/migration.rs | 10 +-- crates/sozo/ops/src/tests/setup.rs | 4 +- .../grpc/src/server/tests/entities_test.rs | 5 +- examples/spawn-and-move/Scarb.toml | 13 ++-- 10 files changed, 63 insertions(+), 87 deletions(-) diff --git a/bin/sozo/tests/register_test.rs b/bin/sozo/tests/register_test.rs index ce0f84d83f..b882b873c9 100644 --- a/bin/sozo/tests/register_test.rs +++ b/bin/sozo/tests/register_test.rs @@ -41,7 +41,7 @@ async fn reregister_models() { account.set_block_id(BlockId::Tag(BlockTag::Pending)); execute_strategy(&ws, &migration, &account, TxnConfig::init_wait()).await.unwrap(); - let world_address = &format!("0x{:x}", &migration.world_address().unwrap()); + let world_address = &format!("0x{:x}", &migration.world_address); let account_address = &format!("0x{:x}", account.address()); let private_key = &format!("0x{:x}", sequencer.account_data(0).private_key.as_ref().unwrap().secret_scalar()); diff --git a/crates/dojo-test-utils/src/migration.rs b/crates/dojo-test-utils/src/migration.rs index a481e51553..c8cb6110a7 100644 --- a/crates/dojo-test-utils/src/migration.rs +++ b/crates/dojo-test-utils/src/migration.rs @@ -34,8 +34,7 @@ pub fn prepare_migration( let world = WorldDiff::compute(manifest, None, default_namespace)?; - let mut strat = prepare_for_migration(None, felt!("0x12345"), &target_dir, world).unwrap(); - strat.resolve_variable(strat.world_address().unwrap()).unwrap(); + let strat = prepare_for_migration(None, felt!("0x12345"), &target_dir, world).unwrap(); Ok(strat) } diff --git a/crates/dojo-world/src/contracts/world_test.rs b/crates/dojo-world/src/contracts/world_test.rs index 9c18a4b05a..42141c7da7 100644 --- a/crates/dojo-world/src/contracts/world_test.rs +++ b/crates/dojo-world/src/contracts/world_test.rs @@ -79,9 +79,8 @@ pub async fn deploy_world( let account = sequencer.account(0); - let mut strategy = + let strategy = prepare_for_migration(None, Felt::from_hex("0x12345").unwrap(), target_dir, world).unwrap(); - strategy.resolve_variable(strategy.world_address().unwrap()).unwrap(); let base_class_hash = strategy.base.unwrap().declare(&account, &TxnConfig::init_wait()).await.unwrap().class_hash; diff --git a/crates/dojo-world/src/migration/strategy.rs b/crates/dojo-world/src/migration/strategy.rs index 3e221d27ba..0a901ebe01 100644 --- a/crates/dojo-world/src/migration/strategy.rs +++ b/crates/dojo-world/src/migration/strategy.rs @@ -22,7 +22,7 @@ pub enum MigrationMetadata { #[derive(Debug, Clone)] pub struct MigrationStrategy { - pub world_address: Option, + pub world_address: Felt, pub world: Option, pub base: Option, pub contracts: Vec, @@ -37,13 +37,6 @@ pub struct MigrationItemsInfo { } impl MigrationStrategy { - pub fn world_address(&self) -> Result { - match &self.world { - Some(c) => Ok(c.contract_address), - None => self.world_address.ok_or(anyhow!("World address not found")), - } - } - pub fn info(&self) -> MigrationItemsInfo { let mut new = 0; let mut update = 0; @@ -145,7 +138,16 @@ pub fn prepare_for_migration( world.contract_address = generated_world_address; } - Ok(MigrationStrategy { world_address, world, base, contracts, models, metadata }) + // If world address is not provided, then we expect the world to be migrated. + // TODO: see if there are any case where world_address is not provided and `world` is none + let world_address = world_address.unwrap_or(world.as_ref().unwrap().contract_address); + + let mut migration = + MigrationStrategy { world_address, world, base, contracts, models, metadata }; + + migration.resolve_variable(world_address)?; + + Ok(migration) } fn evaluate_models_to_migrate( diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index e8f15b41ba..14900dff37 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -71,7 +71,7 @@ pub async fn apply_diff( ws: &Workspace<'_>, account: A, txn_config: TxnConfig, - strategy: &mut MigrationStrategy, + strategy: &MigrationStrategy, ) -> Result where A: ConnectedAccount + Sync + Send, @@ -93,27 +93,18 @@ where ui.print(format!( "\n🎉 Successfully migrated World on block #{} at address {}\n", block_number, - bold_message(format!( - "{:#x}", - strategy.world_address().expect("world address must exist") - )) + bold_message(format!("{:#x}", strategy.world_address)) )); } else { ui.print(format!( "\n🎉 Successfully migrated World at address {}\n", - bold_message(format!( - "{:#x}", - strategy.world_address().expect("world address must exist") - )) + bold_message(format!("{:#x}", strategy.world_address)) )); } } else { ui.print(format!( "\n🚨 Partially migrated World at address {}", - bold_message(format!( - "{:#x}", - strategy.world_address().expect("world address must exist") - )) + bold_message(format!("{:#x}", strategy.world_address)) )); } @@ -207,8 +198,9 @@ where None => {} }; + let world_address = strategy.world_address; let mut migration_output = MigrationOutput { - world_address: strategy.world_address()?, + world_address, world_tx_hash, world_block_number, full: false, @@ -216,8 +208,6 @@ where contracts: vec![], }; - let world_address = strategy.world_address()?; - // register namespaces let mut namespaces = strategy.models.iter().map(|m| get_namespace_from_tag(&m.diff.tag)).collect::>(); @@ -700,42 +690,38 @@ pub fn handle_artifact_error(ui: &Ui, artifact_path: &Path, error: anyhow::Error pub async fn get_contract_operation_name

( provider: P, contract: &ContractMigration, - world_address: Option, + world_address: Felt, ) -> String where P: Provider + Sync + Send, { - if let Some(world_address) = world_address { - if let Ok(base_class_hash) = provider - .call( - FunctionCall { - contract_address: world_address, - calldata: vec![], - entry_point_selector: get_selector_from_name("base").unwrap(), - }, - BlockId::Tag(BlockTag::Pending), - ) - .await - { - let contract_address = - get_contract_address(contract.salt, base_class_hash[0], &[], world_address); + if let Ok(base_class_hash) = provider + .call( + FunctionCall { + contract_address: world_address, + calldata: vec![], + entry_point_selector: get_selector_from_name("base").unwrap(), + }, + BlockId::Tag(BlockTag::Pending), + ) + .await + { + let contract_address = + get_contract_address(contract.salt, base_class_hash[0], &[], world_address); - match provider - .get_class_hash_at(BlockId::Tag(BlockTag::Pending), contract_address) - .await - { - Ok(current_class_hash) if current_class_hash != contract.diff.local_class_hash => { - return format!("{}: Upgrade", contract.diff.tag); - } - Err(ProviderError::StarknetError(StarknetError::ContractNotFound)) => { - return format!("{}: Deploy", contract.diff.tag); - } - Ok(_) => return "Already Deployed".to_string(), - Err(_) => return format!("{}: Deploy", contract.diff.tag), + match provider.get_class_hash_at(BlockId::Tag(BlockTag::Pending), contract_address).await { + Ok(current_class_hash) if current_class_hash != contract.diff.local_class_hash => { + return format!("{}: Upgrade", contract.diff.tag); } + Err(ProviderError::StarknetError(StarknetError::ContractNotFound)) => { + return format!("{}: Deploy", contract.diff.tag); + } + Ok(_) => return "Already Deployed".to_string(), + Err(_) => return format!("{}: Deploy", contract.diff.tag), } + } else { + format!("{}: Deploy", contract.diff.tag) } - format!("deploy {}", contract.diff.tag) } pub async fn print_strategy

( diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index a46163d06d..d6a9bfc340 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -125,12 +125,10 @@ where ui.print("\n✨ No diffs found. Remote World is already up to date!"); } - let mut strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; - let world_address = strategy.world_address().expect("world address must exist"); - strategy.resolve_variable(world_address)?; + let strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; if dry_run { - print_strategy(&ui, account.provider(), &strategy, world_address).await; + print_strategy(&ui, account.provider(), &strategy, strategy.world_address).await; let work = update_manifests_and_abis( ws, @@ -138,7 +136,7 @@ where &manifest_dir, &profile_name, &rpc_url, - world_address, + strategy.world_address, None, name, ) @@ -147,7 +145,7 @@ where Ok(None) } else { // Migrate according to the diff. - let migration_output = match apply_diff(ws, &account, txn_config, &mut strategy).await { + let migration_output = match apply_diff(ws, &account, txn_config, &strategy).await { Ok(migration_output) => Some(migration_output), Err(e) => { let _ = update_manifests_and_abis( @@ -156,7 +154,7 @@ where &manifest_dir, &profile_name, &rpc_url, - world_address, + strategy.world_address, None, name, ) @@ -171,14 +169,14 @@ where &manifest_dir, &profile_name, &rpc_url, - world_address, + strategy.world_address, migration_output.clone(), name, ) .await?; let account = Arc::new(account); - let world = WorldContract::new(world_address, account.clone()); + let world = WorldContract::new(strategy.world_address, account.clone()); match auto_authorize(ws, &world, &txn_config, &local_manifest, &default_namespace, &work) .await diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index b9a573b6bd..d1e7c7cb4f 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -210,14 +210,13 @@ async fn migration_with_correct_calldata_second_time_work_as_expected() { let world = WorldDiff::compute(manifest, Some(remote_manifest), &default_namespace) .expect("failed to update order"); - let mut migration = prepare_for_migration( + let migration = prepare_for_migration( Some(world_address), felt!("0x12345"), &Utf8Path::new(&target_dir).to_path_buf(), world, ) .unwrap(); - migration.resolve_variable(migration.world_address().unwrap()).expect("Failed to resolve"); let migration_output = execute_strategy(&ws, &migration, &account, TxnConfig::init_wait()).await.unwrap(); @@ -262,7 +261,7 @@ async fn migration_from_remote() { let remote_manifest = DeploymentManifest::load_from_remote( JsonRpcClient::new(HttpTransport::new(sequencer.url())), - migration.world_address().unwrap(), + migration.world_address, ) .await .unwrap(); @@ -349,8 +348,7 @@ async fn migrate_with_auto_authorize() { let config = setup::load_config(); let ws = setup::setup_ws(&config); - let mut migration = setup::setup_migration(&config).unwrap(); - migration.resolve_variable(migration.world_address().unwrap()).unwrap(); + let migration = setup::setup_migration(&config).unwrap(); let manifest_base = config.manifest_path().parent().unwrap(); let mut manifest = @@ -372,7 +370,7 @@ async fn migrate_with_auto_authorize() { let output = execute_strategy(&ws, &migration, &account, txn_config).await.unwrap(); - let world_address = migration.world_address().expect("must be present"); + let world_address = migration.world_address; let world = WorldContract::new(world_address, account); let default_namespace = get_default_namespace_from_ws(&ws).unwrap(); diff --git a/crates/sozo/ops/src/tests/setup.rs b/crates/sozo/ops/src/tests/setup.rs index 06d8f8a779..0433b0a995 100644 --- a/crates/sozo/ops/src/tests/setup.rs +++ b/crates/sozo/ops/src/tests/setup.rs @@ -83,9 +83,7 @@ pub async fn setup( let config = load_config(); let ws = setup_ws(&config); - let mut migration = setup_migration(&config)?; - let _ = - migration.resolve_variable(migration.world_address().expect("world address must exist")); + let migration = setup_migration(&config)?; let mut account = sequencer.account(0); account.set_block_id(BlockId::Tag(BlockTag::Pending)); diff --git a/crates/torii/grpc/src/server/tests/entities_test.rs b/crates/torii/grpc/src/server/tests/entities_test.rs index 196bca0bda..682ac2226f 100644 --- a/crates/torii/grpc/src/server/tests/entities_test.rs +++ b/crates/torii/grpc/src/server/tests/entities_test.rs @@ -49,20 +49,19 @@ async fn test_entities_queries() { let default_namespace = get_default_namespace_from_ws(&ws).unwrap(); - let mut migration = prepare_migration( + let migration = prepare_migration( config.manifest_path().parent().unwrap().into(), target_path, dojo_metadata.skip_migration, &default_namespace, ) .unwrap(); - migration.resolve_variable(migration.world_address().unwrap()).unwrap(); let sequencer = KatanaRunner::new().expect("Fail to start runner"); let provider = Arc::new(JsonRpcClient::new(HttpTransport::new(sequencer.url()))); - let world = WorldContractReader::new(migration.world_address().unwrap(), &provider); + let world = WorldContractReader::new(migration.world_address, &provider); let account = sequencer.account(0); diff --git a/examples/spawn-and-move/Scarb.toml b/examples/spawn-and-move/Scarb.toml index ca338c7df5..5c1195a2cd 100644 --- a/examples/spawn-and-move/Scarb.toml +++ b/examples/spawn-and-move/Scarb.toml @@ -10,19 +10,16 @@ edition = "2023_11" sierra-replace-ids = true [dependencies] -dojo = { path = "../../crates/dojo-core" } armory = { path = "../game-lib/armory" } bestiary = { path = "../game-lib/bestiary" } +dojo = { path = "../../crates/dojo-core" } [[target.dojo]] -build-external-contracts = [ - "armory::Flatbow", - "bestiary::RiverSkale", -] +build-external-contracts = [ "armory::Flatbow", "bestiary::RiverSkale" ] [features] -default = ["dungeon"] -dungeon = [] +default = [ "dungeon" ] +dungeon = [ ] # `dev` profile @@ -43,7 +40,7 @@ rpc_url = "http://localhost:5050/" # Default account for katana with seed = 0 account_address = "0x6162896d1d7ab204c7ccac6dd5f8e9e7c25ecd5ae4fcb4ad32e57786bb46e03" private_key = "0x1800000000300000180000000000030000000000003006001800006600" -world_address = "0x5ffa124a5076e590a23e9d393ebe4fc7c66989c2cd228a46f38a9e2d48305a7" +# world_address = "0x5ffa124a5076e590a23e9d393ebe4fc7c66989c2cd228a46f38a9e2d48305a7" [profile.release.tool.dojo] # for more info on how `merge-strategy` works see: From 6537fbc9fd46c5542e03da1210ff8039d53bb74e Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 24 Jul 2024 17:58:26 +0530 Subject: [PATCH 06/17] refactor: separate out updating manifest and deployment metadata --- crates/sozo/ops/src/migration/migrate.rs | 38 +++++++++++------- crates/sozo/ops/src/migration/mod.rs | 49 ++++++++++++++---------- 2 files changed, 53 insertions(+), 34 deletions(-) diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 14900dff37..546d100543 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -789,7 +789,7 @@ pub async fn update_manifests_and_abis( world_address: Felt, migration_output: Option, salt: &str, -) -> Result> { +) -> Result<()> { let ui = ws.config().ui(); ui.print_step(5, "✨", "Updating manifests..."); @@ -797,19 +797,6 @@ pub async fn update_manifests_and_abis( let deployed_path = deployment_dir.join("manifest").with_extension("toml"); let deployed_path_json = deployment_dir.join("manifest").with_extension("json"); - let deployment_metadata_path = deployment_dir.join("metadata").with_extension("toml"); - - let mut deployment_metadata = if deployment_metadata_path.exists() { - DeploymentMetadata::load_from_path(&deployment_metadata_path)? - } else { - DeploymentMetadata::default() - }; - - deployment_metadata.add_missing(&local_manifest); - let work = calculate(migration_output.as_ref(), &mut deployment_metadata); - deployment_metadata - .write_to_path_toml(&deployment_metadata_path) - .with_context(|| "Failed to write deployment metadata")?; let mut local_manifest: DeploymentManifest = local_manifest.into(); @@ -880,6 +867,29 @@ pub async fn update_manifests_and_abis( .with_context(|| "Failed to write json manifest")?; ui.print("\n✨ Done."); + Ok(()) +} + +pub fn update_deployment_metadata( + manifest_dir: &Utf8PathBuf, + local_manifest: &BaseManifest, + migration_output: Option<&MigrationOutput>, +) -> Result> { + let deployment_dir = manifest_dir.join(DEPLOYMENT_DIR); + let deployment_metadata_path = deployment_dir.join("metadata").with_extension("toml"); + + let mut deployment_metadata = if deployment_metadata_path.exists() { + DeploymentMetadata::load_from_path(&deployment_metadata_path)? + } else { + DeploymentMetadata::default() + }; + + deployment_metadata.add_missing(local_manifest); + let work = calculate(migration_output, &mut deployment_metadata); + deployment_metadata + .write_to_path_toml(&deployment_metadata_path) + .with_context(|| "Failed to write deployment metadata")?; + Ok(work) } diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index d6a9bfc340..e5f812775d 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -20,10 +20,10 @@ mod ui; mod utils; pub use self::auto_auth::auto_authorize; -use self::migrate::update_manifests_and_abis; pub use self::migrate::{ apply_diff, execute_strategy, prepare_migration, print_strategy, upload_metadata, }; +use self::migrate::{update_deployment_metadata, update_manifests_and_abis}; use self::ui::MigrationUi; #[derive(Debug, Default, Clone)] @@ -126,11 +126,14 @@ where } let strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; - if dry_run { + if total_diffs == 0 { + return Ok(None); + } + print_strategy(&ui, account.provider(), &strategy, strategy.world_address).await; - let work = update_manifests_and_abis( + update_manifests_and_abis( ws, local_manifest, &manifest_dir, @@ -144,26 +147,29 @@ where Ok(None) } else { - // Migrate according to the diff. - let migration_output = match apply_diff(ws, &account, txn_config, &strategy).await { - Ok(migration_output) => Some(migration_output), - Err(e) => { - let _ = update_manifests_and_abis( - ws, - local_manifest, - &manifest_dir, - &profile_name, - &rpc_url, - strategy.world_address, - None, - name, - ) - .await?; - return Err(e)?; + let migration_output = if total_diffs != 0 { + match apply_diff(ws, &account, txn_config, &strategy).await { + Ok(migration_output) => Some(migration_output), + Err(e) => { + update_manifests_and_abis( + ws, + local_manifest, + &manifest_dir, + &profile_name, + &rpc_url, + strategy.world_address, + None, + name, + ) + .await?; + return Err(e)?; + } } + } else { + None }; - let work = update_manifests_and_abis( + update_manifests_and_abis( ws, local_manifest.clone(), &manifest_dir, @@ -175,6 +181,9 @@ where ) .await?; + let work = + update_deployment_metadata(&manifest_dir, &local_manifest, migration_output.as_ref())?; + let account = Arc::new(account); let world = WorldContract::new(strategy.world_address, account.clone()); From a1e49538679d31bcc47fef563576efefb7f942bc Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 24 Jul 2024 20:09:54 +0530 Subject: [PATCH 07/17] fix tests --- crates/dojo-world/src/migration/strategy.rs | 2 +- crates/sozo/ops/src/migration/mod.rs | 5 ++-- crates/sozo/ops/src/tests/migration.rs | 31 +++++++++++---------- examples/spawn-and-move/Scarb.toml | 2 +- 4 files changed, 21 insertions(+), 19 deletions(-) diff --git a/crates/dojo-world/src/migration/strategy.rs b/crates/dojo-world/src/migration/strategy.rs index 0a901ebe01..46fe09bc34 100644 --- a/crates/dojo-world/src/migration/strategy.rs +++ b/crates/dojo-world/src/migration/strategy.rs @@ -140,7 +140,7 @@ pub fn prepare_for_migration( // If world address is not provided, then we expect the world to be migrated. // TODO: see if there are any case where world_address is not provided and `world` is none - let world_address = world_address.unwrap_or(world.as_ref().unwrap().contract_address); + let world_address = world_address.unwrap_or_else(|| world.as_ref().unwrap().contract_address); let mut migration = MigrationStrategy { world_address, world, base, contracts, models, metadata }; diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index e5f812775d..163ad2933a 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -20,10 +20,11 @@ mod ui; mod utils; pub use self::auto_auth::auto_authorize; +use self::migrate::update_manifests_and_abis; pub use self::migrate::{ - apply_diff, execute_strategy, prepare_migration, print_strategy, upload_metadata, + apply_diff, execute_strategy, prepare_migration, print_strategy, update_deployment_metadata, + upload_metadata, }; -use self::migrate::{update_deployment_metadata, update_manifests_and_abis}; use self::ui::MigrationUi; #[derive(Debug, Default, Clone)] diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index d1e7c7cb4f..0b28203ad1 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -26,7 +26,9 @@ use starknet::providers::jsonrpc::HttpTransport; use starknet::providers::JsonRpcClient; use super::setup; -use crate::migration::{auto_authorize, execute_strategy, upload_metadata}; +use crate::migration::{ + auto_authorize, execute_strategy, update_deployment_metadata, upload_metadata, +}; use crate::utils::get_contract_address_from_reader; #[tokio::test(flavor = "multi_thread")] @@ -103,16 +105,14 @@ async fn migrate_with_small_fee_multiplier_will_fail() { let account = sequencer.account(0); - assert!( - execute_strategy( - &ws, - &migration, - &account, - TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, - ) - .await - .is_err() - ); + assert!(execute_strategy( + &ws, + &migration, + &account, + TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, + ) + .await + .is_err()); } #[tokio::test] @@ -188,7 +188,7 @@ async fn migration_with_correct_calldata_second_time_work_as_expected() { let migration_output = execute_strategy(&ws, &migration, &account, TxnConfig::init_wait()).await.unwrap(); - // first time others will fail due to calldata error + // first time DojoContract named `others` will fail due to calldata error assert!(!migration_output.full); let world_address = migration_output.world_address; @@ -373,9 +373,10 @@ async fn migrate_with_auto_authorize() { let world_address = migration.world_address; let world = WorldContract::new(world_address, account); + let work = + update_deployment_metadata(&manifest_base.to_path_buf(), &manifest, Some(&output)).unwrap(); let default_namespace = get_default_namespace_from_ws(&ws).unwrap(); - let res = - auto_authorize(&ws, &world, &txn_config, &manifest, &output, &default_namespace).await; + let res = auto_authorize(&ws, &world, &txn_config, &manifest, &default_namespace, &work).await; assert!(res.is_ok()); let provider = sequencer.provider(); @@ -419,7 +420,7 @@ async fn migration_with_mismatching_world_address_and_seed() { // The strategy.world has it's address set with the seed directly, and not // from the world address provided by the user. - assert_ne!(strategy.world_address.unwrap(), strategy.world.unwrap().contract_address); + assert_ne!(strategy.world_address, strategy.world.unwrap().contract_address); } /// Get the hash from a IPFS URI diff --git a/examples/spawn-and-move/Scarb.toml b/examples/spawn-and-move/Scarb.toml index 5c1195a2cd..8e98ed8759 100644 --- a/examples/spawn-and-move/Scarb.toml +++ b/examples/spawn-and-move/Scarb.toml @@ -40,7 +40,7 @@ rpc_url = "http://localhost:5050/" # Default account for katana with seed = 0 account_address = "0x6162896d1d7ab204c7ccac6dd5f8e9e7c25ecd5ae4fcb4ad32e57786bb46e03" private_key = "0x1800000000300000180000000000030000000000003006001800006600" -# world_address = "0x5ffa124a5076e590a23e9d393ebe4fc7c66989c2cd228a46f38a9e2d48305a7" +world_address = "0x5ffa124a5076e590a23e9d393ebe4fc7c66989c2cd228a46f38a9e2d48305a7" [profile.release.tool.dojo] # for more info on how `merge-strategy` works see: From 1593ae8755e74ccbd85b39101ae8241dfef83755 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 24 Jul 2024 20:44:03 +0530 Subject: [PATCH 08/17] fix lints --- crates/dojo-world/src/manifest/manifest_test.rs | 2 +- crates/dojo-world/src/manifest/mod.rs | 4 +--- crates/dojo-world/src/migration/world.rs | 2 +- crates/dojo-world/src/migration/world_test.rs | 4 ++-- crates/sozo/ops/src/migration/migrate.rs | 11 +++++------ crates/sozo/ops/src/tests/migration.rs | 6 +++--- 6 files changed, 13 insertions(+), 16 deletions(-) diff --git a/crates/dojo-world/src/manifest/manifest_test.rs b/crates/dojo-world/src/manifest/manifest_test.rs index 4eb5920bfe..ae0e1ccabe 100644 --- a/crates/dojo-world/src/manifest/manifest_test.rs +++ b/crates/dojo-world/src/manifest/manifest_test.rs @@ -357,7 +357,7 @@ fn fetch_remote_manifest() { // compute diff from local and remote manifest - let diff = WorldDiff::compute(local_manifest, Some(remote_manifest), &"dojo-test").unwrap(); + let diff = WorldDiff::compute(local_manifest, Some(remote_manifest), "dojo-test").unwrap(); assert_eq!(diff.count_diffs(), 0, "there should not be any diff"); } diff --git a/crates/dojo-world/src/manifest/mod.rs b/crates/dojo-world/src/manifest/mod.rs index c5e295cca4..7c1e67e600 100644 --- a/crates/dojo-world/src/manifest/mod.rs +++ b/crates/dojo-world/src/manifest/mod.rs @@ -466,9 +466,7 @@ impl DeploymentMetadata { pub fn add_missing(&mut self, manifest: &BaseManifest) { for contract in manifest.contracts.iter() { let name = naming::get_tag_from_filename(&contract.manifest_name).unwrap(); - if !self.contracts.contains_key(&name) { - self.contracts.insert(name, true); - } + self.contracts.entry(name).or_insert(true); } } } diff --git a/crates/dojo-world/src/migration/world.rs b/crates/dojo-world/src/migration/world.rs index e66314c781..e744432f62 100644 --- a/crates/dojo-world/src/migration/world.rs +++ b/crates/dojo-world/src/migration/world.rs @@ -95,7 +95,7 @@ impl WorldDiff { }; let mut diff = WorldDiff { world, base, contracts, models }; - diff.update_order(&default_namespace)?; + diff.update_order(default_namespace)?; Ok(diff) } diff --git a/crates/dojo-world/src/migration/world_test.rs b/crates/dojo-world/src/migration/world_test.rs index 465e400741..c6a0c8b8fa 100644 --- a/crates/dojo-world/src/migration/world_test.rs +++ b/crates/dojo-world/src/migration/world_test.rs @@ -32,7 +32,7 @@ fn no_diff_when_local_and_remote_are_equal() { let mut remote: DeploymentManifest = local.clone().into(); remote.models = remote_models; - let diff = WorldDiff::compute(local, Some(remote), &"dojo-test").unwrap(); + let diff = WorldDiff::compute(local, Some(remote), "dojo-test").unwrap(); assert_eq!(diff.count_diffs(), 0); } @@ -121,7 +121,7 @@ fn diff_when_local_and_remote_are_different() { remote.models[1].inner.class_hash = 33_u32.into(); remote.contracts[0].inner.class_hash = felt!("0x1112"); - let diff = WorldDiff::compute(local, Some(remote), &"dojo-test").unwrap(); + let diff = WorldDiff::compute(local, Some(remote), "dojo-test").unwrap(); assert_eq!(diff.count_diffs(), 3); assert!(diff.models.iter().any(|m| m.tag == get_tag("dojo_mock", "model2"))); diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 546d100543..6d4c2a7abe 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -711,13 +711,13 @@ where match provider.get_class_hash_at(BlockId::Tag(BlockTag::Pending), contract_address).await { Ok(current_class_hash) if current_class_hash != contract.diff.local_class_hash => { - return format!("{}: Upgrade", contract.diff.tag); + format!("{}: Upgrade", contract.diff.tag) } Err(ProviderError::StarknetError(StarknetError::ContractNotFound)) => { - return format!("{}: Deploy", contract.diff.tag); + format!("{}: Deploy", contract.diff.tag) } - Ok(_) => return "Already Deployed".to_string(), - Err(_) => return format!("{}: Deploy", contract.diff.tag), + Ok(_) => "Already Deployed".to_string(), + Err(_) => format!("{}: Deploy", contract.diff.tag), } } else { format!("{}: Deploy", contract.diff.tag) @@ -907,8 +907,7 @@ fn calculate( } // given deployment_metadata.contracts return only the ones that are true - let contracts = - deployment_metadata.contracts.iter().filter(|(_, &v)| v == true).collect::>(); + let contracts = deployment_metadata.contracts.iter().filter(|(_, &v)| v).collect::>(); let mut work = vec![]; for contract in contracts { diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index 0b28203ad1..b4da74aa66 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -136,7 +136,7 @@ async fn metadata_calculated_properly() { manifest.merge(overlay_manifest); } - let world = WorldDiff::compute(manifest, None, &"dojo-test").unwrap(); + let world = WorldDiff::compute(manifest, None, "dojo-test").unwrap(); let migration = prepare_for_migration( None, @@ -175,7 +175,7 @@ async fn migration_with_correct_calldata_second_time_work_as_expected() { ) .unwrap(); - let world = WorldDiff::compute(manifest.clone(), None, &"dojo-test").unwrap(); + let world = WorldDiff::compute(manifest.clone(), None, "dojo-test").unwrap(); let migration = prepare_for_migration( None, @@ -242,7 +242,7 @@ async fn migration_from_remote() { ) .unwrap(); - let world = WorldDiff::compute(manifest, None, &"dojo-test").unwrap(); + let world = WorldDiff::compute(manifest, None, "dojo-test").unwrap(); let migration = prepare_for_migration( None, From f121e0ba1a9b79ff45438611f547a4ee7a02911c Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Thu, 25 Jul 2024 23:36:39 +0530 Subject: [PATCH 09/17] update implementation --- crates/dojo-world/src/manifest/mod.rs | 10 +-- crates/dojo-world/src/manifest/types.rs | 13 ++- crates/sozo/ops/src/migration/auto_auth.rs | 87 ++++++++++++------- crates/sozo/ops/src/migration/migrate.rs | 53 +++++++---- crates/sozo/ops/src/migration/mod.rs | 17 ++-- .../manifests/dev/deployment/metadata.toml | 5 -- 6 files changed, 118 insertions(+), 67 deletions(-) delete mode 100644 examples/spawn-and-move/manifests/dev/deployment/metadata.toml diff --git a/crates/dojo-world/src/manifest/mod.rs b/crates/dojo-world/src/manifest/mod.rs index 7c1e67e600..84fb2ee4d9 100644 --- a/crates/dojo-world/src/manifest/mod.rs +++ b/crates/dojo-world/src/manifest/mod.rs @@ -29,10 +29,10 @@ mod test; mod types; pub use types::{ - AbiFormat, BaseManifest, Class, ComputedValueEntrypoint, DeploymentManifest, - DeploymentMetadata, DojoContract, DojoModel, Manifest, ManifestMethods, Member, OverlayClass, - OverlayContract, OverlayDojoContract, OverlayDojoModel, OverlayManifest, WorldContract, - WorldMetadata, + AbiFormat, BaseManifest, Class, ComputedValueEntrypoint, ContractMetadata, DeploymentManifest, + DeploymentMetadata, DojoContract, DojoModel, Manifest, ManifestMethods, Member, Operation, + OverlayClass, OverlayContract, OverlayDojoContract, OverlayDojoModel, OverlayManifest, + WorldContract, WorldMetadata, }; pub const WORLD_CONTRACT_TAG: &str = "dojo-world"; @@ -466,7 +466,7 @@ impl DeploymentMetadata { pub fn add_missing(&mut self, manifest: &BaseManifest) { for contract in manifest.contracts.iter() { let name = naming::get_tag_from_filename(&contract.manifest_name).unwrap(); - self.contracts.entry(name).or_insert(true); + self.contracts.entry(name).or_insert(ContractMetadata::default()); } } } diff --git a/crates/dojo-world/src/manifest/types.rs b/crates/dojo-world/src/manifest/types.rs index 58500338fb..f55e44296e 100644 --- a/crates/dojo-world/src/manifest/types.rs +++ b/crates/dojo-world/src/manifest/types.rs @@ -37,12 +37,21 @@ pub struct DeploymentManifest { pub models: Vec>, } +pub type ContractMetadata = HashMap; // bool represents authorization has been done for that contract #[derive(Default, Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub struct DeploymentMetadata { - // manifest_name -> bool - pub contracts: HashMap, + pub world_metadata: bool, + // tag -> ContractMetadata + pub contracts: HashMap, +} + +#[derive(Default, Clone, Debug, Serialize, Deserialize)] +pub enum Operation { + #[default] + Grant, + Revoke, } #[derive(Default, Clone, Debug, Serialize, Deserialize)] diff --git a/crates/sozo/ops/src/migration/auto_auth.rs b/crates/sozo/ops/src/migration/auto_auth.rs index 10327f4b83..b1b721265c 100644 --- a/crates/sozo/ops/src/migration/auto_auth.rs +++ b/crates/sozo/ops/src/migration/auto_auth.rs @@ -1,26 +1,25 @@ -use std::str::FromStr; +use std::{collections::HashMap, str::FromStr}; use anyhow::Result; -use dojo_world::contracts::WorldContract; -use dojo_world::manifest::BaseManifest; +use dojo_world::manifest::ContractMetadata; use dojo_world::migration::TxnConfig; +use dojo_world::{contracts::WorldContract, manifest::Operation}; use scarb::core::Workspace; use scarb_ui::Ui; use starknet::accounts::ConnectedAccount; use super::ui::MigrationUi; -use crate::auth::{grant_writer, ResourceWriter}; +use crate::auth::{grant_writer, revoke_writer, ResourceWriter}; pub async fn auto_authorize( ws: &Workspace<'_>, world: &WorldContract, txn_config: &TxnConfig, - local_manifest: &BaseManifest, default_namespace: &str, - work: &Vec, + work: &Vec<(String, ContractMetadata)>, ) -> Result<()> where - A: ConnectedAccount + Sync + Send, + A: ConnectedAccount + Sync + Send + 'static, A::SignError: 'static, { let ui = ws.config().ui(); @@ -28,43 +27,69 @@ where ui.print(" "); ui.print_step(6, "🖋️", "Authorizing systems based on overlay..."); ui.print(" "); - let new_writers = create_writers(&ui, local_manifest, work)?; - grant_writer(&ui, world, new_writers, *txn_config, default_namespace).await + let (grant, revoke) = create_writers(&ui, work)?; + grant_writer(&ui, world, grant, *txn_config, default_namespace).await?; + revoke_writer(&ui, world, revoke, *txn_config, default_namespace).await?; + + Ok(()) } pub fn create_writers( ui: &Ui, - local_manifest: &BaseManifest, - tags: &Vec, -) -> Result> { - let mut res = vec![]; - let local_contracts = &local_manifest.contracts; + work: &Vec<(String, ContractMetadata)>, +) -> Result<(Vec, Vec)> { + let mut grant = HashMap::new(); + let mut revoke = HashMap::new(); // From all the contracts that were migrated successfully. - for tag in tags { - // Find that contract from local_manifest based on its tag. - let contract = - local_contracts.iter().find(|c| tag == &c.inner.tag).expect("unexpected tag found"); - - if !contract.inner.writes.is_empty() { - ui.print_sub(format!( - "Authorizing {} for resources: {:?}", - contract.inner.tag, contract.inner.writes - )); + for (tag, contract_metadata) in work { + // separate out the resources that are being granted and revoked. + // based on the Operation type in the contract_metadata. + for (resource, operation) in contract_metadata { + match operation { + Operation::Grant => { + let entry = grant.entry(tag).or_insert(vec![]); + entry.push(resource); + } + Operation::Revoke => { + let entry = revoke.entry(tag).or_insert(vec![]); + entry.push(resource); + } + } } + } - for tag_with_prefix in &contract.inner.writes { - let resource_type = if tag_with_prefix.contains(':') { - tag_with_prefix.to_string() + let mut grant_writer = vec![]; + for (tag, resources) in grant.iter() { + ui.print_sub(format!("Authorizing write access of {} for resources: {:?}", tag, resources)); + + for resource in resources { + let resource = if resource.contains(':') { + resource.to_string() } else { // Default to model if no prefix was given. - format!("m:{}", tag_with_prefix) + format!("m:{}", resource) }; + let resource = format!("{},{}", resource, tag); + grant_writer.push(ResourceWriter::from_str(&resource)?); + } + } + + let mut revoke_writer = vec![]; + for (tag, resources) in revoke.iter() { + ui.print_sub(format!("Revoking write access of {} for resources: {:?}", tag, resources)); - let resource = format!("{},{}", resource_type, tag); - res.push(ResourceWriter::from_str(&resource)?); + for resource in resources { + let resource = if resource.contains(':') { + resource.to_string() + } else { + // Default to model if no prefix was given. + format!("m:{}", resource) + }; + let resource = format!("{},{}", resource, tag); + revoke_writer.push(ResourceWriter::from_str(&resource)?); } } - Ok(res) + Ok((grant_writer, revoke_writer)) } diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 6d4c2a7abe..bf6b84b425 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::path::Path; use anyhow::{anyhow, bail, Context, Result}; @@ -9,9 +10,10 @@ use dojo_world::contracts::naming::{ }; use dojo_world::contracts::{cairo_utils, WorldContract}; use dojo_world::manifest::{ - AbiFormat, BaseManifest, Class, DeploymentManifest, DeploymentMetadata, DojoContract, - DojoModel, Manifest, ManifestMethods, WorldContract as ManifestWorldContract, WorldMetadata, - ABIS_DIR, BASE_DIR, DEPLOYMENT_DIR, MANIFESTS_DIR, + AbiFormat, BaseManifest, Class, ContractMetadata, DeploymentManifest, DeploymentMetadata, + DojoContract, DojoModel, Manifest, ManifestMethods, Operation, + WorldContract as ManifestWorldContract, WorldMetadata, ABIS_DIR, BASE_DIR, DEPLOYMENT_DIR, + MANIFESTS_DIR, }; use dojo_world::metadata::{dojo_metadata_from_workspace, ResourceMetadata}; use dojo_world::migration::class::ClassMigration; @@ -251,13 +253,13 @@ where /// into the Dojo resource registry. /// /// # Arguments -/// * `element_name` - fully qualified name of the element linked to the metadata -/// * `resource_id` - the id of the resource to create. -/// * `artifact` - the artifact to upload on IPFS. +/// * `ui` - The user interface object for displaying information +/// * `resource_id` - The id of the resource to create +/// * `metadata` - The ResourceMetadata object containing the metadata to upload /// /// # Returns -/// A [`ResourceData`] object to register in the Dojo resource register -/// on success. +/// A [`world::ResourceMetadata`] object to register in the Dojo resource register +/// on success, or an error if the upload fails. async fn upload_on_ipfs_and_create_resource( ui: &Ui, resource_id: Felt, @@ -874,7 +876,7 @@ pub fn update_deployment_metadata( manifest_dir: &Utf8PathBuf, local_manifest: &BaseManifest, migration_output: Option<&MigrationOutput>, -) -> Result> { +) -> Result> { let deployment_dir = manifest_dir.join(DEPLOYMENT_DIR); let deployment_metadata_path = deployment_dir.join("metadata").with_extension("toml"); @@ -885,7 +887,9 @@ pub fn update_deployment_metadata( }; deployment_metadata.add_missing(local_manifest); - let work = calculate(migration_output, &mut deployment_metadata); + + let work = calculate(migration_output, &mut deployment_metadata, &local_manifest); + deployment_metadata .write_to_path_toml(&deployment_metadata_path) .with_context(|| "Failed to write deployment metadata")?; @@ -896,22 +900,41 @@ pub fn update_deployment_metadata( fn calculate( migration_output: Option<&MigrationOutput>, deployment_metadata: &mut DeploymentMetadata, -) -> Vec { + local_manifest: &BaseManifest, +) -> Vec<(String, ContractMetadata)> { if let Some(migration_output) = migration_output { + if migration_output.world_tx_hash.is_some() { + deployment_metadata.world_metadata = true; + } + migration_output.contracts.iter().flatten().for_each(|contract_output| { let name = contract_output.tag.clone(); - // overwrite existing information, since we now need to do the migration - deployment_metadata.contracts.insert(name, true); + let writes = deployment_metadata + .contracts + .entry(name.clone()) + .or_insert_with(|| HashMap::default()); + + // since `name` is coming from `migration_output` its safe to unwrap + let contract = local_manifest.contracts.iter().find(|c| c.inner.tag == name).unwrap(); + + contract.inner.writes.iter().for_each(|i| { + writes.insert(i.to_string(), Operation::Grant); + }); }); } // given deployment_metadata.contracts return only the ones that are true - let contracts = deployment_metadata.contracts.iter().filter(|(_, &v)| v).collect::>(); + let contracts = deployment_metadata + .contracts + .clone() + .into_iter() + .filter(|(_, v)| v.keys().len() > 0) + .collect::)>>(); let mut work = vec![]; for contract in contracts { - work.push(contract.0.clone()); + work.push(contract); } work diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index 163ad2933a..ffdc6084d1 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -47,9 +47,7 @@ pub struct ContractMigrationOutput { pub base_class_hash: Felt, } -// TODO: when diff is zero we still need to run auto authorize // TODO: read deployment manifest to find diff in `writes` for auto authorize -// TODO: update method names to better reflect what they now do // TODO: add tests // TODO: general cleanup #[allow(clippy::too_many_arguments)] @@ -64,7 +62,7 @@ pub async fn migrate( skip_manifests: Option>, ) -> Result> where - A: ConnectedAccount + Sync + Send, + A: ConnectedAccount + Sync + Send + 'static, A::Provider: Send, A::SignError: 'static, { @@ -127,6 +125,9 @@ where } let strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; + // TODO: dry run can also show the diffs for things apart from world state + // what new authorizations would be granted, if ipfs data would change or not, + // etc... if dry_run { if total_diffs == 0 { return Ok(None); @@ -188,9 +189,7 @@ where let account = Arc::new(account); let world = WorldContract::new(strategy.world_address, account.clone()); - match auto_authorize(ws, &world, &txn_config, &local_manifest, &default_namespace, &work) - .await - { + match auto_authorize(ws, &world, &txn_config, &default_namespace, &work).await { Ok(()) => { let deployment_dir = manifest_dir.join(DEPLOYMENT_DIR); let deployment_metadata_path = @@ -200,11 +199,11 @@ where let mut deployment_metadata = DeploymentMetadata::load_from_path(&deployment_metadata_path)?; - work.iter().for_each(|tag| { - let contract = + work.iter().for_each(|(tag, _)| { + let contract_metadata = deployment_metadata.contracts.get_mut(tag).expect("unexpected tag found"); - *contract = false; + contract_metadata.clear(); }); deployment_metadata.write_to_path_toml(&deployment_metadata_path)?; diff --git a/examples/spawn-and-move/manifests/dev/deployment/metadata.toml b/examples/spawn-and-move/manifests/dev/deployment/metadata.toml deleted file mode 100644 index c3b7f4b53a..0000000000 --- a/examples/spawn-and-move/manifests/dev/deployment/metadata.toml +++ /dev/null @@ -1,5 +0,0 @@ -[contracts] -dojo_examples-mock_token = false -dojo_examples-others = false -dojo_examples-dungeon = false -dojo_examples-actions = false From 1994ba4515bc87252b9161010cde056da36fb20a Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Thu, 25 Jul 2024 23:46:02 +0530 Subject: [PATCH 10/17] update message --- crates/sozo/ops/src/migration/auto_auth.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/sozo/ops/src/migration/auto_auth.rs b/crates/sozo/ops/src/migration/auto_auth.rs index b1b721265c..d73d6af4c5 100644 --- a/crates/sozo/ops/src/migration/auto_auth.rs +++ b/crates/sozo/ops/src/migration/auto_auth.rs @@ -61,7 +61,7 @@ pub fn create_writers( let mut grant_writer = vec![]; for (tag, resources) in grant.iter() { - ui.print_sub(format!("Authorizing write access of {} for resources: {:?}", tag, resources)); + ui.print_sub(format!("Granting write access to {} for resources: {:?}", tag, resources)); for resource in resources { let resource = if resource.contains(':') { @@ -77,7 +77,7 @@ pub fn create_writers( let mut revoke_writer = vec![]; for (tag, resources) in revoke.iter() { - ui.print_sub(format!("Revoking write access of {} for resources: {:?}", tag, resources)); + ui.print_sub(format!("Revoking write access from {} for resources: {:?}", tag, resources)); for resource in resources { let resource = if resource.contains(':') { From 2b52edd5ee57e644e46ee0bc08d3ef36db0d4a98 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Tue, 30 Jul 2024 21:22:29 +0530 Subject: [PATCH 11/17] update implementation to read writes data while loading remote manifest --- bin/sozo/src/commands/auth.rs | 16 +- crates/dojo-world/src/manifest/mod.rs | 108 ++++++++++---- crates/dojo-world/src/manifest/types.rs | 23 +-- crates/dojo-world/src/migration/contract.rs | 2 + crates/dojo-world/src/migration/world.rs | 12 ++ crates/sozo/ops/src/auth.rs | 33 +++-- crates/sozo/ops/src/migration/auto_auth.rs | 73 +--------- crates/sozo/ops/src/migration/migrate.rs | 137 ++++++++++-------- crates/sozo/ops/src/migration/mod.rs | 44 ++---- .../spawn-and-move/overlays/dev/actions.toml | 2 +- 10 files changed, 230 insertions(+), 220 deletions(-) diff --git a/bin/sozo/src/commands/auth.rs b/bin/sozo/src/commands/auth.rs index 67ab7a5016..aa76060082 100644 --- a/bin/sozo/src/commands/auth.rs +++ b/bin/sozo/src/commands/auth.rs @@ -156,7 +156,7 @@ pub async fn grant( contracts=?models_contracts, "Granting Writer permissions." ); - auth::grant_writer(ui, &world, models_contracts, transaction.into(), default_namespace) + auth::grant_writer(ui, &world, &models_contracts, transaction.into(), default_namespace) .await } AuthKind::Owner { owners_resources } => { @@ -164,7 +164,7 @@ pub async fn grant( resources=?owners_resources, "Granting Owner permissions." ); - auth::grant_owner(ui, &world, owners_resources, transaction.into(), default_namespace) + auth::grant_owner(ui, &world, &owners_resources, transaction.into(), default_namespace) .await } } @@ -192,15 +192,21 @@ pub async fn revoke( contracts=?models_contracts, "Revoking Writer permissions." ); - auth::revoke_writer(ui, &world, models_contracts, transaction.into(), default_namespace) - .await + auth::revoke_writer( + ui, + &world, + &models_contracts, + transaction.into(), + default_namespace, + ) + .await } AuthKind::Owner { owners_resources } => { trace!( resources=?owners_resources, "Revoking Owner permissions." ); - auth::revoke_owner(ui, &world, owners_resources, transaction.into(), default_namespace) + auth::revoke_owner(ui, &world, &owners_resources, transaction.into(), default_namespace) .await } } diff --git a/crates/dojo-world/src/manifest/mod.rs b/crates/dojo-world/src/manifest/mod.rs index 84fb2ee4d9..b7a3e0c0a1 100644 --- a/crates/dojo-world/src/manifest/mod.rs +++ b/crates/dojo-world/src/manifest/mod.rs @@ -3,7 +3,7 @@ use std::path::PathBuf; use std::{fs, io}; use anyhow::Result; -use cainome::cairo_serde::{ByteArray, CairoSerde, Error as CainomeError}; +use cainome::cairo_serde::{ByteArray, CairoSerde, Error as CainomeError, Zeroable}; use camino::Utf8PathBuf; use serde::de::DeserializeOwned; use serde::Serialize; @@ -29,10 +29,9 @@ mod test; mod types; pub use types::{ - AbiFormat, BaseManifest, Class, ComputedValueEntrypoint, ContractMetadata, DeploymentManifest, - DeploymentMetadata, DojoContract, DojoModel, Manifest, ManifestMethods, Member, Operation, - OverlayClass, OverlayContract, OverlayDojoContract, OverlayDojoModel, OverlayManifest, - WorldContract, WorldMetadata, + AbiFormat, BaseManifest, Class, ComputedValueEntrypoint, DeploymentManifest, DojoContract, + DojoModel, Manifest, ManifestMethods, Member, OverlayClass, OverlayContract, + OverlayDojoContract, OverlayDojoModel, OverlayManifest, WorldContract, WorldMetadata, }; pub const WORLD_CONTRACT_TAG: &str = "dojo-world"; @@ -445,31 +444,22 @@ impl DeploymentManifest { } } -impl DeploymentMetadata { - pub fn load_from_path(path: &Utf8PathBuf) -> Result { - let manifest: Self = toml::from_str(&fs::read_to_string(path)?).unwrap(); +// impl DeploymentMetadata { +// pub fn load_from_path(path: &Utf8PathBuf) -> Result { +// let manifest: Self = toml::from_str(&fs::read_to_string(path)?).unwrap(); - Ok(manifest) - } +// Ok(manifest) +// } - pub fn write_to_path_toml(&self, path: &Utf8PathBuf) -> Result<()> { - fs::create_dir_all(path.parent().unwrap())?; - - let deployed_manifest = toml::to_string_pretty(&self)?; - fs::write(path, deployed_manifest)?; +// pub fn write_to_path_toml(&self, path: &Utf8PathBuf) -> Result<()> { +// fs::create_dir_all(path.parent().unwrap())?; - Ok(()) - } +// let deployed_manifest = toml::to_string_pretty(&self)?; +// fs::write(path, deployed_manifest)?; - // adds any missing contracts to the metadata - // this is required so we add any newly added contract to the metadata - pub fn add_missing(&mut self, manifest: &BaseManifest) { - for contract in manifest.contracts.iter() { - let name = naming::get_tag_from_filename(&contract.manifest_name).unwrap(); - self.contracts.entry(name).or_insert(ContractMetadata::default()); - } - } -} +// Ok(()) +// } +// } // TODO: currently implementing this method using trait is causing lifetime issue due to // `async_trait` macro which is hard to debug. So moved it as a async method on type itself. @@ -494,6 +484,7 @@ where let registered_models_event_name = starknet_keccak("ModelRegistered".as_bytes()); let contract_deployed_event_name = starknet_keccak("ContractDeployed".as_bytes()); let contract_upgraded_event_name = starknet_keccak("ContractUpgraded".as_bytes()); + let writer_updated_event_name = starknet_keccak("WriterUpdated".as_bytes()); let events = get_events( &provider, @@ -502,6 +493,7 @@ where registered_models_event_name, contract_deployed_event_name, contract_upgraded_event_name, + writer_updated_event_name, ]], ) .await?; @@ -509,6 +501,7 @@ where let mut registered_models_events = vec![]; let mut contract_deployed_events = vec![]; let mut contract_upgraded_events = vec![]; + let mut writer_updated_events = vec![]; for event in events { match event.keys.first() { @@ -521,12 +514,19 @@ where Some(event_name) if *event_name == contract_upgraded_event_name => { contract_upgraded_events.push(event) } + Some(event_name) if *event_name == writer_updated_event_name => { + writer_updated_events.push(event) + } _ => {} } } let models = parse_models_events(registered_models_events); - let mut contracts = parse_contracts_events(contract_deployed_events, contract_upgraded_events); + let mut contracts = parse_contracts_events( + contract_deployed_events, + contract_upgraded_events, + writer_updated_events, + ); for contract in &mut contracts { contract.manifest_name = naming::get_filename_from_tag(&contract.inner.tag); @@ -566,7 +566,54 @@ async fn get_events( fn parse_contracts_events( deployed: Vec, upgraded: Vec, + granted: Vec, ) -> Vec> { + fn retain_only_latest_grant_events(events: Vec) -> HashMap> { + // create a map with some extra data which will be flattened later + // system -> (block_num, (resource -> perm)) + let mut grants: HashMap)> = HashMap::new(); + events.into_iter().for_each(|event| { + let mut data = event.data.into_iter(); + let block_num = event.block_number; + let resource = data.next().expect("resource is missing from event"); + let contract = data.next().expect("contract is missing from event"); + let value = data.next().expect("value is missing from event"); + + let value = if value.is_zero() { false } else { true }; + + // Events that do not have a block number are ignored because we are unable to evaluate + // whether the events happened before or after the latest event that has been processed. + if let Some(num) = block_num { + grants + .entry(contract) + .and_modify(|(current_block, current_resource)| { + if *current_block < num { + *current_block = num; + current_resource.insert(resource, value); + } + }) + .or_insert((num, HashMap::from([(resource, value)]))); + } + }); + + // flatten out the map to remove block_number information and only include resources that are true + // i.e. system -> [resources] + let ret = grants + .into_iter() + .map(|(contract, (_, resources))| { + ( + contract, + resources + .into_iter() + .filter_map(|(resource, bool)| if bool { Some(resource) } else { None }) + .collect(), + ) + }) + .collect(); + // dbg!(&ret); + ret + } + fn retain_only_latest_upgrade_events(events: Vec) -> HashMap { // addr -> (block_num, class_hash) let mut upgrades: HashMap = HashMap::new(); @@ -597,6 +644,7 @@ fn parse_contracts_events( } let upgradeds = retain_only_latest_upgrade_events(upgraded); + let grants = retain_only_latest_grant_events(granted); deployed .into_iter() @@ -623,12 +671,18 @@ fn parse_contracts_events( class_hash = *upgrade; } + let mut writes = vec![]; + if let Some(contract) = grants.get(&address) { + writes.extend(contract.iter().map(|f| f.to_hex_string())); + } + Manifest::new( DojoContract { address: Some(address), class_hash, abi: None, tag: tag.clone(), + writes, ..Default::default() }, naming::get_filename_from_tag(&tag), diff --git a/crates/dojo-world/src/manifest/types.rs b/crates/dojo-world/src/manifest/types.rs index f55e44296e..a266d898af 100644 --- a/crates/dojo-world/src/manifest/types.rs +++ b/crates/dojo-world/src/manifest/types.rs @@ -1,4 +1,3 @@ -use std::collections::HashMap; use std::fs; use camino::Utf8PathBuf; @@ -33,26 +32,18 @@ pub struct BaseManifest { pub struct DeploymentManifest { pub world: Manifest, pub base: Manifest, + // NOTE: `writes` field in contracts is of String but we read the values which are resource hashes + // from the events, so needs to be handled accordingly pub contracts: Vec>, pub models: Vec>, } -pub type ContractMetadata = HashMap; // bool represents authorization has been done for that contract -#[derive(Default, Clone, Debug, Serialize, Deserialize)] -#[cfg_attr(test, derive(PartialEq))] -pub struct DeploymentMetadata { - pub world_metadata: bool, - // tag -> ContractMetadata - pub contracts: HashMap, -} - -#[derive(Default, Clone, Debug, Serialize, Deserialize)] -pub enum Operation { - #[default] - Grant, - Revoke, -} +// #[derive(Default, Clone, Debug, Serialize, Deserialize)] +// #[cfg_attr(test, derive(PartialEq))] +// pub struct DeploymentMetadata { +// pub world_metadata: bool, +// } #[derive(Default, Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] diff --git a/crates/dojo-world/src/migration/contract.rs b/crates/dojo-world/src/migration/contract.rs index 2339454f1a..7cbc78eba5 100644 --- a/crates/dojo-world/src/migration/contract.rs +++ b/crates/dojo-world/src/migration/contract.rs @@ -18,6 +18,8 @@ pub struct ContractDiff { pub base_class_hash: Felt, pub remote_class_hash: Option, pub init_calldata: Vec, + pub local_writes: Vec, + pub remote_writes: Vec, } impl StateDiff for ContractDiff { diff --git a/crates/dojo-world/src/migration/world.rs b/crates/dojo-world/src/migration/world.rs index e744432f62..46d76f5569 100644 --- a/crates/dojo-world/src/migration/world.rs +++ b/crates/dojo-world/src/migration/world.rs @@ -74,6 +74,16 @@ impl WorldDiff { .map(|r| *r.inner.class_hash()) }), init_calldata: contract.inner.init_calldata.clone(), + local_writes: contract.inner.writes.clone(), + remote_writes: remote + .as_ref() + .and_then(|m| { + m.contracts + .iter() + .find(|r| r.inner.class_hash() == contract.inner.class_hash()) + .map(|r| r.inner.writes.clone()) + }) + .unwrap_or_default(), } }) .collect::>(); @@ -92,6 +102,8 @@ impl WorldDiff { base_class_hash: *local.base.inner.class_hash(), remote_class_hash: remote.map(|m| *m.world.inner.class_hash()), init_calldata: vec![], + local_writes: vec![], + remote_writes: vec![], }; let mut diff = WorldDiff { world, base, contracts, models }; diff --git a/crates/sozo/ops/src/auth.rs b/crates/sozo/ops/src/auth.rs index 64b3bdadbb..3d6984d5de 100644 --- a/crates/sozo/ops/src/auth.rs +++ b/crates/sozo/ops/src/auth.rs @@ -20,6 +20,8 @@ pub enum ResourceType { Contract(String), Namespace(String), Model(String), + // this can be a selector for any other resource type + Selector(Felt), } impl FromStr for ResourceType { @@ -35,10 +37,14 @@ impl FromStr for ResourceType { Some(("namespace", name)) | Some(("ns", name)) => { ResourceType::Namespace(name.to_string()) } - _ => anyhow::bail!( + Some(("selector", name)) | Some(("s", name)) => { + ResourceType::Selector(Felt::from_str(name)?) + } + _ => anyhow::bail!(format!( "Resource is expected to be in the format `resource_type:resource_name`: `sozo \ - auth grant owner resource_type:resource_name,0x1234`" - ), + auth grant owner resource_type:resource_name,0x1234`, Found: {}.", + s + )), }; Ok(resource) } @@ -101,7 +107,7 @@ impl FromStr for ResourceOwner { pub async fn grant_writer<'a, A>( ui: &'a Ui, world: &WorldContract, - new_writers: Vec, + new_writers: &Vec, txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -115,7 +121,7 @@ where let resource_selector = get_resource_selector(ui, world, &new_writer.resource, default_namespace).await?; let contract_address = - utils::get_contract_address(world, new_writer.tag_or_address).await?; + utils::get_contract_address(world, new_writer.tag_or_address.clone()).await?; calls.push(world.grant_writer_getcall(&resource_selector, &contract_address.into())); } @@ -143,7 +149,7 @@ where pub async fn grant_owner( ui: &Ui, world: &WorldContract, - new_owners: Vec, + new_owners: &Vec, txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -154,7 +160,8 @@ where for new_owner in new_owners { let resource_selector = - get_resource_selector(ui, world, &new_owner.resource, default_namespace).await?; + get_resource_selector(ui, world, &new_owner.resource.clone(), default_namespace) + .await?; calls.push(world.grant_owner_getcall(&new_owner.owner.into(), &resource_selector)); } @@ -180,7 +187,7 @@ where pub async fn revoke_writer( ui: &Ui, world: &WorldContract, - new_writers: Vec, + new_writers: &Vec, txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -193,7 +200,7 @@ where let resource_selector = get_resource_selector(ui, world, &new_writer.resource, default_namespace).await?; let contract_address = - utils::get_contract_address(world, new_writer.tag_or_address).await?; + utils::get_contract_address(world, new_writer.tag_or_address.clone()).await?; calls.push(world.revoke_writer_getcall(&resource_selector, &contract_address.into())); } @@ -221,7 +228,7 @@ where pub async fn revoke_owner( ui: &Ui, world: &WorldContract, - new_owners: Vec, + new_owners: &Vec, txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -232,7 +239,8 @@ where for new_owner in new_owners { let resource_selector = - get_resource_selector(ui, world, &new_owner.resource, default_namespace).await?; + get_resource_selector(ui, world, &new_owner.resource.clone(), default_namespace) + .await?; calls.push(world.revoke_owner_getcall(&new_owner.owner.into(), &resource_selector)); } @@ -255,7 +263,7 @@ where Ok(()) } -async fn get_resource_selector( +pub async fn get_resource_selector( ui: &Ui, world: &WorldContract, resource: &ResourceType, @@ -297,6 +305,7 @@ where compute_selector_from_tag(&tag) } ResourceType::Namespace(name) => compute_bytearray_hash(name), + ResourceType::Selector(selector) => *selector, }; Ok(resource_selector) diff --git a/crates/sozo/ops/src/migration/auto_auth.rs b/crates/sozo/ops/src/migration/auto_auth.rs index d73d6af4c5..08878328cd 100644 --- a/crates/sozo/ops/src/migration/auto_auth.rs +++ b/crates/sozo/ops/src/migration/auto_auth.rs @@ -1,14 +1,9 @@ -use std::{collections::HashMap, str::FromStr}; - use anyhow::Result; -use dojo_world::manifest::ContractMetadata; +use dojo_world::contracts::WorldContract; use dojo_world::migration::TxnConfig; -use dojo_world::{contracts::WorldContract, manifest::Operation}; use scarb::core::Workspace; -use scarb_ui::Ui; use starknet::accounts::ConnectedAccount; -use super::ui::MigrationUi; use crate::auth::{grant_writer, revoke_writer, ResourceWriter}; pub async fn auto_authorize( @@ -16,7 +11,8 @@ pub async fn auto_authorize( world: &WorldContract, txn_config: &TxnConfig, default_namespace: &str, - work: &Vec<(String, ContractMetadata)>, + grant: &Vec, + revoke: &Vec, ) -> Result<()> where A: ConnectedAccount + Sync + Send + 'static, @@ -25,71 +21,8 @@ where let ui = ws.config().ui(); ui.print(" "); - ui.print_step(6, "🖋️", "Authorizing systems based on overlay..."); - ui.print(" "); - let (grant, revoke) = create_writers(&ui, work)?; grant_writer(&ui, world, grant, *txn_config, default_namespace).await?; revoke_writer(&ui, world, revoke, *txn_config, default_namespace).await?; Ok(()) } - -pub fn create_writers( - ui: &Ui, - work: &Vec<(String, ContractMetadata)>, -) -> Result<(Vec, Vec)> { - let mut grant = HashMap::new(); - let mut revoke = HashMap::new(); - - // From all the contracts that were migrated successfully. - for (tag, contract_metadata) in work { - // separate out the resources that are being granted and revoked. - // based on the Operation type in the contract_metadata. - for (resource, operation) in contract_metadata { - match operation { - Operation::Grant => { - let entry = grant.entry(tag).or_insert(vec![]); - entry.push(resource); - } - Operation::Revoke => { - let entry = revoke.entry(tag).or_insert(vec![]); - entry.push(resource); - } - } - } - } - - let mut grant_writer = vec![]; - for (tag, resources) in grant.iter() { - ui.print_sub(format!("Granting write access to {} for resources: {:?}", tag, resources)); - - for resource in resources { - let resource = if resource.contains(':') { - resource.to_string() - } else { - // Default to model if no prefix was given. - format!("m:{}", resource) - }; - let resource = format!("{},{}", resource, tag); - grant_writer.push(ResourceWriter::from_str(&resource)?); - } - } - - let mut revoke_writer = vec![]; - for (tag, resources) in revoke.iter() { - ui.print_sub(format!("Revoking write access from {} for resources: {:?}", tag, resources)); - - for resource in resources { - let resource = if resource.contains(':') { - resource.to_string() - } else { - // Default to model if no prefix was given. - format!("m:{}", resource) - }; - let resource = format!("{},{}", resource, tag); - revoke_writer.push(ResourceWriter::from_str(&resource)?); - } - } - - Ok((grant_writer, revoke_writer)) -} diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index bf6b84b425..1a17e7ef7f 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -1,5 +1,6 @@ use std::collections::HashMap; use std::path::Path; +use std::str::FromStr; use anyhow::{anyhow, bail, Context, Result}; use cainome::cairo_serde::ByteArray; @@ -10,10 +11,9 @@ use dojo_world::contracts::naming::{ }; use dojo_world::contracts::{cairo_utils, WorldContract}; use dojo_world::manifest::{ - AbiFormat, BaseManifest, Class, ContractMetadata, DeploymentManifest, DeploymentMetadata, - DojoContract, DojoModel, Manifest, ManifestMethods, Operation, - WorldContract as ManifestWorldContract, WorldMetadata, ABIS_DIR, BASE_DIR, DEPLOYMENT_DIR, - MANIFESTS_DIR, + AbiFormat, BaseManifest, Class, DeploymentManifest, DojoContract, DojoModel, Manifest, + ManifestMethods, WorldContract as ManifestWorldContract, WorldMetadata, ABIS_DIR, BASE_DIR, + DEPLOYMENT_DIR, MANIFESTS_DIR, }; use dojo_world::metadata::{dojo_metadata_from_workspace, ResourceMetadata}; use dojo_world::migration::class::ClassMigration; @@ -28,7 +28,7 @@ use futures::future; use itertools::Itertools; use scarb::core::Workspace; use scarb_ui::Ui; -use starknet::accounts::ConnectedAccount; +use starknet::accounts::{Account, ConnectedAccount}; use starknet::core::types::{ BlockId, BlockTag, Felt, FunctionCall, InvokeTransactionResult, StarknetError, }; @@ -38,6 +38,8 @@ use starknet::core::utils::{ use starknet::providers::{Provider, ProviderError}; use tokio::fs; +use crate::auth::{get_resource_selector, ResourceType, ResourceWriter}; + use super::ui::{bold_message, italic_message, MigrationUi}; use super::{ ContractDeploymentOutput, ContractMigrationOutput, ContractUpgradeOutput, MigrationOutput, @@ -872,72 +874,85 @@ pub async fn update_manifests_and_abis( Ok(()) } -pub fn update_deployment_metadata( - manifest_dir: &Utf8PathBuf, - local_manifest: &BaseManifest, +pub async fn find_authorization_diff( + ui: &Ui, + world: &WorldContract, + diff: &WorldDiff, migration_output: Option<&MigrationOutput>, -) -> Result> { - let deployment_dir = manifest_dir.join(DEPLOYMENT_DIR); - let deployment_metadata_path = deployment_dir.join("metadata").with_extension("toml"); - - let mut deployment_metadata = if deployment_metadata_path.exists() { - DeploymentMetadata::load_from_path(&deployment_metadata_path)? - } else { - DeploymentMetadata::default() - }; - - deployment_metadata.add_missing(local_manifest); - - let work = calculate(migration_output, &mut deployment_metadata, &local_manifest); - - deployment_metadata - .write_to_path_toml(&deployment_metadata_path) - .with_context(|| "Failed to write deployment metadata")?; - - Ok(work) -} + default_namespace: &str, +) -> Result<(Vec, Vec)> +where + A: ConnectedAccount + Sync + Send, + ::SignError: 'static, +{ + let mut grant = vec![]; + let mut revoke = vec![]; + + for c in &diff.contracts { + // remote is none meants its not deployed. + // now it could have been deployed during this run + // which can be checked from migration_output + // if c.remote_class_hash.is_none() { + // continue; + // } + + let mut local = HashMap::new(); + for write in &c.local_writes { + let write = + if write.contains(':') { write.to_string() } else { format!("m:{}", write) }; + + let resource = ResourceType::from_str(&write)?; + let selector = get_resource_selector(ui, world, &resource, default_namespace) + .await + .with_context(|| format!("Failed to get selector for {}", write))?; -fn calculate( - migration_output: Option<&MigrationOutput>, - deployment_metadata: &mut DeploymentMetadata, - local_manifest: &BaseManifest, -) -> Vec<(String, ContractMetadata)> { - if let Some(migration_output) = migration_output { - if migration_output.world_tx_hash.is_some() { - deployment_metadata.world_metadata = true; + let resource_writer = ResourceWriter::from_str(&format!("{},{}", write, c.tag))?; + local.insert(selector, resource_writer); } - migration_output.contracts.iter().flatten().for_each(|contract_output| { - let name = contract_output.tag.clone(); - - let writes = deployment_metadata - .contracts - .entry(name.clone()) - .or_insert_with(|| HashMap::default()); - - // since `name` is coming from `migration_output` its safe to unwrap - let contract = local_manifest.contracts.iter().find(|c| c.inner.tag == name).unwrap(); + for write in &c.remote_writes { + // This value is fetched from onchain events, so we get them as felts + let selector = Felt::from_str(write).with_context(|| "Expected write to be a felt")?; + if let Some(_) = local.remove(&selector) { + // do nothing for one which are already onchain + } else { + // revoke ones that are not present in local + assert!(Felt::from_str(write).is_ok()); + revoke.push(ResourceWriter::from_str(&format!("s:{},{}", write, c.tag))?); + } + } - contract.inner.writes.iter().for_each(|i| { - writes.insert(i.to_string(), Operation::Grant); - }); + // apply remaining + local.iter().for_each(|(_, resource_writer)| { + grant.push(resource_writer.clone()); }); - } - // given deployment_metadata.contracts return only the ones that are true - let contracts = deployment_metadata - .contracts - .clone() - .into_iter() - .filter(|(_, v)| v.keys().len() > 0) - .collect::)>>(); + let contract_grants: Vec<_> = + grant.iter().filter(|rw| rw.tag_or_address == c.tag).cloned().collect(); + if !contract_grants.is_empty() { + ui.print_sub(format!( + "Granting write access to {} for resources: {:?}", + c.tag, + contract_grants.iter().map(|rw| rw.resource.clone()).collect::>() + )); + } - let mut work = vec![]; - for contract in contracts { - work.push(contract); + let contract_revokes: Vec<_> = + revoke.iter().filter(|rw| rw.tag_or_address == c.tag).cloned().collect(); + if !contract_revokes.is_empty() { + ui.print_sub(format!( + "Revoking write access to {} for resources: {:?}", + c.tag, + contract_revokes.iter().map(|rw| rw.resource.clone()).collect::>() + )); + } + + if !contract_grants.is_empty() || !contract_revokes.is_empty() { + ui.print(" "); + } } - work + Ok((grant, revoke)) } // copy abi files from `base/abi` to `deployment/abi` and update abi path in diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index ffdc6084d1..8277a30924 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -2,9 +2,7 @@ use std::sync::Arc; use anyhow::{anyhow, bail, Result}; use dojo_world::contracts::WorldContract; -use dojo_world::manifest::{ - DeploymentMetadata, BASE_DIR, DEPLOYMENT_DIR, MANIFESTS_DIR, OVERLAYS_DIR, -}; +use dojo_world::manifest::{BASE_DIR, MANIFESTS_DIR, OVERLAYS_DIR}; use dojo_world::metadata::get_default_namespace_from_ws; use dojo_world::migration::world::WorldDiff; use dojo_world::migration::{DeployOutput, TxnConfig, UpgradeOutput}; @@ -20,11 +18,10 @@ mod ui; mod utils; pub use self::auto_auth::auto_authorize; -use self::migrate::update_manifests_and_abis; pub use self::migrate::{ - apply_diff, execute_strategy, prepare_migration, print_strategy, update_deployment_metadata, - upload_metadata, + apply_diff, execute_strategy, prepare_migration, print_strategy, upload_metadata, }; +use self::migrate::{find_authorization_diff, update_manifests_and_abis}; use self::ui::MigrationUi; #[derive(Debug, Default, Clone)] @@ -124,7 +121,7 @@ where ui.print("\n✨ No diffs found. Remote World is already up to date!"); } - let strategy = prepare_migration(&target_dir, diff, name, world_address, &ui)?; + let strategy = prepare_migration(&target_dir, diff.clone(), name, world_address, &ui)?; // TODO: dry run can also show the diffs for things apart from world state // what new authorizations would be granted, if ipfs data would change or not, // etc... @@ -183,31 +180,22 @@ where ) .await?; - let work = - update_deployment_metadata(&manifest_dir, &local_manifest, migration_output.as_ref())?; - let account = Arc::new(account); let world = WorldContract::new(strategy.world_address, account.clone()); - match auto_authorize(ws, &world, &txn_config, &default_namespace, &work).await { - Ok(()) => { - let deployment_dir = manifest_dir.join(DEPLOYMENT_DIR); - let deployment_metadata_path = - deployment_dir.join("metadata").with_extension("toml"); - - // read back metadata file and update it - let mut deployment_metadata = - DeploymentMetadata::load_from_path(&deployment_metadata_path)?; - - work.iter().for_each(|(tag, _)| { - let contract_metadata = - deployment_metadata.contracts.get_mut(tag).expect("unexpected tag found"); - - contract_metadata.clear(); - }); - - deployment_metadata.write_to_path_toml(&deployment_metadata_path)?; + ui.print(" "); + ui.print_step(6, "🖋️", "Authorizing systems based on overlay..."); + let (grant, revoke) = find_authorization_diff( + &ui, + &world, + &diff, + migration_output.as_ref(), + &default_namespace, + ) + .await?; + match auto_authorize(ws, &world, &txn_config, &default_namespace, &grant, &revoke).await { + Ok(()) => { ui.print_sub("Auto authorize completed successfully"); } Err(e) => { diff --git a/examples/spawn-and-move/overlays/dev/actions.toml b/examples/spawn-and-move/overlays/dev/actions.toml index 31849b7894..72ac4c1e00 100644 --- a/examples/spawn-and-move/overlays/dev/actions.toml +++ b/examples/spawn-and-move/overlays/dev/actions.toml @@ -1,2 +1,2 @@ tag = "dojo_examples-actions" -writes = ["dojo_examples-Moves", "dojo_examples-Position"] +writes = [ "dojo_examples-Moves", "dojo_examples-Position" ] From a6e4bb533f5a319cd60451454c01feb412ba32b4 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 31 Jul 2024 12:30:05 +0530 Subject: [PATCH 12/17] use migration_output to migrate when contract was actually deployed --- crates/dojo-world/src/manifest/mod.rs | 12 ++++------ crates/dojo-world/src/manifest/types.rs | 4 ++-- crates/sozo/ops/src/migration/migrate.rs | 30 ++++++++++++++++-------- crates/sozo/ops/src/migration/mod.rs | 3 --- crates/sozo/ops/src/tests/migration.rs | 18 +++++++------- 5 files changed, 37 insertions(+), 30 deletions(-) diff --git a/crates/dojo-world/src/manifest/mod.rs b/crates/dojo-world/src/manifest/mod.rs index b7a3e0c0a1..10b0f119dd 100644 --- a/crates/dojo-world/src/manifest/mod.rs +++ b/crates/dojo-world/src/manifest/mod.rs @@ -579,7 +579,7 @@ fn parse_contracts_events( let contract = data.next().expect("contract is missing from event"); let value = data.next().expect("value is missing from event"); - let value = if value.is_zero() { false } else { true }; + let value = !value.is_zero(); // Events that do not have a block number are ignored because we are unable to evaluate // whether the events happened before or after the latest event that has been processed. @@ -596,9 +596,9 @@ fn parse_contracts_events( } }); - // flatten out the map to remove block_number information and only include resources that are true - // i.e. system -> [resources] - let ret = grants + // flatten out the map to remove block_number information and only include resources that + // are true i.e. system -> [resources] + grants .into_iter() .map(|(contract, (_, resources))| { ( @@ -609,9 +609,7 @@ fn parse_contracts_events( .collect(), ) }) - .collect(); - // dbg!(&ret); - ret + .collect() } fn retain_only_latest_upgrade_events(events: Vec) -> HashMap { diff --git a/crates/dojo-world/src/manifest/types.rs b/crates/dojo-world/src/manifest/types.rs index a266d898af..1165262660 100644 --- a/crates/dojo-world/src/manifest/types.rs +++ b/crates/dojo-world/src/manifest/types.rs @@ -32,8 +32,8 @@ pub struct BaseManifest { pub struct DeploymentManifest { pub world: Manifest, pub base: Manifest, - // NOTE: `writes` field in contracts is of String but we read the values which are resource hashes - // from the events, so needs to be handled accordingly + // NOTE: `writes` field in contracts is of String but we read the values which are resource + // hashes from the events, so needs to be handled accordingly pub contracts: Vec>, pub models: Vec>, } diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 1a17e7ef7f..3ea56f1a3d 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::Path; use std::str::FromStr; @@ -38,12 +38,11 @@ use starknet::core::utils::{ use starknet::providers::{Provider, ProviderError}; use tokio::fs; -use crate::auth::{get_resource_selector, ResourceType, ResourceWriter}; - use super::ui::{bold_message, italic_message, MigrationUi}; use super::{ ContractDeploymentOutput, ContractMigrationOutput, ContractUpgradeOutput, MigrationOutput, }; +use crate::auth::{get_resource_selector, ResourceType, ResourceWriter}; pub fn prepare_migration( target_dir: &Utf8PathBuf, @@ -888,13 +887,24 @@ where let mut grant = vec![]; let mut revoke = vec![]; + let mut recently_migrated = HashSet::new(); + + if let Some(migration_output) = migration_output { + recently_migrated = migration_output + .contracts + .iter() + .flatten() + .map(|m| m.tag.clone()) + .collect::>() + } + for c in &diff.contracts { - // remote is none meants its not deployed. - // now it could have been deployed during this run - // which can be checked from migration_output - // if c.remote_class_hash.is_none() { - // continue; - // } + // remote is none meants it was not previously deployed. + // but if it didn't get deployed even during this run we should skip migration for it + if c.remote_class_hash.is_none() && !recently_migrated.contains(&c.tag) { + ui.print_sub(format!("Skipping migration for contract {}", c.tag)); + continue; + } let mut local = HashMap::new(); for write in &c.local_writes { @@ -913,7 +923,7 @@ where for write in &c.remote_writes { // This value is fetched from onchain events, so we get them as felts let selector = Felt::from_str(write).with_context(|| "Expected write to be a felt")?; - if let Some(_) = local.remove(&selector) { + if local.remove(&selector).is_some() { // do nothing for one which are already onchain } else { // revoke ones that are not present in local diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index 8277a30924..f7b2fb9521 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -44,9 +44,6 @@ pub struct ContractMigrationOutput { pub base_class_hash: Felt, } -// TODO: read deployment manifest to find diff in `writes` for auto authorize -// TODO: add tests -// TODO: general cleanup #[allow(clippy::too_many_arguments)] pub async fn migrate( ws: &Workspace<'_>, diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index b4da74aa66..e5c18a044f 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -105,14 +105,16 @@ async fn migrate_with_small_fee_multiplier_will_fail() { let account = sequencer.account(0); - assert!(execute_strategy( - &ws, - &migration, - &account, - TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, - ) - .await - .is_err()); + assert!( + execute_strategy( + &ws, + &migration, + &account, + TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, + ) + .await + .is_err() + ); } #[tokio::test] From 30d200d49c27320dc71489823e3b87922b4624ef Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 31 Jul 2024 13:37:00 +0530 Subject: [PATCH 13/17] fix tests --- crates/dojo-test-utils/src/migration.rs | 5 ++- .../dojo-world/src/manifest/manifest_test.rs | 6 +-- crates/dojo-world/src/manifest/types.rs | 7 ---- crates/dojo-world/src/migration/strategy.rs | 1 - crates/sozo/ops/src/auth.rs | 8 ++-- crates/sozo/ops/src/migration/mod.rs | 5 ++- crates/sozo/ops/src/tests/auth.rs | 12 +++--- crates/sozo/ops/src/tests/migration.rs | 41 ++++++++++--------- crates/sozo/ops/src/tests/setup.rs | 6 ++- crates/torii/core/src/sql_test.rs | 9 ++-- crates/torii/graphql/src/tests/mod.rs | 3 +- 11 files changed, 52 insertions(+), 51 deletions(-) diff --git a/crates/dojo-test-utils/src/migration.rs b/crates/dojo-test-utils/src/migration.rs index c8cb6110a7..37c9a8e550 100644 --- a/crates/dojo-test-utils/src/migration.rs +++ b/crates/dojo-test-utils/src/migration.rs @@ -45,7 +45,7 @@ pub fn prepare_migration_with_world_and_seed( world_address: Option, seed: &str, default_namespace: &str, -) -> Result { +) -> Result<(MigrationStrategy, WorldDiff)> { // In testing, profile name is always dev. let profile_name = "dev"; @@ -63,5 +63,6 @@ pub fn prepare_migration_with_world_and_seed( let world = WorldDiff::compute(manifest, None, default_namespace)?; let seed = cairo_short_string_to_felt(seed).unwrap(); - prepare_for_migration(world_address, seed, &target_dir, world) + let strat = prepare_for_migration(world_address, seed, &target_dir, world.clone())?; + Ok((strat, world)) } diff --git a/crates/dojo-world/src/manifest/manifest_test.rs b/crates/dojo-world/src/manifest/manifest_test.rs index ae0e1ccabe..fea5baabc7 100644 --- a/crates/dojo-world/src/manifest/manifest_test.rs +++ b/crates/dojo-world/src/manifest/manifest_test.rs @@ -136,7 +136,7 @@ fn parse_deployed_contracts_events_without_upgrade() { build_deploy_event(vec![felt!("0x0"), felt!("0x3"), felt!("0x789")], "ns3", "c3"), ]; - let actual_contracts = parse_contracts_events(events, vec![]); + let actual_contracts = parse_contracts_events(events, vec![], vec![]); assert_eq!(actual_contracts, expected_contracts); } @@ -213,7 +213,7 @@ fn parse_deployed_contracts_events_with_upgrade() { }, ]; - let actual_contracts = parse_contracts_events(deployed_events, upgrade_events); + let actual_contracts = parse_contracts_events(deployed_events, upgrade_events, vec![]); similar_asserts::assert_eq!(actual_contracts, expected_contracts); } @@ -293,7 +293,7 @@ fn events_without_block_number_arent_parsed() { }, ]; - let actual_contracts = parse_contracts_events(deployed_events, upgrade_events); + let actual_contracts = parse_contracts_events(deployed_events, upgrade_events, vec![]); similar_asserts::assert_eq!(actual_contracts, expected_contracts); } diff --git a/crates/dojo-world/src/manifest/types.rs b/crates/dojo-world/src/manifest/types.rs index 1165262660..5de7cdbf65 100644 --- a/crates/dojo-world/src/manifest/types.rs +++ b/crates/dojo-world/src/manifest/types.rs @@ -38,13 +38,6 @@ pub struct DeploymentManifest { pub models: Vec>, } -// bool represents authorization has been done for that contract -// #[derive(Default, Clone, Debug, Serialize, Deserialize)] -// #[cfg_attr(test, derive(PartialEq))] -// pub struct DeploymentMetadata { -// pub world_metadata: bool, -// } - #[derive(Default, Clone, Debug, Serialize, Deserialize)] #[cfg_attr(test, derive(PartialEq))] pub struct OverlayManifest { diff --git a/crates/dojo-world/src/migration/strategy.rs b/crates/dojo-world/src/migration/strategy.rs index 46fe09bc34..fcce6e6f82 100644 --- a/crates/dojo-world/src/migration/strategy.rs +++ b/crates/dojo-world/src/migration/strategy.rs @@ -139,7 +139,6 @@ pub fn prepare_for_migration( } // If world address is not provided, then we expect the world to be migrated. - // TODO: see if there are any case where world_address is not provided and `world` is none let world_address = world_address.unwrap_or_else(|| world.as_ref().unwrap().contract_address); let mut migration = diff --git a/crates/sozo/ops/src/auth.rs b/crates/sozo/ops/src/auth.rs index 3d6984d5de..aa5239ab7f 100644 --- a/crates/sozo/ops/src/auth.rs +++ b/crates/sozo/ops/src/auth.rs @@ -107,7 +107,7 @@ impl FromStr for ResourceOwner { pub async fn grant_writer<'a, A>( ui: &'a Ui, world: &WorldContract, - new_writers: &Vec, + new_writers: &[ResourceWriter], txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -149,7 +149,7 @@ where pub async fn grant_owner( ui: &Ui, world: &WorldContract, - new_owners: &Vec, + new_owners: &[ResourceOwner], txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -187,7 +187,7 @@ where pub async fn revoke_writer( ui: &Ui, world: &WorldContract, - new_writers: &Vec, + new_writers: &[ResourceWriter], txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> @@ -228,7 +228,7 @@ where pub async fn revoke_owner( ui: &Ui, world: &WorldContract, - new_owners: &Vec, + new_owners: &[ResourceOwner], txn_config: TxnConfig, default_namespace: &str, ) -> Result<()> diff --git a/crates/sozo/ops/src/migration/mod.rs b/crates/sozo/ops/src/migration/mod.rs index f7b2fb9521..c0efa102a6 100644 --- a/crates/sozo/ops/src/migration/mod.rs +++ b/crates/sozo/ops/src/migration/mod.rs @@ -18,10 +18,11 @@ mod ui; mod utils; pub use self::auto_auth::auto_authorize; +use self::migrate::update_manifests_and_abis; pub use self::migrate::{ - apply_diff, execute_strategy, prepare_migration, print_strategy, upload_metadata, + apply_diff, execute_strategy, find_authorization_diff, prepare_migration, print_strategy, + upload_metadata, }; -use self::migrate::{find_authorization_diff, update_manifests_and_abis}; use self::ui::MigrationUi; #[derive(Debug, Default, Clone)] diff --git a/crates/sozo/ops/src/tests/auth.rs b/crates/sozo/ops/src/tests/auth.rs index ef467cdf05..b3a4006844 100644 --- a/crates/sozo/ops/src/tests/auth.rs +++ b/crates/sozo/ops/src/tests/auth.rs @@ -44,7 +44,7 @@ async fn auth_grant_writer_ok() { auth::grant_writer( &Ui::new(Verbosity::Normal, OutputFormat::Text), &world, - vec![moves_mc, position_mc], + &[moves_mc, position_mc], TxnConfig { wait: true, ..Default::default() }, DEFAULT_NAMESPACE, ) @@ -86,7 +86,7 @@ async fn auth_revoke_writer_ok() { auth::grant_writer( &Ui::new(Verbosity::Normal, OutputFormat::Text), &world, - vec![moves_mc.clone(), position_mc.clone()], + &[moves_mc.clone(), position_mc.clone()], TxnConfig { wait: true, ..Default::default() }, DEFAULT_NAMESPACE, ) @@ -100,7 +100,7 @@ async fn auth_revoke_writer_ok() { auth::revoke_writer( &Ui::new(Verbosity::Normal, OutputFormat::Text), &world, - vec![moves_mc, position_mc], + &[moves_mc, position_mc], TxnConfig { wait: true, ..Default::default() }, DEFAULT_NAMESPACE, ) @@ -141,7 +141,7 @@ async fn auth_grant_owner_ok() { auth::grant_owner( &Ui::new(Verbosity::Normal, OutputFormat::Text), &world, - vec![moves, position], + &[moves, position], TxnConfig { wait: true, ..Default::default() }, DEFAULT_NAMESPACE, ) @@ -181,7 +181,7 @@ async fn auth_revoke_owner_ok() { auth::grant_owner( &Ui::new(Verbosity::Normal, OutputFormat::Text), &world, - vec![moves.clone(), position.clone()], + &[moves.clone(), position.clone()], TxnConfig { wait: true, ..Default::default() }, DEFAULT_NAMESPACE, ) @@ -193,7 +193,7 @@ async fn auth_revoke_owner_ok() { auth::revoke_owner( &Ui::new(Verbosity::Normal, OutputFormat::Text), &world, - vec![moves, position], + &[moves, position], TxnConfig { wait: true, ..Default::default() }, DEFAULT_NAMESPACE, ) diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index e5c18a044f..5aa47f9ec5 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -27,7 +27,7 @@ use starknet::providers::JsonRpcClient; use super::setup; use crate::migration::{ - auto_authorize, execute_strategy, update_deployment_metadata, upload_metadata, + auto_authorize, execute_strategy, find_authorization_diff, upload_metadata, }; use crate::utils::get_contract_address_from_reader; @@ -60,7 +60,7 @@ async fn migrate_with_auto_mine() { let config = setup::load_config(); let ws = setup::setup_ws(&config); - let migration = setup::setup_migration(&config).unwrap(); + let (migration, _) = setup::setup_migration(&config).unwrap(); let sequencer = KatanaRunner::new().expect("Fail to start runner"); @@ -75,7 +75,7 @@ async fn migrate_with_block_time() { let config = setup::load_config(); let ws = setup::setup_ws(&config); - let migration = setup::setup_migration(&config).unwrap(); + let (migration, _) = setup::setup_migration(&config).unwrap(); let sequencer = KatanaRunner::new_with_config(KatanaRunnerConfig { block_time: Some(1000), @@ -95,7 +95,7 @@ async fn migrate_with_small_fee_multiplier_will_fail() { let config = setup::load_config(); let ws = setup::setup_ws(&config); - let migration = setup::setup_migration(&config).unwrap(); + let (migration, _) = setup::setup_migration(&config).unwrap(); let sequencer = KatanaRunner::new_with_config(KatanaRunnerConfig { disable_fee: true, @@ -105,16 +105,14 @@ async fn migrate_with_small_fee_multiplier_will_fail() { let account = sequencer.account(0); - assert!( - execute_strategy( - &ws, - &migration, - &account, - TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, - ) - .await - .is_err() - ); + assert!(execute_strategy( + &ws, + &migration, + &account, + TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, + ) + .await + .is_err()); } #[tokio::test] @@ -277,7 +275,7 @@ async fn migrate_with_metadata() { let config = setup::load_config(); let ws = setup::setup_ws(&config); - let migration = setup::setup_migration(&config).unwrap(); + let (migration, _) = setup::setup_migration(&config).unwrap(); let sequencer = KatanaRunner::new().expect("Fail to start runner"); @@ -350,7 +348,7 @@ async fn migrate_with_auto_authorize() { let config = setup::load_config(); let ws = setup::setup_ws(&config); - let migration = setup::setup_migration(&config).unwrap(); + let (migration, diff) = setup::setup_migration(&config).unwrap(); let manifest_base = config.manifest_path().parent().unwrap(); let mut manifest = @@ -375,10 +373,13 @@ async fn migrate_with_auto_authorize() { let world_address = migration.world_address; let world = WorldContract::new(world_address, account); - let work = - update_deployment_metadata(&manifest_base.to_path_buf(), &manifest, Some(&output)).unwrap(); let default_namespace = get_default_namespace_from_ws(&ws).unwrap(); - let res = auto_authorize(&ws, &world, &txn_config, &manifest, &default_namespace, &work).await; + let (grant, revoke) = + find_authorization_diff(&config.ui(), &world, &diff, Some(&output), &default_namespace) + .await + .unwrap(); + + let res = auto_authorize(&ws, &world, &txn_config, &default_namespace, &grant, &revoke).await; assert!(res.is_ok()); let provider = sequencer.provider(); @@ -411,7 +412,7 @@ async fn migration_with_mismatching_world_address_and_seed() { let default_namespace = get_default_namespace_from_ws(&ws).unwrap(); - let strategy = prepare_migration_with_world_and_seed( + let (strategy, _) = prepare_migration_with_world_and_seed( base_dir, target_dir, Some(Felt::ONE), diff --git a/crates/sozo/ops/src/tests/setup.rs b/crates/sozo/ops/src/tests/setup.rs index 0433b0a995..7018cb47ec 100644 --- a/crates/sozo/ops/src/tests/setup.rs +++ b/crates/sozo/ops/src/tests/setup.rs @@ -4,6 +4,7 @@ use dojo_test_utils::migration::prepare_migration_with_world_and_seed; use dojo_world::contracts::world::WorldContract; use dojo_world::metadata::get_default_namespace_from_ws; use dojo_world::migration::strategy::MigrationStrategy; +use dojo_world::migration::world::WorldDiff; use dojo_world::migration::TxnConfig; use katana_runner::KatanaRunner; use scarb::compiler::Profile; @@ -49,7 +50,7 @@ pub fn setup_ws(config: &Config) -> Workspace<'_> { /// # Returns /// /// A [`MigrationStrategy`] to execute to migrate the full spawn-and-moves project. -pub fn setup_migration(config: &Config) -> Result { +pub fn setup_migration(config: &Config) -> Result<(MigrationStrategy, WorldDiff)> { let ws = setup_ws(config); let manifest_path = config.manifest_path(); @@ -83,7 +84,7 @@ pub async fn setup( let config = load_config(); let ws = setup_ws(&config); - let migration = setup_migration(&config)?; + let (migration, _) = setup_migration(&config)?; let mut account = sequencer.account(0); account.set_block_id(BlockId::Tag(BlockTag::Pending)); @@ -95,6 +96,7 @@ pub async fn setup( TxnConfig { wait: true, ..Default::default() }, ) .await?; + // TODO: do we need to do authorization in setup? let world = WorldContract::new(output.world_address, account) .with_block(BlockId::Tag(BlockTag::Pending)); diff --git a/crates/torii/core/src/sql_test.rs b/crates/torii/core/src/sql_test.rs index f3f61bd2bf..963ba1731f 100644 --- a/crates/torii/core/src/sql_test.rs +++ b/crates/torii/core/src/sql_test.rs @@ -72,7 +72,7 @@ async fn test_load_from_remote() { &ws, None, sequencer.url().to_string(), - &account, + account, "dojo_examples", false, TxnConfig::init_wait(), @@ -82,6 +82,7 @@ async fn test_load_from_remote() { .unwrap() .unwrap(); + let account = sequencer.account(0); // spawn let tx = &account .execute_v1(vec![Call { @@ -204,7 +205,7 @@ async fn test_load_from_remote_del() { &ws, None, sequencer.url().to_string(), - &account, + account, "dojo_examples", false, TxnConfig::init_wait(), @@ -214,6 +215,7 @@ async fn test_load_from_remote_del() { .unwrap() .unwrap(); + let account = sequencer.account(0); // spawn account .execute_v1(vec![Call { @@ -316,7 +318,7 @@ async fn test_get_entity_keys() { &ws, None, sequencer.url().to_string(), - &account, + account, "dojo_examples", false, TxnConfig::init_wait(), @@ -326,6 +328,7 @@ async fn test_get_entity_keys() { .unwrap() .unwrap(); + let account = sequencer.account(0); // spawn account .execute_v1(vec![Call { diff --git a/crates/torii/graphql/src/tests/mod.rs b/crates/torii/graphql/src/tests/mod.rs index 9006a712ea..b2eb69adb8 100644 --- a/crates/torii/graphql/src/tests/mod.rs +++ b/crates/torii/graphql/src/tests/mod.rs @@ -285,7 +285,7 @@ pub async fn spinup_types_test() -> Result { &ws, None, sequencer.url().to_string(), - &account, + account, "types_test", false, TxnConfig::init_wait(), @@ -295,6 +295,7 @@ pub async fn spinup_types_test() -> Result { .unwrap() .unwrap(); + let account = sequencer.account(0); // Execute `create` and insert 11 records into storage let records_contract = migration_output .contracts From 3ba48596e44858e4a156bf39731af2abd37411ee Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 31 Jul 2024 13:53:51 +0530 Subject: [PATCH 14/17] make suggested changes --- crates/sozo/ops/src/auth.rs | 9 +- crates/sozo/ops/src/execute.rs | 2 +- crates/sozo/ops/src/migration/auto_auth.rs | 4 +- crates/sozo/ops/src/migration/migrate.rs | 1 + crates/sozo/ops/src/tests/call.rs | 102 ++++++++++----------- crates/sozo/ops/src/tests/utils.rs | 5 +- crates/sozo/ops/src/utils.rs | 6 +- 7 files changed, 59 insertions(+), 70 deletions(-) diff --git a/crates/sozo/ops/src/auth.rs b/crates/sozo/ops/src/auth.rs index aa5239ab7f..6559f7b754 100644 --- a/crates/sozo/ops/src/auth.rs +++ b/crates/sozo/ops/src/auth.rs @@ -121,7 +121,7 @@ where let resource_selector = get_resource_selector(ui, world, &new_writer.resource, default_namespace).await?; let contract_address = - utils::get_contract_address(world, new_writer.tag_or_address.clone()).await?; + utils::get_contract_address(world, &new_writer.tag_or_address).await?; calls.push(world.grant_writer_getcall(&resource_selector, &contract_address.into())); } @@ -160,8 +160,7 @@ where for new_owner in new_owners { let resource_selector = - get_resource_selector(ui, world, &new_owner.resource.clone(), default_namespace) - .await?; + get_resource_selector(ui, world, &new_owner.resource, default_namespace).await?; calls.push(world.grant_owner_getcall(&new_owner.owner.into(), &resource_selector)); } @@ -200,7 +199,7 @@ where let resource_selector = get_resource_selector(ui, world, &new_writer.resource, default_namespace).await?; let contract_address = - utils::get_contract_address(world, new_writer.tag_or_address.clone()).await?; + utils::get_contract_address(world, &new_writer.tag_or_address).await?; calls.push(world.revoke_writer_getcall(&resource_selector, &contract_address.into())); } @@ -283,7 +282,7 @@ where } else { ensure_namespace(tag_or_address, default_namespace) }; - utils::get_contract_address(world, tag_or_address).await? + utils::get_contract_address(world, &tag_or_address).await? } ResourceType::Model(tag_or_name) => { // TODO: Is some models have version 0 (using the name of the struct instead of the diff --git a/crates/sozo/ops/src/execute.rs b/crates/sozo/ops/src/execute.rs index 24dd6c6ee2..ce91bd9d1c 100644 --- a/crates/sozo/ops/src/execute.rs +++ b/crates/sozo/ops/src/execute.rs @@ -20,7 +20,7 @@ pub async fn execute( where A: ConnectedAccount + Sync + Send + 'static, { - let contract_address = utils::get_contract_address(world, tag_or_address).await?; + let contract_address = utils::get_contract_address(world, &tag_or_address).await?; let res = world .account .execute_v1(vec![Call { diff --git a/crates/sozo/ops/src/migration/auto_auth.rs b/crates/sozo/ops/src/migration/auto_auth.rs index 08878328cd..ae64a67e6f 100644 --- a/crates/sozo/ops/src/migration/auto_auth.rs +++ b/crates/sozo/ops/src/migration/auto_auth.rs @@ -11,8 +11,8 @@ pub async fn auto_authorize( world: &WorldContract, txn_config: &TxnConfig, default_namespace: &str, - grant: &Vec, - revoke: &Vec, + grant: &[ResourceWriter], + revoke: &[ResourceWriter], ) -> Result<()> where A: ConnectedAccount + Sync + Send + 'static, diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 3ea56f1a3d..91613c4aa4 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -873,6 +873,7 @@ pub async fn update_manifests_and_abis( Ok(()) } +// For now we juust handle writers, handling of owners might be added in the future pub async fn find_authorization_diff( ui: &Ui, world: &WorldContract, diff --git a/crates/sozo/ops/src/tests/call.rs b/crates/sozo/ops/src/tests/call.rs index 8265a04d42..f14053afa0 100644 --- a/crates/sozo/ops/src/tests/call.rs +++ b/crates/sozo/ops/src/tests/call.rs @@ -23,17 +23,15 @@ async fn call_with_bad_address() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!( - call::call( - world_reader, - "0xBadCoffeeBadCode".to_string(), - ENTRYPOINT.to_string(), - vec![Felt::ZERO, Felt::ZERO], - None - ) - .await - .is_err() - ); + assert!(call::call( + world_reader, + "0xBadCoffeeBadCode".to_string(), + ENTRYPOINT.to_string(), + vec![Felt::ZERO, Felt::ZERO], + None + ) + .await + .is_err()); } #[tokio::test] @@ -44,17 +42,15 @@ async fn call_with_bad_name() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!( - call::call( - world_reader, - "BadName".to_string(), - ENTRYPOINT.to_string(), - vec![Felt::ZERO, Felt::ZERO], - None - ) - .await - .is_err() - ); + assert!(call::call( + world_reader, + "BadName".to_string(), + ENTRYPOINT.to_string(), + vec![Felt::ZERO, Felt::ZERO], + None + ) + .await + .is_err()); } #[tokio::test] @@ -65,17 +61,15 @@ async fn call_with_bad_entrypoint() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!( - call::call( - world_reader, - CONTRACT_TAG.to_string(), - "BadEntryPoint".to_string(), - vec![Felt::ZERO, Felt::ZERO], - None - ) - .await - .is_err() - ); + assert!(call::call( + world_reader, + CONTRACT_TAG.to_string(), + "BadEntryPoint".to_string(), + vec![Felt::ZERO, Felt::ZERO], + None + ) + .await + .is_err()); } #[tokio::test] @@ -86,17 +80,15 @@ async fn call_with_bad_calldata() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!( - call::call( - world_reader, - CONTRACT_TAG.to_string(), - ENTRYPOINT.to_string(), - vec![Felt::ZERO], - None - ) - .await - .is_err() - ); + assert!(call::call( + world_reader, + CONTRACT_TAG.to_string(), + ENTRYPOINT.to_string(), + vec![Felt::ZERO], + None + ) + .await + .is_err()); } #[tokio::test] @@ -124,19 +116,17 @@ async fn call_with_contract_address() { let contract_address = utils::get_contract_address::< SingleOwnerAccount, LocalWallet>, - >(&world, CONTRACT_TAG.to_string()) + >(&world, CONTRACT_TAG) .await .unwrap(); - assert!( - call::call( - world_reader, - format!("{:#x}", contract_address), - ENTRYPOINT.to_string(), - vec![], - None, - ) - .await - .is_ok() - ); + assert!(call::call( + world_reader, + format!("{:#x}", contract_address), + ENTRYPOINT.to_string(), + vec![], + None, + ) + .await + .is_ok()); } diff --git a/crates/sozo/ops/src/tests/utils.rs b/crates/sozo/ops/src/tests/utils.rs index 24585a9616..bd8c4c5568 100644 --- a/crates/sozo/ops/src/tests/utils.rs +++ b/crates/sozo/ops/src/tests/utils.rs @@ -15,8 +15,7 @@ async fn get_contract_address_from_world() { let world = setup::setup(&sequencer).await.unwrap(); - let contract_address = - utils::get_contract_address(&world, ACTION_CONTRACT_TAG.to_string()).await.unwrap(); + let contract_address = utils::get_contract_address(&world, ACTION_CONTRACT_TAG).await.unwrap(); assert!(contract_address != Felt::ZERO); } @@ -28,7 +27,7 @@ async fn get_contract_address_from_string() { let account = sequencer.account(0); let world = WorldContract::new(Felt::ZERO, account); - let contract_address = utils::get_contract_address(&world, "0x1234".to_string()).await.unwrap(); + let contract_address = utils::get_contract_address(&world, "0x1234").await.unwrap(); assert_eq!(contract_address, Felt::from_hex("0x1234").unwrap()); } diff --git a/crates/sozo/ops/src/utils.rs b/crates/sozo/ops/src/utils.rs index 0b6cdd70c4..83bdf458a2 100644 --- a/crates/sozo/ops/src/utils.rs +++ b/crates/sozo/ops/src/utils.rs @@ -23,14 +23,14 @@ use starknet::providers::Provider; /// A [`Felt`] with the address of the contract on success. pub async fn get_contract_address( world: &WorldContract, - tag_or_address: String, + tag_or_address: &str, ) -> Result { if tag_or_address.starts_with("0x") { - Felt::from_hex(&tag_or_address).map_err(anyhow::Error::from) + Felt::from_hex(tag_or_address).map_err(anyhow::Error::from) } else { let contract_class_hash = world.base().call().await?; Ok(starknet::core::utils::get_contract_address( - generate_salt(&get_name_from_tag(&tag_or_address)), + generate_salt(&get_name_from_tag(tag_or_address)), contract_class_hash.into(), &[], world.address, From 2420ac401f60091e48c7db931cb0db486f736a75 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 31 Jul 2024 14:08:34 +0530 Subject: [PATCH 15/17] fix formatting --- crates/sozo/ops/src/tests/call.rs | 100 ++++++++++++++----------- crates/sozo/ops/src/tests/migration.rs | 18 +++-- 2 files changed, 65 insertions(+), 53 deletions(-) diff --git a/crates/sozo/ops/src/tests/call.rs b/crates/sozo/ops/src/tests/call.rs index f14053afa0..a45162985d 100644 --- a/crates/sozo/ops/src/tests/call.rs +++ b/crates/sozo/ops/src/tests/call.rs @@ -23,15 +23,17 @@ async fn call_with_bad_address() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!(call::call( - world_reader, - "0xBadCoffeeBadCode".to_string(), - ENTRYPOINT.to_string(), - vec![Felt::ZERO, Felt::ZERO], - None - ) - .await - .is_err()); + assert!( + call::call( + world_reader, + "0xBadCoffeeBadCode".to_string(), + ENTRYPOINT.to_string(), + vec![Felt::ZERO, Felt::ZERO], + None + ) + .await + .is_err() + ); } #[tokio::test] @@ -42,15 +44,17 @@ async fn call_with_bad_name() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!(call::call( - world_reader, - "BadName".to_string(), - ENTRYPOINT.to_string(), - vec![Felt::ZERO, Felt::ZERO], - None - ) - .await - .is_err()); + assert!( + call::call( + world_reader, + "BadName".to_string(), + ENTRYPOINT.to_string(), + vec![Felt::ZERO, Felt::ZERO], + None + ) + .await + .is_err() + ); } #[tokio::test] @@ -61,15 +65,17 @@ async fn call_with_bad_entrypoint() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!(call::call( - world_reader, - CONTRACT_TAG.to_string(), - "BadEntryPoint".to_string(), - vec![Felt::ZERO, Felt::ZERO], - None - ) - .await - .is_err()); + assert!( + call::call( + world_reader, + CONTRACT_TAG.to_string(), + "BadEntryPoint".to_string(), + vec![Felt::ZERO, Felt::ZERO], + None + ) + .await + .is_err() + ); } #[tokio::test] @@ -80,15 +86,17 @@ async fn call_with_bad_calldata() { let provider = sequencer.provider(); let world_reader = WorldContractReader::new(world.address, provider); - assert!(call::call( - world_reader, - CONTRACT_TAG.to_string(), - ENTRYPOINT.to_string(), - vec![Felt::ZERO], - None - ) - .await - .is_err()); + assert!( + call::call( + world_reader, + CONTRACT_TAG.to_string(), + ENTRYPOINT.to_string(), + vec![Felt::ZERO], + None + ) + .await + .is_err() + ); } #[tokio::test] @@ -120,13 +128,15 @@ async fn call_with_contract_address() { .await .unwrap(); - assert!(call::call( - world_reader, - format!("{:#x}", contract_address), - ENTRYPOINT.to_string(), - vec![], - None, - ) - .await - .is_ok()); + assert!( + call::call( + world_reader, + format!("{:#x}", contract_address), + ENTRYPOINT.to_string(), + vec![], + None, + ) + .await + .is_ok() + ); } diff --git a/crates/sozo/ops/src/tests/migration.rs b/crates/sozo/ops/src/tests/migration.rs index 5aa47f9ec5..8665b4866e 100644 --- a/crates/sozo/ops/src/tests/migration.rs +++ b/crates/sozo/ops/src/tests/migration.rs @@ -105,14 +105,16 @@ async fn migrate_with_small_fee_multiplier_will_fail() { let account = sequencer.account(0); - assert!(execute_strategy( - &ws, - &migration, - &account, - TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, - ) - .await - .is_err()); + assert!( + execute_strategy( + &ws, + &migration, + &account, + TxnConfig { fee_estimate_multiplier: Some(0.2f64), ..Default::default() }, + ) + .await + .is_err() + ); } #[tokio::test] From ccd9a599ffdae463e5076ce090bb74edab0258b5 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 31 Jul 2024 15:56:13 +0530 Subject: [PATCH 16/17] map selector resource type to other resource type where possible --- crates/sozo/ops/src/migration/migrate.rs | 43 +++++++++++++- crates/sozo/ops/src/migration/utils.rs | 71 +++++++++++++++++++++++- 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 5f8e529d64..99d8f955f4 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -39,6 +39,7 @@ use starknet::providers::{Provider, ProviderError}; use tokio::fs; use super::ui::{bold_message, italic_message, MigrationUi}; +use super::utils::generate_resource_map; use super::{ ContractDeploymentOutput, ContractMigrationOutput, ContractUpgradeOutput, MigrationOutput, }; @@ -900,6 +901,12 @@ where .collect::>() } + // Generate a map of `Felt` (resource selector) -> `ResourceType` that are available locally + // so we can check if the resource being revoked is known locally. + // + // if the selector is not found in the map we just print its selector + let resource_map = generate_resource_map(ui, world, diff).await?; + for c in &diff.contracts { // remote is none meants it was not previously deployed. // but if it didn't get deployed even during this run we should skip migration for it @@ -945,7 +952,22 @@ where ui.print_sub(format!( "Granting write access to {} for resources: {:?}", c.tag, - contract_grants.iter().map(|rw| rw.resource.clone()).collect::>() + contract_grants + .iter() + .map(|rw| { + let resource = &rw.resource; + match resource { + ResourceType::Selector(s) => { + if let Some(r) = resource_map.get(&s.to_hex_string()) { + r.clone() + } else { + resource.clone() + } + } + _ => resource.clone(), + } + }) + .collect::>() )); } @@ -955,7 +977,24 @@ where ui.print_sub(format!( "Revoking write access to {} for resources: {:?}", c.tag, - contract_revokes.iter().map(|rw| rw.resource.clone()).collect::>() + contract_revokes + .iter() + .map(|rw| { + let resource = &rw.resource; + match resource { + // Replace selector with appropriate resource type if present in + // resource_map + ResourceType::Selector(s) => { + if let Some(r) = resource_map.get(&s.to_hex_string()) { + r.clone() + } else { + resource.clone() + } + } + _ => resource.clone(), + } + }) + .collect::>() )); } diff --git a/crates/sozo/ops/src/migration/utils.rs b/crates/sozo/ops/src/migration/utils.rs index dc91f26c1f..4e2aedeff6 100644 --- a/crates/sozo/ops/src/migration/utils.rs +++ b/crates/sozo/ops/src/migration/utils.rs @@ -1,13 +1,20 @@ -use anyhow::{anyhow, Result}; +use std::collections::HashMap; + +use anyhow::{anyhow, Context, Result}; use camino::Utf8PathBuf; +use dojo_world::contracts::naming::get_namespace_from_tag; +use dojo_world::contracts::WorldContract; use dojo_world::manifest::{ AbstractManifestError, BaseManifest, DeploymentManifest, OverlayManifest, }; +use dojo_world::migration::world::WorldDiff; +use itertools::Itertools; use scarb_ui::Ui; -use starknet::accounts::ConnectedAccount; +use starknet::accounts::{Account, ConnectedAccount}; use starknet::core::types::Felt; use super::ui::MigrationUi; +use crate::auth::{get_resource_selector, ResourceType}; /// Loads: /// - `BaseManifest` from filesystem @@ -63,3 +70,63 @@ where Ok((local_manifest, remote_manifest)) } + +pub async fn generate_resource_map( + ui: &Ui, + world: &WorldContract, + diff: &WorldDiff, +) -> Result> +where + A: ConnectedAccount + Sync + Send, + ::SignError: 'static, +{ + let mut resource_map = HashMap::new(); + + for contract in diff.contracts.iter() { + let resource = ResourceType::Contract(contract.tag.clone()); + // we know the tag already contains the namespace + let default_namespace = get_namespace_from_tag(&contract.tag); + let selector = + get_resource_selector(ui, world, &resource, &default_namespace).await.with_context( + || format!("Failed to get resource selector for contract: {}", contract.tag), + )?; + + resource_map.insert(selector.to_hex_string(), resource); + } + + for model in diff.models.iter() { + let resource = ResourceType::Model(model.tag.clone()); + // we know the tag already contains the namespace + let default_namespace = get_namespace_from_tag(&model.tag); + let selector = get_resource_selector(ui, world, &resource, &default_namespace) + .await + .with_context(|| format!("Failed to get resource selector for model: {}", model.tag))?; + + resource_map.insert(selector.to_hex_string(), resource); + } + + // Collect all the namespaces from the contracts and models + let namespaces = { + let mut namespaces = + diff.models.iter().map(|m| get_namespace_from_tag(&m.tag)).collect::>(); + + namespaces.extend( + diff.contracts.iter().map(|c| get_namespace_from_tag(&c.tag)).collect::>(), + ); + + // remove duplicates + namespaces.into_iter().unique().collect::>() + }; + + for namespace in &namespaces { + let resource = ResourceType::Namespace(namespace.clone()); + let selector = + get_resource_selector(ui, world, &resource, "").await.with_context(|| { + format!("Failed to get resource selector for namespace: {}", namespace) + })?; + + resource_map.insert(selector.to_hex_string(), resource); + } + + Ok(resource_map) +} From 0aa5d3ee8ab446676ad0dc31b6019a81d3a2d274 Mon Sep 17 00:00:00 2001 From: lambda-0x <0xlambda@protonmail.com> Date: Wed, 31 Jul 2024 18:13:39 +0530 Subject: [PATCH 17/17] make code more idiomatic --- crates/sozo/ops/src/migration/migrate.rs | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/sozo/ops/src/migration/migrate.rs b/crates/sozo/ops/src/migration/migrate.rs index 99d8f955f4..a5fa60ab2d 100644 --- a/crates/sozo/ops/src/migration/migrate.rs +++ b/crates/sozo/ops/src/migration/migrate.rs @@ -957,13 +957,12 @@ where .map(|rw| { let resource = &rw.resource; match resource { - ResourceType::Selector(s) => { - if let Some(r) = resource_map.get(&s.to_hex_string()) { - r.clone() - } else { - resource.clone() - } - } + // Replace selector with appropriate resource type if present in + // resource_map + ResourceType::Selector(s) => resource_map + .get(&s.to_hex_string()) + .cloned() + .unwrap_or_else(|| rw.resource.clone()), _ => resource.clone(), } }) @@ -984,13 +983,10 @@ where match resource { // Replace selector with appropriate resource type if present in // resource_map - ResourceType::Selector(s) => { - if let Some(r) = resource_map.get(&s.to_hex_string()) { - r.clone() - } else { - resource.clone() - } - } + ResourceType::Selector(s) => resource_map + .get(&s.to_hex_string()) + .cloned() + .unwrap_or_else(|| rw.resource.clone()), _ => resource.clone(), } })