diff --git a/pallets/afloat/src/functions.rs b/pallets/afloat/src/functions.rs index 7539c183..15c83801 100644 --- a/pallets/afloat/src/functions.rs +++ b/pallets/afloat/src/functions.rs @@ -4,7 +4,6 @@ use frame_support::pallet_prelude::*; use frame_system::{pallet_prelude::*, RawOrigin}; use pallet_fruniques::types::{Attributes, CollectionDescription, FruniqueRole, ParentInfo}; use pallet_gated_marketplace::types::{Marketplace, MarketplaceRole}; -use sp_runtime::{traits::StaticLookup, Permill}; // use frame_support::traits::OriginTrait; use core::convert::TryInto; use frame_support::traits::Time; @@ -12,8 +11,9 @@ use pallet_rbac::types::{IdOrVec, RoleBasedAccessControl, RoleId}; use scale_info::prelude::vec; use sp_io::hashing::blake2_256; use sp_runtime::{ + Permill, sp_std::{str, vec::Vec}, - traits::Zero, + traits::{Zero, StaticLookup, CheckedMul}, }; impl Pallet { @@ -473,10 +473,12 @@ impl Pallet { offer.tax_credit_amount_remaining >= tax_credit_amount, Error::::NotEnoughTaxCreditsAvailable ); + + let price_per_credit: T::Balance = offer.price_per_credit.into(); + let total_price: T::Balance = Self::safe_multiply_offer(price_per_credit, tax_credit_amount)?; //ensure user has enough afloat balance ensure!( - Self::do_get_afloat_balance(who.clone())? >= - offer.price_per_credit * tax_credit_amount.into(), + Self::do_get_afloat_balance(who.clone())? >= total_price, Error::::NotEnoughAfloatBalanceAvailable ); let zero_balance: T::Balance = Zero::zero(); @@ -484,8 +486,6 @@ impl Pallet { ensure!(tax_credit_amount > zero_balance, Error::::Underflow); let creation_date: u64 = T::Timestamp::now().into(); - let price_per_credit: T::Balance = offer.price_per_credit.into(); - let total_price: T::Balance = price_per_credit * tax_credit_amount; let fee: Option = None; let tax_credit_id: ::ItemId = offer.tax_credit_id; let seller_id: T::AccountId = offer.creator_id; @@ -531,6 +531,8 @@ impl Pallet { Ok(()) } + + /// Confirms a sell transaction. /// /// # Arguments @@ -933,6 +935,14 @@ impl Pallet { }) } + fn safe_multiply_offer( + factor1: T::Balance, + factor2: T::Balance + ) -> Result { + let result = factor1.checked_mul(&factor2).ok_or_else(|| Error::::ArithmeticOverflow)?; + Ok(result) + } + fn get_all_roles_for_user(account_id: T::AccountId) -> Result, DispatchError> { let roles_storage = ::Rbac::get_roles_by_user( account_id.clone(), diff --git a/pallets/afloat/src/lib.rs b/pallets/afloat/src/lib.rs index 918b47c3..9edaad82 100644 --- a/pallets/afloat/src/lib.rs +++ b/pallets/afloat/src/lib.rs @@ -146,6 +146,8 @@ pub mod pallet { ChildOfferIdNotFound, // Tax credit amount underflow Underflow, + // Arithmetic multiplication overflow + ArithmeticOverflow, // Afloat marketplace label too long LabelTooLong, // Afloat asset has not been set diff --git a/pallets/afloat/src/mock.rs b/pallets/afloat/src/mock.rs index 032d4f17..ff935bc6 100644 --- a/pallets/afloat/src/mock.rs +++ b/pallets/afloat/src/mock.rs @@ -187,6 +187,7 @@ impl pallet_rbac::Config for Test { type MaxPermissionsPerRole = MaxPermissionsPerRole; type MaxRolesPerUser = MaxRolesPerUser; type MaxUsersPerRole = MaxUsersPerRole; + type WeightInfo = (); } impl pallet_timestamp::Config for Test { diff --git a/pallets/afloat/src/tests.rs b/pallets/afloat/src/tests.rs index 295126c4..f15b4cc9 100644 --- a/pallets/afloat/src/tests.rs +++ b/pallets/afloat/src/tests.rs @@ -1,5 +1,5 @@ use super::*; -use crate::{mock::*, types::*, Error}; +use crate::{mock::*, types::*, AfloatOffers, Error}; use frame_support::{assert_noop, assert_ok, traits::Currency, BoundedVec}; use frame_system::RawOrigin; @@ -16,6 +16,61 @@ fn dummy_description() -> BoundedVec { //buy_fee = 2% //sell_fee = 4% +#[test] +fn replicate_overflow_for_start_take_sell_order() { + TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| { + let user = new_account(3); + let other_user = new_account(4); + let item_id = 0; + + Balances::make_free_balance_be(&user, 100); + Balances::make_free_balance_be(&other_user, 100); + + let args = SignUpArgs::BuyerOrSeller { + cid: ShortString::try_from(b"cid".to_vec()).unwrap(), + cid_creator: ShortString::try_from(b"cid_creator".to_vec()).unwrap(), + group: ShortString::try_from(b"Group".to_vec()).unwrap(), + }; + + assert_ok!(Afloat::sign_up(RawOrigin::Signed(user.clone()).into(), args.clone())); + assert_ok!(Afloat::sign_up(RawOrigin::Signed(other_user.clone()).into(), args.clone())); + + assert_ok!(Afloat::set_afloat_balance( + RuntimeOrigin::signed(1), + other_user.clone(), + 100000 + )); + assert_eq!(Afloat::do_get_afloat_balance(other_user.clone()).unwrap(), 100000); + + assert_ok!(Afloat::create_tax_credit( + RawOrigin::Signed(user.clone()).into(), + dummy_description(), + None, + None, + )); + + assert_ok!(Afloat::create_offer( + RawOrigin::Signed(user.clone()).into(), + CreateOfferArgs::Sell { + tax_credit_id: item_id, + price_per_credit: 18446744073709551615, + tax_credit_amount: 10, + expiration_date: 1000 + } + )); + + let (offer_id, _offer) = AfloatOffers::::iter().next().unwrap().clone(); + assert_noop!( + Afloat::start_take_sell_order( + RawOrigin::Signed(other_user.clone()).into(), + offer_id, + 10 + ), + Error::::ArithmeticOverflow + ); + }); +} + #[test] fn sign_up_works() { TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| { diff --git a/pallets/fund-admin/src/functions.rs b/pallets/fund-admin/src/functions.rs index 1c4dc0b7..a867f444 100644 --- a/pallets/fund-admin/src/functions.rs +++ b/pallets/fund-admin/src/functions.rs @@ -2008,8 +2008,8 @@ impl Pallet { // Create revenue transaction id let revenue_transaction_id = - (revenue_id, job_eligible_id, project_id, timestamp).using_encoded(blake2_256); - + (revenue_id, revenue_amount, job_eligible_id, project_id, timestamp) + .using_encoded(blake2_256); // Ensure revenue transaction id doesn't exist ensure!( !RevenueTransactionsInfo::::contains_key(revenue_transaction_id), @@ -2031,11 +2031,6 @@ impl Pallet { }; // Insert revenue transaction data into RevenueTransactionsInfo - // Ensure revenue transaction id doesn't exist - ensure!( - !RevenueTransactionsInfo::::contains_key(revenue_transaction_id), - Error::::RevenueTransactionIdAlreadyExists - ); >::insert(revenue_transaction_id, revenue_transaction_data); // Insert revenue transaction id into TransactionsByRevenue @@ -3125,6 +3120,14 @@ impl Pallet { Ok(()) } + fn safe_add( + a: u64, + b: u64 + ) -> Result { + let result = a.checked_add(b).ok_or_else(|| Error::::ArithmeticOverflow)?; + Ok(result) + } + fn do_calculate_drawdown_total_amount( project_id: [u8; 32], drawdown_id: [u8; 32], @@ -3147,7 +3150,7 @@ impl Pallet { .ok_or(Error::::TransactionNotFound)?; // Add transaction amount to drawdown total amount - drawdown_total_amount += transaction_data.amount; + drawdown_total_amount = Self::safe_add(drawdown_total_amount, transaction_data.amount)?; } } @@ -3299,7 +3302,7 @@ impl Pallet { .ok_or(Error::::RevenueTransactionNotFound)?; // Add revenue transaction amount to revenue total amount - revenue_total_amount += revenue_transaction_data.amount; + revenue_total_amount = Self::safe_add(revenue_total_amount, revenue_transaction_data.amount)?; } } diff --git a/pallets/fund-admin/src/lib.rs b/pallets/fund-admin/src/lib.rs index eb1988d6..66ec3600 100644 --- a/pallets/fund-admin/src/lib.rs +++ b/pallets/fund-admin/src/lib.rs @@ -694,6 +694,8 @@ pub mod pallet { AdminHasNoFreeBalance, /// Administrator account has insuficiente balance to register a new user InsufficientFundsToTransfer, + // Arithmetic addition overflow + ArithmeticOverflow, } // E X T R I N S I C S diff --git a/pallets/fund-admin/src/mock.rs b/pallets/fund-admin/src/mock.rs index 1bac50bf..826f5645 100644 --- a/pallets/fund-admin/src/mock.rs +++ b/pallets/fund-admin/src/mock.rs @@ -152,6 +152,7 @@ impl pallet_rbac::Config for Test { type MaxRolesPerUser = MaxRolesPerUser; type MaxUsersPerRole = MaxUsersPerRole; type RemoveOrigin = EnsureRoot; + type WeightInfo = (); } // Build genesis storage according to the mock runtime. diff --git a/pallets/fund-admin/src/tests.rs b/pallets/fund-admin/src/tests.rs index 40283513..143eac3f 100644 --- a/pallets/fund-admin/src/tests.rs +++ b/pallets/fund-admin/src/tests.rs @@ -4174,6 +4174,50 @@ fn drawdowns_a_user_submits_a_drawdown_for_approval_works() { }); } +#[test] +fn replicate_overflow_for_a_drawdown_submission() { + new_test_ext().execute_with(|| { + assert_ok!(make_default_full_project()); + let project_id = ProjectsInfo::::iter_keys().next().unwrap(); + + let drawdown_id = get_drawdown_id(project_id, DrawdownType::EB5, 1); + let expenditure_id = get_budget_expenditure_id( + project_id, + make_field_name("Expenditure Test 1"), + ExpenditureType::HardCost, + ); + + let transaction_data = + make_transaction(Some(expenditure_id), Some(1000), CUDAction::Create, None); + + assert_ok!(FundAdmin::submit_drawdown( + RuntimeOrigin::signed(2), + project_id, + drawdown_id, + Some(transaction_data), + false, + )); + + let second_transaction_data = make_transaction( + Some(expenditure_id), + Some(18446744073709551615), + CUDAction::Create, + None, + ); + + assert_noop!( + FundAdmin::submit_drawdown( + RuntimeOrigin::signed(2), + project_id, + drawdown_id, + Some(second_transaction_data), + true, + ), + Error::::ArithmeticOverflow + ); + }); +} + #[test] fn drawdowns_a_user_submits_a_draft_drawdown_for_approval_works() { new_test_ext().execute_with(|| { @@ -5790,6 +5834,46 @@ fn revenues_after_a_revenue_is_submitted_the_next_one_is_generated_automaticaly_ }); } +#[test] +fn replicate_overflow_for_a_revenue_submission() { + new_test_ext().execute_with(|| { + assert_ok!(make_default_full_project()); + let project_id = ProjectsInfo::::iter_keys().next().unwrap(); + + let revenue_id = get_revenue_id(project_id, 1); + let job_eligible_id = get_job_eligible_id(project_id, make_field_name("Job Eligible Test")); + + let revenue_transaction_data = + make_revenue_transaction(Some(job_eligible_id), Some(10000), CUDAction::Create, None); + + assert_ok!(FundAdmin::submit_revenue( + RuntimeOrigin::signed(2), + project_id, + revenue_id, + Some(revenue_transaction_data), + false, + )); + + let second_revenue_transaction_data = make_revenue_transaction( + Some(job_eligible_id), + Some(18446744073709551615), + CUDAction::Create, + None, + ); + + assert_noop!( + FundAdmin::submit_revenue( + RuntimeOrigin::signed(2), + project_id, + revenue_id, + Some(second_revenue_transaction_data), + true, + ), + Error::::ArithmeticOverflow + ); + }); +} + #[test] fn revenues_an_administrator_rejects_a_given_revenue_works() { new_test_ext().execute_with(|| {