From 121b413299b483a35b41f0264e050dd584e59169 Mon Sep 17 00:00:00 2001 From: Alexander Cyon Date: Thu, 23 Jan 2025 08:26:02 +0100 Subject: [PATCH] Polish --- .../src/unsecurified_entity.rs | 2 +- ...on_os_apply_security_shield_interaction.rs | 5 +- crates/transaction/manifests/src/lib.rs | 2 + .../src/manifest_builder_from_manifest.rs | 26 +++ .../manifests_security_shield.rs | 148 +----------------- .../src/manifests_security_shield/mod.rs | 6 + .../top_up_access_controller_xrd_vault.rs | 133 ++++++++++++++++ 7 files changed, 170 insertions(+), 152 deletions(-) create mode 100644 crates/transaction/manifests/src/manifest_builder_from_manifest.rs rename crates/transaction/manifests/src/{ => manifests_security_shield}/manifests_security_shield.rs (65%) create mode 100644 crates/transaction/manifests/src/manifests_security_shield/mod.rs create mode 100644 crates/transaction/manifests/src/manifests_security_shield/top_up_access_controller_xrd_vault.rs diff --git a/crates/profile/models/supporting-types/src/unsecurified_entity.rs b/crates/profile/models/supporting-types/src/unsecurified_entity.rs index f8389e135..5ba614284 100644 --- a/crates/profile/models/supporting-types/src/unsecurified_entity.rs +++ b/crates/profile/models/supporting-types/src/unsecurified_entity.rs @@ -57,7 +57,7 @@ where unsecured_entity_control, )), EntitySecurityState::Securified { .. } => { - Err(CommonError::AESDecryptionFailed) + Err(CommonError::SecurityStateSecurifiedButExpectedUnsecurified) } } } diff --git a/crates/system/os/factors/src/apply_security_shield/sargon_os_apply_security_shield_interaction.rs b/crates/system/os/factors/src/apply_security_shield/sargon_os_apply_security_shield_interaction.rs index a0d98d0b8..30e6d19e2 100644 --- a/crates/system/os/factors/src/apply_security_shield/sargon_os_apply_security_shield_interaction.rs +++ b/crates/system/os/factors/src/apply_security_shield/sargon_os_apply_security_shield_interaction.rs @@ -28,12 +28,9 @@ impl OsApplySecurityShieldInteraction for SargonOS { .unsecurified_erased() .iter() .map(|e| { - let provisional = e.entity.get_provisional().expect("Entity should have a provisional config set since we applied shield above"); let derived = provisional.as_factor_instances_derived().expect("Should have derived factors"); - - let input = TransactionManifestApplySecurityShieldUnsecurifiedInput::new(derived.clone()); - + let input = TransactionManifestApplySecurityShieldUnsecurifiedInput::new(derived.clone()); TransactionManifest::apply_security_shield_for_unsecurified_entity( e, input, diff --git a/crates/transaction/manifests/src/lib.rs b/crates/transaction/manifests/src/lib.rs index 13493d2cb..45ff0b5a8 100644 --- a/crates/transaction/manifests/src/lib.rs +++ b/crates/transaction/manifests/src/lib.rs @@ -4,6 +4,7 @@ mod delete_account; mod high_level; mod manifest_account_locker; mod manifest_assets_transfers; +mod manifest_builder_from_manifest; mod manifests; mod manifests_create_tokens; mod manifests_security_shield; @@ -18,6 +19,7 @@ pub mod prelude { pub use crate::high_level::*; pub use crate::manifest_account_locker::*; pub use crate::manifest_assets_transfers::*; + pub use crate::manifest_builder_from_manifest::*; pub use crate::manifests::*; pub use crate::manifests_create_tokens::*; pub use crate::manifests_security_shield::*; diff --git a/crates/transaction/manifests/src/manifest_builder_from_manifest.rs b/crates/transaction/manifests/src/manifest_builder_from_manifest.rs new file mode 100644 index 000000000..27c286068 --- /dev/null +++ b/crates/transaction/manifests/src/manifest_builder_from_manifest.rs @@ -0,0 +1,26 @@ +use radix_transactions::prelude::ManifestBuilder; + +use crate::prelude::*; + +pub trait BuilderFromManifest { + fn with_instructions( + instructions: impl IntoIterator, + ) -> ManifestBuilder; + + fn with_manifest(manifest: TransactionManifest) -> ManifestBuilder { + Self::with_instructions(manifest.instructions().clone()) + } +} + +impl BuilderFromManifest for ManifestBuilder { + fn with_instructions( + instructions: impl IntoIterator, + ) -> ManifestBuilder { + instructions.into_iter().fold( + ManifestBuilder::new(), + |builder, instruction| { + builder.add_instruction_advanced(instruction).0 + }, + ) + } +} diff --git a/crates/transaction/manifests/src/manifests_security_shield.rs b/crates/transaction/manifests/src/manifests_security_shield/manifests_security_shield.rs similarity index 65% rename from crates/transaction/manifests/src/manifests_security_shield.rs rename to crates/transaction/manifests/src/manifests_security_shield/manifests_security_shield.rs index ca4289073..43d797ccf 100644 --- a/crates/transaction/manifests/src/manifests_security_shield.rs +++ b/crates/transaction/manifests/src/manifests_security_shield/manifests_security_shield.rs @@ -1,16 +1,10 @@ use profile_supporting_types::AnyUnsecurifiedEntity; -use radix_common::prelude::{ - ManifestGlobalAddress, - ACCESS_CONTROLLER_PACKAGE as SCRYPTO_ACCESS_CONTROLLER_PACKAGE, -}; +use radix_common::prelude::ACCESS_CONTROLLER_PACKAGE as SCRYPTO_ACCESS_CONTROLLER_PACKAGE; use radix_engine_interface::blueprints::{ access_controller::{ - AccessControllerContributeRecoveryFeeManifestInput, AccessControllerCreateManifestInput as ScryptoAccessControllerCreateManifestInput, ACCESS_CONTROLLER_BLUEPRINT as SCRYPTO_ACCESS_CONTROLLER_BLUEPRINT, - ACCESS_CONTROLLER_CONTRIBUTE_RECOVERY_FEE_IDENT as SCRYPTO_ACCESS_CONTROLLER_CONTRIBUTE_RECOVERY_FEE_IDENT, ACCESS_CONTROLLER_CREATE_IDENT as SCRYPTO_ACCESS_CONTROLLER_CREATE_IDENT, - ACCESS_CONTROLLER_CREATE_PROOF_IDENT, }, account::AccountSecurifyManifestInput as ScryptoAccountSecurifyManifestInput, }; @@ -35,149 +29,12 @@ impl TransactionManifestApplySecurityShieldUnsecurifiedInput { } } -pub trait BuilderFromManifest { - fn with_instructions( - instructions: impl IntoIterator, - ) -> ManifestBuilder; - - fn with_manifest(manifest: TransactionManifest) -> ManifestBuilder { - Self::with_instructions(manifest.instructions().clone()) - } -} - -impl BuilderFromManifest for ManifestBuilder { - fn with_instructions( - instructions: impl IntoIterator, - ) -> ManifestBuilder { - instructions.into_iter().fold( - ManifestBuilder::new(), - |builder, instruction| { - builder.add_instruction_advanced(instruction).0 - }, - ) - } -} - pub trait TransactionManifestSecurifyEntity: Sized { fn apply_security_shield_for_unsecurified_entity( unsecurified_entity: AnyUnsecurifiedEntity, input: TransactionManifestApplySecurityShieldUnsecurifiedInput, ) -> Result; - /// A method modifying manifests which applies security shield. We - /// The `manifest` which applies the security shield could not include - /// the instructions which we append in this method since the host did - /// not know the `payer` at the time of creating that manifest. The host - /// will call this method when the `payer` is known, which appends instructions - /// at the end of `manifest` for topping the XRD vault of the access controller - /// with `top_up_amount` many XRD. - /// - /// N.B. We will call this method for both when `entity_applying_shield` is - /// securified or unsecurified. In the case of unsecurified entity we will use - /// the address reservation of `apply_security_shield_for_unsecurified_entity` - /// (`ACCESS_CONTROLLER_ADDRESS_RESERVATION_NAME`) which we cannot access by id - /// since Radix Engine discard those ids and uses internal ones, instead we need - /// to use `ManifestGlobalAddress::Named(ScryptoManifestNamedAddress(0))`. - /// If `entity_applying_shield` is securified we will use the address of the - /// already existing access controller. - /// - /// If `payer` is securified we will also add a `create_proof` instruction for - /// authenticating the withdrawal of XRD from the payer. - /// - /// If `top_up_amount` is `None` the `XRD_TO_AC_VAULT_FIRST_TOP_UP` will be used. - /// We allow to pass amount so that we can top of with more or less based on - /// token balance of `payer` and current balance of the access controller (when - /// we use this method for securified entities.) - fn modify_manifest_add_withdraw_of_xrd_for_access_controller_xrd_vault_top_up_paid_by_account( - payer: Account, - // TODO: remove `entity_applying_shield`, this should be read out from the manifest in a throwing function, `manifest.get_address_of_entity_applying_shield()` or similar which Omar need to provide us with, oh well we need the account here, so elsewhere, in SargonOS where we have access to Profile we would call `manifest.get_address_of_entity_applying_shield` and then lookup the entity. - entity_applying_shield: Account, - manifest: TransactionManifest, - top_up_amount: impl Into>, - ) -> TransactionManifest { - let address_of_paying_account = payer.address(); - let mut builder = ManifestBuilder::with_manifest(manifest); - - let address_of_access_controller_to_top_up = - match entity_applying_shield.security_state() { - EntitySecurityState::Securified { value: sec } => { - ManifestGlobalAddress::Static( - sec.access_controller_address.scrypto(), - ) - } - EntitySecurityState::Unsecured { .. } => { - // We are securifying an unsecurified account => use the - // address reservation at index 0, - // which `apply_security_shield_for_unsecurified_entity` - // is using to create the access controller address - ManifestGlobalAddress::Named(ScryptoManifestNamedAddress(0)) - } - }; - - let address_of_access_controller_of_payer = { - match payer.security_state() { - EntitySecurityState::Securified { value: sec } => { - Some(ManifestGlobalAddress::Static( - sec.access_controller_address.scrypto(), - )) - } - EntitySecurityState::Unsecured { .. } => { - // No access controller to create proof for - None - } - } - }; - - // Add `create_proof` instruction for the access controller - if let Some(address_of_access_controller_of_payer) = - address_of_access_controller_of_payer - { - builder = builder.call_method( - address_of_access_controller_of_payer, - ACCESS_CONTROLLER_CREATE_PROOF_IDENT, - (), - ); - } - - let top_up_amount = top_up_amount - .into() - .map(ScryptoDecimal192::from) - .unwrap_or(XRD_TO_AC_VAULT_FIRST_TOP_UP); - - // Add withdraw XRD instruction - builder = builder.withdraw_from_account( - address_of_paying_account.scrypto(), - XRD, - top_up_amount, - ); - - // Deposit XRD into the access controllers XRD vault - // ... by first taking the XRD from the work top - let xrd_to_top_up_ac_vault_bucket_name = - "xrd_to_top_up_ac_vault_bucket"; - builder = builder.take_from_worktop( - XRD, - top_up_amount, - xrd_to_top_up_ac_vault_bucket_name, - ); - let xrd_to_top_up_ac_vault_bucket = - builder.bucket(xrd_to_top_up_ac_vault_bucket_name); - - // ... then deposit to XRD vault of access controller - builder = builder.call_method( - address_of_access_controller_to_top_up, - SCRYPTO_ACCESS_CONTROLLER_CONTRIBUTE_RECOVERY_FEE_IDENT, - AccessControllerContributeRecoveryFeeManifestInput { - bucket: xrd_to_top_up_ac_vault_bucket, - }, - ); - - TransactionManifest::sargon_built( - builder, - address_of_paying_account.network_id(), - ) - } - fn set_rola_key( builder: ManifestBuilder, authentication_signing_factor_instance: &HierarchicalDeterministicFactorInstance, @@ -185,9 +42,6 @@ pub trait TransactionManifestSecurifyEntity: Sized { ) -> ManifestBuilder; } -const XRD_TO_AC_VAULT_FIRST_TOP_UP: ScryptoDecimal192 = - ScryptoDecimal192::ONE_HUNDRED; - impl TransactionManifestSecurifyEntity for TransactionManifest { fn set_rola_key( builder: ManifestBuilder, diff --git a/crates/transaction/manifests/src/manifests_security_shield/mod.rs b/crates/transaction/manifests/src/manifests_security_shield/mod.rs new file mode 100644 index 000000000..8e47aa265 --- /dev/null +++ b/crates/transaction/manifests/src/manifests_security_shield/mod.rs @@ -0,0 +1,6 @@ +#[allow(clippy::module_inception)] +mod manifests_security_shield; +mod top_up_access_controller_xrd_vault; + +pub use manifests_security_shield::*; +pub use top_up_access_controller_xrd_vault::*; diff --git a/crates/transaction/manifests/src/manifests_security_shield/top_up_access_controller_xrd_vault.rs b/crates/transaction/manifests/src/manifests_security_shield/top_up_access_controller_xrd_vault.rs new file mode 100644 index 000000000..d9943dba4 --- /dev/null +++ b/crates/transaction/manifests/src/manifests_security_shield/top_up_access_controller_xrd_vault.rs @@ -0,0 +1,133 @@ +use radix_common::prelude::ManifestGlobalAddress; +use radix_engine_interface::blueprints::access_controller::{ + AccessControllerContributeRecoveryFeeManifestInput, + ACCESS_CONTROLLER_CONTRIBUTE_RECOVERY_FEE_IDENT as SCRYPTO_ACCESS_CONTROLLER_CONTRIBUTE_RECOVERY_FEE_IDENT, + ACCESS_CONTROLLER_CREATE_PROOF_IDENT as SCRYPTO_ACCESS_CONTROLLER_CREATE_PROOF_IDENT, +}; +use radix_transactions::prelude::ManifestBuilder; + +use crate::prelude::*; + +const XRD_TO_AC_VAULT_FIRST_TOP_UP: ScryptoDecimal192 = + ScryptoDecimal192::ONE_HUNDRED; + +pub trait TransactionManifestAccessControllerXrdVaultToppingUp { + /// A method modifying manifests which applies security shield. We + /// The `manifest` which applies the security shield could not include + /// the instructions which we append in this method since the host did + /// not know the `payer` at the time of creating that manifest. The host + /// will call this method when the `payer` is known, which appends instructions + /// at the end of `manifest` for topping the XRD vault of the access controller + /// with `top_up_amount` many XRD. + /// + /// N.B. We will call this method for both when `entity_applying_shield` is + /// securified or unsecurified. In the case of unsecurified entity we will use + /// the address reservation of `apply_security_shield_for_unsecurified_entity` + /// (`ACCESS_CONTROLLER_ADDRESS_RESERVATION_NAME`) which we cannot access by id + /// since Radix Engine discard those ids and uses internal ones, instead we need + /// to use `ManifestGlobalAddress::Named(ScryptoManifestNamedAddress(0))`. + /// If `entity_applying_shield` is securified we will use the address of the + /// already existing access controller. + /// + /// If `payer` is securified we will also add a `create_proof` instruction for + /// authenticating the withdrawal of XRD from the payer. + /// + /// If `top_up_amount` is `None` the `XRD_TO_AC_VAULT_FIRST_TOP_UP` will be used. + /// We allow to pass amount so that we can top of with more or less based on + /// token balance of `payer` and current balance of the access controller (when + /// we use this method for securified entities.) + fn modify_manifest_add_withdraw_of_xrd_for_access_controller_xrd_vault_top_up_paid_by_account( + payer: Account, + // TODO: remove `entity_applying_shield`, this should be read out from the manifest in a throwing function, `manifest.get_address_of_entity_applying_shield()` or similar which Omar need to provide us with, oh well we need the account here, so elsewhere, in SargonOS where we have access to Profile we would call `manifest.get_address_of_entity_applying_shield` and then lookup the entity. + entity_applying_shield: Account, + manifest: TransactionManifest, + top_up_amount: impl Into>, + ) -> TransactionManifest { + let address_of_paying_account = payer.address(); + let mut builder = ManifestBuilder::with_manifest(manifest); + + let address_of_access_controller_to_top_up = + match entity_applying_shield.security_state() { + EntitySecurityState::Securified { value: sec } => { + ManifestGlobalAddress::Static( + sec.access_controller_address.scrypto(), + ) + } + EntitySecurityState::Unsecured { .. } => { + // We are securifying an unsecurified account => use the + // address reservation at index 0, + // which `apply_security_shield_for_unsecurified_entity` + // is using to create the access controller address + ManifestGlobalAddress::Named(ScryptoManifestNamedAddress(0)) + } + }; + + let address_of_access_controller_of_payer = { + match payer.security_state() { + EntitySecurityState::Securified { value: sec } => { + Some(ManifestGlobalAddress::Static( + sec.access_controller_address.scrypto(), + )) + } + EntitySecurityState::Unsecured { .. } => { + // No access controller to create proof for + None + } + } + }; + + // Add `create_proof` instruction for the access controller + if let Some(address_of_access_controller_of_payer) = + address_of_access_controller_of_payer + { + builder = builder.call_method( + address_of_access_controller_of_payer, + SCRYPTO_ACCESS_CONTROLLER_CREATE_PROOF_IDENT, + (), + ); + } + + let top_up_amount = top_up_amount + .into() + .map(ScryptoDecimal192::from) + .unwrap_or(XRD_TO_AC_VAULT_FIRST_TOP_UP); + + // Add withdraw XRD instruction + builder = builder.withdraw_from_account( + address_of_paying_account.scrypto(), + XRD, + top_up_amount, + ); + + // Deposit XRD into the access controllers XRD vault + // ... by first taking the XRD from the work top + let xrd_to_top_up_ac_vault_bucket_name = + "xrd_to_top_up_ac_vault_bucket"; + builder = builder.take_from_worktop( + XRD, + top_up_amount, + xrd_to_top_up_ac_vault_bucket_name, + ); + let xrd_to_top_up_ac_vault_bucket = + builder.bucket(xrd_to_top_up_ac_vault_bucket_name); + + // ... then deposit to XRD vault of access controller + builder = builder.call_method( + address_of_access_controller_to_top_up, + SCRYPTO_ACCESS_CONTROLLER_CONTRIBUTE_RECOVERY_FEE_IDENT, + AccessControllerContributeRecoveryFeeManifestInput { + bucket: xrd_to_top_up_ac_vault_bucket, + }, + ); + + TransactionManifest::sargon_built( + builder, + address_of_paying_account.network_id(), + ) + } +} + +impl TransactionManifestAccessControllerXrdVaultToppingUp + for TransactionManifest +{ +}