Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pallets/referrals/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "pallet-referrals"
version = "1.6.0"
version = "1.6.1"
authors = ['GalacticCouncil']
edition = "2021"
license = "Apache-2.0"
Expand Down
32 changes: 20 additions & 12 deletions pallets/referrals/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,11 @@ pub mod pallet {
from: AssetAmount<T::AssetId>,
to: AssetAmount<T::AssetId>,
},
/// A pending asset could not be converted; skipped, not blocking.
ConversionFailed {
asset_id: T::AssetId,
reason: DispatchError,
},
/// Rewards claimed.
Claimed {
who: T::AccountId,
Expand Down Expand Up @@ -482,20 +487,18 @@ pub mod pallet {
let who = ensure_signed(origin)?;
for (asset_id, _) in PendingConversions::<T>::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::<T>::ConversionMinTradingAmountNotReached.into()
&& error != Error::<T>::ConversionZeroAmountReceived.into()
{
return Err(error);
}
) {
Self::deposit_event(Event::ConversionFailed {
asset_id: asset_id.clone(),
reason,
});
}
PendingConversions::<T>::remove(asset_id);
}
Expand Down Expand Up @@ -623,12 +626,17 @@ pub mod pallet {
for asset_id in PendingConversions::<T>::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::<T>::remove(asset_id);
actual_converts += 1;
}
Expand Down
17 changes: 17 additions & 0 deletions pallets/referrals/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ thread_local! {
pub static TIER_VOLUME: RefCell<HashMap<Level, Option<Balance>>> = RefCell::new(HashMap::default());
pub static TIER_REWARDS: RefCell<HashMap<Level, FeeDistribution>> = RefCell::new(HashMap::default());
pub static SEED_AMOUNT: RefCell<Balance> = RefCell::new(Balance::zero());
// Mirrors runtime `MinTradingLimit`: reject sub-minimum amounts. 0 = disabled.
pub static CONVERT_MIN_AMOUNT: RefCell<Balance> = RefCell::new(Balance::zero());
}

construct_runtime!(
Expand Down Expand Up @@ -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![],
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -364,6 +376,11 @@ impl Convert<AccountId, AssetId, Balance> for AssetConvert {
asset_to: AssetId,
amount: Balance,
) -> Result<Balance, Self::Error> {
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::<Test>::ConversionMinTradingAmountNotReached)?;
Expand Down
34 changes: 34 additions & 0 deletions pallets/referrals/src/tests/claim.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Test>::pot_account_id(), HDX, 20_000_000_000_000),
// 500 raw units of DAI — below the converter's min trading limit (1_000).
(Pallet::<Test>::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))
Comment on lines +88 to +90
.with_convert_min_amount(1_000)
.build()
.execute_with(|| {
assert_eq!(PendingConversions::<Test>::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::<Test>::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()
Expand Down
2 changes: 1 addition & 1 deletion runtime/hydradx/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "hydradx-runtime"
version = "426.0.0"
version = "427.0.0"
authors = ["GalacticCouncil"]
edition = "2021"
license = "Apache 2.0"
Expand Down
2 changes: 1 addition & 1 deletion runtime/hydradx/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
Loading