From 90087f55b8297579ad1d34ffa7e3ed126bb32daa Mon Sep 17 00:00:00 2001 From: Henrique Nogara Date: Wed, 26 Feb 2025 11:18:58 -0300 Subject: [PATCH] Mesh 1922 - Remove `with_transaction` (#1791) * Remove with_transaction - part 1 * Remove with_transaction - part 2 --------- Co-authored-by: Robert Gabriel Jakabosky --- .../corporate-actions/src/distribution/mod.rs | 39 +++---- pallets/corporate-actions/src/lib.rs | 103 +++++++++--------- pallets/external-agents/src/lib.rs | 10 +- pallets/pips/src/lib.rs | 33 +++--- pallets/settlement/src/lib.rs | 8 +- pallets/sto/src/lib.rs | 88 ++++++++------- 6 files changed, 132 insertions(+), 149 deletions(-) diff --git a/pallets/corporate-actions/src/distribution/mod.rs b/pallets/corporate-actions/src/distribution/mod.rs index 551f0163b..51d689ec3 100644 --- a/pallets/corporate-actions/src/distribution/mod.rs +++ b/pallets/corporate-actions/src/distribution/mod.rs @@ -77,9 +77,8 @@ use pallet_identity::PermissionedCallOriginData; use polymesh_common_utilities::protocol_fee::{ChargeProtocolFee, ProtocolOp}; use polymesh_primitives::asset::AssetId; use polymesh_primitives::{ - constants::currency::ONE_UNIT, storage_migration_ver, traits::PortfolioSubTrait, - with_transaction, Balance, EventDid, IdentityId, Moment, PortfolioId, PortfolioNumber, - SecondaryKey, WeightMeter, + constants::currency::ONE_UNIT, storage_migration_ver, traits::PortfolioSubTrait, Balance, + EventDid, IdentityId, Moment, PortfolioId, PortfolioNumber, SecondaryKey, WeightMeter, }; use scale_info::TypeInfo; use sp_runtime::traits::Zero; @@ -583,24 +582,22 @@ impl Pallet { gain / ONE_UNIT * ONE_UNIT }; - with_transaction(|| { - // Unlock `benefit` of `currency` from the calling agent's portfolio. - Self::unlock(&dist, benefit)?; - - // Transfer remainder (`gain`) to DID. - let to = PortfolioId::default_portfolio(holder); - let mut weight_meter = WeightMeter::max_limit_no_minimum(); - >::base_transfer( - dist.from, - to, - dist.currency, - gain, - None, - None, - actor.clone().risky_into_inner(), - &mut weight_meter, - ) - })?; + // Unlock `benefit` of `currency` from the calling agent's portfolio. + Self::unlock(&dist, benefit)?; + + // Transfer remainder (`gain`) to DID. + let to = PortfolioId::default_portfolio(holder); + let mut weight_meter = WeightMeter::max_limit_no_minimum(); + >::base_transfer( + dist.from, + to, + dist.currency, + gain, + None, + None, + actor.clone().risky_into_inner(), + &mut weight_meter, + )?; // Note that DID was paid. HolderPaid::::insert((ca_id, holder), true); diff --git a/pallets/corporate-actions/src/lib.rs b/pallets/corporate-actions/src/lib.rs index dff33b224..a93098b34 100644 --- a/pallets/corporate-actions/src/lib.rs +++ b/pallets/corporate-actions/src/lib.rs @@ -108,8 +108,8 @@ use pallet_identity::{Config as IdentityConfig, PermissionedCallOriginData}; use polymesh_common_utilities::checkpoint::ScheduleId; use polymesh_primitives::asset::AssetId; use polymesh_primitives::{ - asset::CheckpointId, impl_checked_inc, storage_migration_ver, with_transaction, Balance, - DocumentId, EventDid, IdentityId, Moment, PortfolioNumber, GC_DID, + asset::CheckpointId, impl_checked_inc, storage_migration_ver, Balance, DocumentId, EventDid, + IdentityId, Moment, PortfolioNumber, GC_DID, }; use polymesh_primitives_derive::VecU8StrongTyped; use scale_info::TypeInfo; @@ -827,31 +827,28 @@ pub mod pallet { let agent = caller_did.for_event(); let mut ca = Self::ensure_ca_exists(ca_id)?; - with_transaction(|| -> DispatchResult { - // If provided, either use the existing CP ID or schedule one to be made. - Self::dec_strong_ref_count(ca_id, ca.record_date); - ca.record_date = record_date - .map(|date| Self::handle_record_date(caller_did, ca_id.asset_id, date)) - .transpose()?; - - // Ensure associated services allow changing the date. - match ca.kind { - CAKind::Other | CAKind::Reorganization => {} - CAKind::IssuerNotice => { - if let Some(range) = TimeRanges::::get(ca_id) { - Self::ensure_record_date_before_start(&ca, range.start)?; - >::ensure_ballot_not_started(range)?; - } + // If provided, either use the existing CP ID or schedule one to be made. + Self::dec_strong_ref_count(ca_id, ca.record_date); + ca.record_date = record_date + .map(|date| Self::handle_record_date(caller_did, ca_id.asset_id, date)) + .transpose()?; + + // Ensure associated services allow changing the date. + match ca.kind { + CAKind::Other | CAKind::Reorganization => {} + CAKind::IssuerNotice => { + if let Some(range) = TimeRanges::::get(ca_id) { + Self::ensure_record_date_before_start(&ca, range.start)?; + >::ensure_ballot_not_started(range)?; } - CAKind::PredictableBenefit | CAKind::UnpredictableBenefit => { - if let Some(dist) = Distributions::::get(ca_id) { - Self::ensure_record_date_before_start(&ca, dist.payment_at)?; - >::ensure_distribution_not_started(&dist)?; - } + } + CAKind::PredictableBenefit | CAKind::UnpredictableBenefit => { + if let Some(dist) = Distributions::::get(ca_id) { + Self::ensure_record_date_before_start(&ca, dist.payment_at)?; + >::ensure_distribution_not_started(&dist)?; } } - Ok(()) - })?; + } // Commit changes + emit event. CorporateActions::::insert(ca_id.asset_id, ca_id.local_id, ca.clone()); @@ -890,31 +887,31 @@ pub mod pallet { .. } = >::ensure_agent_asset_perms(origin, asset_id)?; - with_transaction(|| { - let ca_id = Self::unsafe_initiate_corporate_action( - caller_did, - asset_id, - kind, - decl_date, - record_date, - details, - targets, - default_withholding_tax, - withholding_tax, - )?; - - >::unverified_distribute( - caller_did, - secondary_key, - ca_id, - portfolio, - currency, - per_share, - amount, - payment_at, - expires_at, - ) - }) + let ca_id = Self::unsafe_initiate_corporate_action( + caller_did, + asset_id, + kind, + decl_date, + record_date, + details, + targets, + default_withholding_tax, + withholding_tax, + )?; + + >::unverified_distribute( + caller_did, + secondary_key, + ca_id, + portfolio, + currency, + per_share, + amount, + payment_at, + expires_at, + )?; + + Ok(()) } } @@ -1025,11 +1022,9 @@ impl Pallet { // If provided, either use the existing CP ID or schedule one to be made. let record_date = record_date .map(|date| { - with_transaction(|| -> Result<_, DispatchError> { - let rd = Self::handle_record_date(caller_did, asset_id, date)?; - ensure!(decl_date <= rd.date, Error::::DeclDateAfterRecordDate); - Ok(rd) - }) + let rd = Self::handle_record_date(caller_did, asset_id, date)?; + ensure!(decl_date <= rd.date, Error::::DeclDateAfterRecordDate); + Ok::(rd) }) .transpose()?; diff --git a/pallets/external-agents/src/lib.rs b/pallets/external-agents/src/lib.rs index b68ff21ef..59fa3d778 100644 --- a/pallets/external-agents/src/lib.rs +++ b/pallets/external-agents/src/lib.rs @@ -63,9 +63,8 @@ use pallet_permissions::{CurrentDispatchableName, CurrentPalletName}; use polymesh_primitives::agent::{AGId, AgentGroup}; use polymesh_primitives::asset::AssetId; use polymesh_primitives::{ - extract_auth, storage_migration_ver, traits::AssetFnConfig, with_transaction, - AuthorizationData, EventDid, ExtrinsicPermissions, IdentityId, PalletPermissions, Signatory, - SubsetRestriction, + extract_auth, storage_migration_ver, traits::AssetFnConfig, AuthorizationData, EventDid, + ExtrinsicPermissions, IdentityId, PalletPermissions, Signatory, SubsetRestriction, }; use sp_std::prelude::*; @@ -376,9 +375,8 @@ pub mod pallet { perms: ExtrinsicPermissions, agent: IdentityId, ) -> DispatchResult { - with_transaction(|| { - Self::base_create_and_change_custom_group(origin, asset_id, perms, agent) - }) + Self::base_create_and_change_custom_group(origin, asset_id, perms, agent)?; + Ok(()) } } } diff --git a/pallets/pips/src/lib.rs b/pallets/pips/src/lib.rs index 5d8dff480..dc0fc1982 100644 --- a/pallets/pips/src/lib.rs +++ b/pallets/pips/src/lib.rs @@ -94,9 +94,9 @@ use pallet_base::{ensure_opt_string_limited, try_next_post}; use pallet_identity::{Config as IdentityConfig, PermissionedCallOriginData}; use polymesh_common_utilities::protocol_fee::{ChargeProtocolFee, ProtocolOp}; use polymesh_primitives::constants::PIP_MAX_REPORTING_SIZE; +use polymesh_primitives::storage_migration_ver; use polymesh_primitives::traits::group::GroupTrait; use polymesh_primitives::traits::GovernanceGroupTrait; -use polymesh_primitives::{storage_migration_ver, with_transaction}; use polymesh_primitives::{Balance, IdentityId, MaybeBlock, Url}; use polymesh_primitives::{GC_DID, TECHNICAL_DID, UPGRADE_DID}; use polymesh_runtime_common::PipsEnactSnapshotMaximumWeight; @@ -737,11 +737,8 @@ pub mod pallet { ); // Lock the deposit + charge protocol fees. - // Both do check-modify so we need a transaction. - with_transaction(|| { - Self::increase_lock(proposer, deposit)?; - charge() - })?; + Self::increase_lock(proposer, deposit)?; + charge()?; } else { // Committee PIPs cannot have a deposit. ensure!(deposit.is_zero(), Error::::NotFromCommunity); @@ -877,19 +874,17 @@ pub mod pallet { let old_res = Self::aggregate_result(id); - with_transaction(|| { - // Reserve the deposit, or refund if needed. - let curr_deposit = Deposits::::get(id, &voter) - .map(|d| d.amount) - .unwrap_or_default(); - if deposit < curr_deposit { - Self::reduce_lock(&voter, curr_deposit - deposit)?; - } else { - Self::increase_lock(&voter, deposit - curr_deposit)?; - } - // Save the vote. - Self::unsafe_vote(id, voter.clone(), Vote(aye_or_nay, deposit)) - })?; + // Reserve the deposit, or refund if needed. + let curr_deposit = Deposits::::get(id, &voter) + .map(|d| d.amount) + .unwrap_or_default(); + if deposit < curr_deposit { + Self::reduce_lock(&voter, curr_deposit - deposit)?; + } else { + Self::increase_lock(&voter, deposit - curr_deposit)?; + } + // Save the vote. + Self::unsafe_vote(id, voter.clone(), Vote(aye_or_nay, deposit))?; // Adjust live queue. Self::adjust_live_queue(id, old_res); diff --git a/pallets/settlement/src/lib.rs b/pallets/settlement/src/lib.rs index 8491345a5..6b1fc1e34 100644 --- a/pallets/settlement/src/lib.rs +++ b/pallets/settlement/src/lib.rs @@ -1371,12 +1371,12 @@ impl Pallet { amount, .. } => T::Portfolio::lock_tokens(&sender, &asset_id, *amount), - Leg::NonFungible { sender, nfts, .. } => with_transaction(|| { + Leg::NonFungible { sender, nfts, .. } => { for nft_id in nfts.ids() { T::Portfolio::lock_nft(&sender, nfts.asset_id(), &nft_id)?; } Ok(()) - }), + } Leg::OffChain { .. } => Err(Error::::OffChainAssetCantBeLocked.into()), } } @@ -1389,12 +1389,12 @@ impl Pallet { amount, .. } => T::Portfolio::unlock_tokens(&sender, &asset_id, *amount), - Leg::NonFungible { sender, nfts, .. } => with_transaction(|| { + Leg::NonFungible { sender, nfts, .. } => { for nft_id in nfts.ids() { T::Portfolio::unlock_nft(&sender, nfts.asset_id(), &nft_id)?; } Ok(()) - }), + } Leg::OffChain { .. } => Err(Error::::OffChainAssetCantBeLocked.into()), } } diff --git a/pallets/sto/src/lib.rs b/pallets/sto/src/lib.rs index 6e7f26453..954c2571b 100644 --- a/pallets/sto/src/lib.rs +++ b/pallets/sto/src/lib.rs @@ -42,8 +42,8 @@ use polymesh_primitives::asset::AssetId; use polymesh_primitives::impl_checked_inc; use polymesh_primitives::settlement::{Leg, ReceiptDetails, SettlementType, VenueId, VenueType}; use polymesh_primitives::{ - storage_migration_ver, traits::PortfolioSubTrait, with_transaction, Balance, EventDid, - IdentityId, PortfolioId, WeightMeter, + storage_migration_ver, traits::PortfolioSubTrait, Balance, EventDid, IdentityId, PortfolioId, + WeightMeter, }; use polymesh_primitives_derive::VecU8StrongTyped; @@ -559,49 +559,47 @@ pub mod pallet { }, ]; - with_transaction(|| { - >::unlock_tokens( - &fundraiser.offering_portfolio, - &fundraiser.offering_asset, - purchase_amount, - )?; - - let instruction_id = Settlement::::base_add_instruction( - fundraiser.creator, - Some(fundraiser.venue_id), - SettlementType::SettleOnAffirmation, - None, - None, - legs, - None, - None, - )?; - - let portfolios = [fundraiser.offering_portfolio, fundraiser.raising_portfolio] - .iter() - .copied() - .collect::>(); - Settlement::::unsafe_affirm_instruction( - fundraiser.creator, - instruction_id, - portfolios, - None, - None, - )?; - - let portfolios = [investment_portfolio, funding_portfolio] - .iter() - .copied() - .collect::>(); - Settlement::::affirm_and_execute_instruction( - origin, - instruction_id, - receipt, - portfolios, - did, - &mut WeightMeter::max_limit_no_minimum(), - ) - })?; + >::unlock_tokens( + &fundraiser.offering_portfolio, + &fundraiser.offering_asset, + purchase_amount, + )?; + + let instruction_id = Settlement::::base_add_instruction( + fundraiser.creator, + Some(fundraiser.venue_id), + SettlementType::SettleOnAffirmation, + None, + None, + legs, + None, + None, + )?; + + let portfolios = [fundraiser.offering_portfolio, fundraiser.raising_portfolio] + .iter() + .copied() + .collect::>(); + Settlement::::unsafe_affirm_instruction( + fundraiser.creator, + instruction_id, + portfolios, + None, + None, + )?; + + let portfolios = [investment_portfolio, funding_portfolio] + .iter() + .copied() + .collect::>(); + Settlement::::affirm_and_execute_instruction( + origin, + instruction_id, + receipt, + portfolios, + did, + &mut WeightMeter::max_limit_no_minimum(), + )?; for (id, amount) in purchases { fundraiser.tiers[id].remaining -= amount;