diff --git a/Cargo.lock b/Cargo.lock index 82dcef669..b636e13d3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5780,7 +5780,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "426.0.0" +version = "427.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system", @@ -10388,7 +10388,7 @@ dependencies = [ [[package]] name = "pallet-referrals" -version = "1.6.0" +version = "1.6.1" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/pallets/referrals/Cargo.toml b/pallets/referrals/Cargo.toml index 81555e732..2cf39dc97 100644 --- a/pallets/referrals/Cargo.toml +++ b/pallets/referrals/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-referrals" -version = "1.6.0" +version = "1.6.1" authors = ['GalacticCouncil'] edition = "2021" license = "Apache-2.0" diff --git a/pallets/referrals/src/lib.rs b/pallets/referrals/src/lib.rs index 4c18ab262..4434e5dec 100644 --- a/pallets/referrals/src/lib.rs +++ b/pallets/referrals/src/lib.rs @@ -298,6 +298,11 @@ pub mod pallet { from: AssetAmount, to: AssetAmount, }, + /// A pending asset could not be converted; skipped, not blocking. + ConversionFailed { + asset_id: T::AssetId, + reason: DispatchError, + }, /// Rewards claimed. Claimed { who: T::AccountId, @@ -482,20 +487,18 @@ pub mod pallet { let who = ensure_signed(origin)?; for (asset_id, _) in PendingConversions::::iter() { let asset_balance = T::Currency::balance(asset_id.clone(), &Self::pot_account_id()); - let r = T::Convert::convert( + // Best-effort, like `on_idle`: skip an un-convertible slice rather than revert the + // whole claim. Funds stay in the pot and re-queue on the next fee. + if let Err(reason) = T::Convert::convert( Self::pot_account_id(), asset_id.clone(), T::RewardAsset::get(), asset_balance, - ); - if let Err(error) = r { - // We allow these errors to continue claiming as the current amount of asset that needed to be converted - // has very low impact on the rewards. - if error != Error::::ConversionMinTradingAmountNotReached.into() - && error != Error::::ConversionZeroAmountReceived.into() - { - return Err(error); - } + ) { + Self::deposit_event(Event::ConversionFailed { + asset_id: asset_id.clone(), + reason, + }); } PendingConversions::::remove(asset_id); } @@ -623,12 +626,17 @@ pub mod pallet { for asset_id in PendingConversions::::iter_keys().take(max_converts as usize) { let asset_balance = T::Currency::balance(asset_id.clone(), &Self::pot_account_id()); // remove the asset_id from PendingConversions even when the conversion fails - let _ = T::Convert::convert( + if let Err(reason) = T::Convert::convert( Self::pot_account_id(), asset_id.clone(), T::RewardAsset::get(), asset_balance, - ); + ) { + Self::deposit_event(Event::ConversionFailed { + asset_id: asset_id.clone(), + reason, + }); + } PendingConversions::::remove(asset_id); actual_converts += 1; } diff --git a/pallets/referrals/src/tests.rs b/pallets/referrals/src/tests.rs index 3de782792..283968fe9 100644 --- a/pallets/referrals/src/tests.rs +++ b/pallets/referrals/src/tests.rs @@ -70,6 +70,8 @@ thread_local! { pub static TIER_VOLUME: RefCell>> = RefCell::new(HashMap::default()); pub static TIER_REWARDS: RefCell> = RefCell::new(HashMap::default()); pub static SEED_AMOUNT: RefCell = RefCell::new(Balance::zero()); + // Mirrors runtime `MinTradingLimit`: reject sub-minimum amounts. 0 = disabled. + pub static CONVERT_MIN_AMOUNT: RefCell = RefCell::new(Balance::zero()); } construct_runtime!( @@ -218,6 +220,10 @@ impl Default for ExtBuilder { v.borrow_mut().clear(); }); + CONVERT_MIN_AMOUNT.with(|v| { + *v.borrow_mut() = 0u128; + }); + Self { endowed_accounts: vec![(ALICE, HDX, INITIAL_ALICE_BALANCE)], referrer_shares: vec![], @@ -260,6 +266,12 @@ impl ExtBuilder { }); self } + pub fn with_convert_min_amount(self, amount: Balance) -> Self { + CONVERT_MIN_AMOUNT.with(|v| { + *v.borrow_mut() = amount; + }); + self + } pub fn with_seed_amount(self, amount: Balance) -> Self { SEED_AMOUNT.with(|v| { let mut m = v.borrow_mut(); @@ -364,6 +376,11 @@ impl Convert for AssetConvert { asset_to: AssetId, amount: Balance, ) -> Result { + let min_amount = CONVERT_MIN_AMOUNT.with(|v| *v.borrow()); + if amount > 0 && amount < min_amount { + // Like `ConvertViaOmnipool`: sub-min amounts fail with a non-referrals error. + return Err(DispatchError::Other("InsufficientTradingAmount")); + } let price = CONVERSION_RATE .with(|v| v.borrow().get(&(asset_to, asset_from)).copied()) .ok_or(Error::::ConversionMinTradingAmountNotReached)?; diff --git a/pallets/referrals/src/tests/claim.rs b/pallets/referrals/src/tests/claim.rs index 0f680ccb0..44e063f2d 100644 --- a/pallets/referrals/src/tests/claim.rs +++ b/pallets/referrals/src/tests/claim.rs @@ -74,6 +74,40 @@ fn claim_rewards_should_remove_assets_from_the_list_when_not_successful() { }); } +// Regression: sub-`MinTradingLimit` dust in `PendingConversions` must not block reward claims. +#[test] +fn claim_rewards_should_succeed_when_pending_asset_balance_is_below_min_trading_limit() { + ExtBuilder::default() + .with_endowed_accounts(vec![ + (Pallet::::pot_account_id(), HDX, 20_000_000_000_000), + // 500 raw units of DAI — below the converter's min trading limit (1_000). + (Pallet::::pot_account_id(), DAI, 500), + ]) + .with_referrer_shares(vec![(BOB, 5_000_000_000_000), (ALICE, 15_000_000_000_000)]) + .with_assets(vec![DAI]) + // Price present so the converter reaches the min-amount check (not the no-price branch, + // whose error the claim loop happens to tolerate). + .with_conversion_price((HDX, DAI), EmaPrice::new(1_000_000_000_000, 1_000_000_000_000_000_000)) + .with_convert_min_amount(1_000) + .build() + .execute_with(|| { + assert_eq!(PendingConversions::::count(), 1); + + // Bob can still claim his share despite the un-convertible dust asset. + assert_ok!(Referrals::claim_rewards(RuntimeOrigin::signed(BOB))); + + // Bob received his portion of the HDX reward reserve. + assert_eq!(Tokens::free_balance(HDX, &BOB), 5_000_000_000_000); + // The dust entry is dropped rather than left to block future claims. + assert_eq!(PendingConversions::::count(), 0); + // The skipped conversion is surfaced as an event, not silently swallowed. + assert!(System::events().iter().any(|r| matches!( + &r.event, + RuntimeEvent::Referrals(Event::ConversionFailed { asset_id, .. }) if *asset_id == DAI + ))); + }); +} + #[test] fn claim_rewards_should_calculate_correct_portion_when_claimed() { ExtBuilder::default() diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index 42cb4ede0..bc8a98828 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "426.0.0" +version = "427.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index 3da81766b..a1046eebe 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -129,7 +129,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: Cow::Borrowed("hydradx"), impl_name: Cow::Borrowed("hydradx"), authoring_version: 1, - spec_version: 426, + spec_version: 427, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1,