review comments

Author: enthusiastmartin

integration-tests/src/fee_processor.rs

@@ -132,18 +132,20 @@ pub mod pallet {
 		fn on_idle(_n: BlockNumberFor<T>, remaining_weight: Weight) -> Weight {
 			let convert_weight = T::WeightInfo::convert();
 
-			if remaining_weight.ref_time() < convert_weight.ref_time() {
+			// Budget conversions against BOTH weight dimensions: each `do_convert` runs a real
+			// Omnipool sell with non-trivial proof size, so gating on `ref_time` alone could
+			// overweight the block's PoV. A zero-cost dimension imposes no limit.
+			let fits = |budget: u64, cost: u64| if cost == 0 { u64::MAX } else { budget / cost };
+			let max_conversions = fits(remaining_weight.ref_time(), convert_weight.ref_time())
+				.min(fits(remaining_weight.proof_size(), convert_weight.proof_size()))
+				.min(T::MaxConversionsPerBlock::get() as u64);
+
+			if max_conversions == 0 {
 				return Weight::zero();
 			}
 
 			let mut used_weight = Weight::zero();
 
-			let max_conversions = remaining_weight
-				.ref_time()
-				.checked_div(convert_weight.ref_time())
-				.unwrap_or(0)
-				.min(T::MaxConversionsPerBlock::get() as u64);
-
 			for asset_id in PendingConversions::<T>::iter_keys().take(max_conversions as usize) {
 				match Self::do_convert(asset_id) {
 					Ok(_) => {}

pallets/fee-processor/src/lib.rs

@@ -18,6 +18,7 @@
 mod claim;
 mod flow;
 mod link;
+mod migration;
 mod mock_amm;
 mod register;
 mod tiers;

pallets/referrals/src/tests.rs

@@ -0,0 +1,67 @@
+use crate::tests::*;
+
+use crate::migration::migrate_to_accumulator;
+use sp_core::U256;
+
+const PRECISION: u128 = crate::pallet::PRECISION;
+
+// A legacy chain (pre-accumulator) holds `TotalShares > 0` and a funded pot, but the new
+// `RewardPerShare` accumulator defaults to zero. Without the migration a claim computes
+// `shares * 0 / PRECISION - 0 = 0` and pays nothing; the migration must seed the accumulator
+// so a legacy holder claims exactly the old `shares * (pot - seed) / total_shares`.
+#[test]
+fn migrate_to_accumulator_should_preserve_legacy_claim_amount() {
+	let seed = ONE;
+	let alice_shares = 3_000_000_000u128;
+	let rewards = 600 * ONE;
+	let pot = Referrals::pot_account_id();
+
+	// The builder mints `seed` into the pot itself, so only the claimable rewards are endowed here.
+	ExtBuilder::default()
+		.with_seed_amount(seed)
+		.with_referrer_shares(vec![(ALICE, alice_shares)])
+		.with_endowed_accounts(vec![(pot, HDX, rewards)])
+		.build()
+		.execute_with(|| {
+			// Legacy state: accumulator at its default, no debt, shares + funded pot present.
+			assert!(RewardPerShare::<Test>::get().is_zero());
+			assert_eq!(TotalShares::<Test>::get(), alice_shares);
+
+			migrate_to_accumulator::<Test>();
+
+			let expected_rps = U256::from(rewards) * U256::from(PRECISION) / U256::from(alice_shares);
+			assert_eq!(RewardPerShare::<Test>::get(), expected_rps);
+
+			let alice_before = Tokens::free_balance(HDX, &ALICE);
+			assert_ok!(Referrals::claim_rewards(RuntimeOrigin::signed(ALICE)));
+			let claimed = Tokens::free_balance(HDX, &ALICE) - alice_before;
+
+			assert_eq!(
+				claimed, rewards,
+				"legacy holder must claim pot-minus-seed after the accumulator migration"
+			);
+		});
+}
+
+// The migration must be a no-op when there are no shares (fresh chain / pre-adoption):
+// dividing the pot by `total_shares == 0` is undefined, so the accumulator stays at zero.
+#[test]
+fn migrate_to_accumulator_should_be_noop_when_no_shares() {
+	let seed = ONE;
+	let pot = Referrals::pot_account_id();
+
+	ExtBuilder::default()
+		.with_seed_amount(seed)
+		.with_endowed_accounts(vec![(pot, HDX, 500 * ONE)])
+		.build()
+		.execute_with(|| {
+			assert_eq!(TotalShares::<Test>::get(), 0);
+
+			migrate_to_accumulator::<Test>();
+
+			assert!(
+				RewardPerShare::<Test>::get().is_zero(),
+				"accumulator must stay zero when there are no shares"
+			);
+		});
+}

pallets/referrals/src/tests/migration.rs

@@ -2342,7 +2342,14 @@ impl hydradx_traits::fee_processor::FeeReceiver<AccountId, Balance> for Referral
 	type Error = sp_runtime::DispatchError;
 
 	fn destination() -> AccountId {
-		pallet_referrals::Pallet::<Runtime>::pot_account_id()
+		// With no shareholders the accumulator can't attribute a deposit (it divides by
+		// `TotalShares`), so route the slice to the treasury instead of stranding it in the pot
+		// where nobody could ever claim it (audit 2026-06-02, finding 5).
+		if pallet_referrals::Pallet::<Runtime>::total_shares() == 0 {
+			TreasuryAccount::get()
+		} else {
+			pallet_referrals::Pallet::<Runtime>::pot_account_id()
+		}
 	}
 
 	fn percentage() -> Permill {
@@ -2354,6 +2361,11 @@ impl hydradx_traits::fee_processor::FeeReceiver<AccountId, Balance> for Referral
 	}
 
 	fn on_fee_received(amount: Balance) -> Result<(), Self::Error> {
+		// Mirror `destination()`: with no shareholders the slice went to the treasury, not the
+		// pot, so there is nothing to accrue into the accumulator.
+		if pallet_referrals::Pallet::<Runtime>::total_shares() == 0 {
+			return Ok(());
+		}
 		pallet_referrals::Pallet::<Runtime>::on_hdx_deposited(amount)
 	}
 }

runtime/hydradx/src/assets.rs

@@ -0,0 +1,89 @@
+// This file is part of HydraDX-node
+
+// Copyright (C) 2020-2025  Intergalactic, Limited (GIB).
+// SPDX-License-Identifier: Apache-2.0
+
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#![cfg(feature = "runtime-benchmarks")]
+
+use crate::{AccountId, AssetId, Balance, Currencies, FeeProcessor, Omnipool, Runtime};
+
+use crate::benchmarking::set_period;
+use frame_benchmarking::account;
+use frame_system::RawOrigin;
+use orml_benchmarking::runtime_benchmarks;
+use orml_traits::MultiCurrency;
+use sp_runtime::DispatchResult;
+use sp_std::vec;
+
+const HDX: AssetId = 0;
+const DAI: AssetId = 2;
+const ONE: Balance = 1_000_000_000_000;
+
+fn fund(who: AccountId, asset: AssetId, amount: Balance) -> DispatchResult {
+	Currencies::update_balance(RawOrigin::Root.into(), who, asset, amount as i128)
+}
+
+runtime_benchmarks! {
+	{ Runtime, pallet_fee_processor }
+
+	// Worst case: a non-HDX pot balance is sold via Omnipool and distributed to all four
+	// receivers (the slice routed to the referrals receiver is the heaviest leg).
+	convert {
+		crate::benchmarking::omnipool_liquidity_mining::initialize_omnipool(None)?;
+
+		// Pre-create every account the conversion touches (receiver pots, treasury, referrals
+		// pot) so that distributing to a not-yet-existing account — including the tiny fee-on-fee
+		// slices from the Omnipool sell itself — never trips ED. On the live chain these are all
+		// genesis-funded.
+		let seed = 1_000 * ONE;
+		for p in [
+			FeeProcessor::pot_account_id(),
+			pallet_staking::Pallet::<Runtime>::pot_account_id(),
+			pallet_gigahdx::Pallet::<Runtime>::gigapot_account_id(),
+			pallet_gigahdx_rewards::Pallet::<Runtime>::reward_accumulator_pot(),
+			pallet_referrals::Pallet::<Runtime>::pot_account_id(),
+			crate::Treasury::account_id(),
+		] {
+			fund(p, HDX, seed)?;
+		}
+
+		// Warm the EMA oracle with HDX/DAI trades across a period — the conversion's Omnipool
+		// sell and the dynamic fee both read the oracle. Each HDX->DAI sell also accrues a DAI
+		// fee into the fee-processor pot (the non-HDX path), which is what `convert` consumes.
+		let trader: AccountId = account("trader", 0, 0);
+		fund(trader.clone(), HDX, 100_000 * ONE)?;
+		Omnipool::sell(RawOrigin::Signed(trader.clone()).into(), HDX, DAI, 100 * ONE, 0)?;
+		set_period(24);
+		Omnipool::sell(RawOrigin::Signed(trader.clone()).into(), HDX, DAI, 100 * ONE, 0)?;
+
+		let pot = FeeProcessor::pot_account_id();
+		fund(pot.clone(), DAI, 100 * ONE)?;
+
+		let caller: AccountId = account("caller", 1, 0);
+		fund(caller.clone(), HDX, seed)?;
+	}: _(RawOrigin::Signed(caller), DAI)
+	verify {
+		assert_eq!(
+			<Currencies as MultiCurrency<AccountId>>::free_balance(DAI, &FeeProcessor::pot_account_id()),
+			0,
+			"the entire pot balance must be converted"
+		);
+	}
+}
+
+// NOTE: no `impl_benchmark_test_suite!` here. `convert` drives a real Omnipool sell whose
+// downstream fee distribution touches accounts that only exist in the full chain genesis, so it
+// is generated/validated via the benchmarking CLI against the runtime genesis rather than a
+// minimal stand-alone externalities.

runtime/hydradx/src/benchmarking/fee_processor.rs

@@ -6,6 +6,7 @@ pub mod duster;
 pub mod dynamic_evm_fee;
 pub mod ema_oracle;
 pub mod evm_accounts;
+pub mod fee_processor;
 pub mod multi_payment;
 pub mod omnipool;
 pub mod omnipool_liquidity_mining;

runtime/hydradx/src/benchmarking/mod.rs

@@ -139,7 +139,7 @@ fn initialize_yield_farm(owner: AccountId, id: GlobalFarmId, asset: AssetId) ->
 	OmnipoolLiquidityMining::create_yield_farm(RawOrigin::Signed(owner).into(), id, asset, FixedU128::one(), None)
 }
 
-fn initialize_omnipool(additional_asset: Option<AssetId>) -> DispatchResult {
+pub fn initialize_omnipool(additional_asset: Option<AssetId>) -> DispatchResult {
 	let stable_amount: Balance = 1_000_000_000_000_000u128;
 	let native_amount: Balance = 1_000_000_000_000_000u128;
 	let stable_price: FixedU128 = FixedU128::from((1, 2));

runtime/hydradx/src/benchmarking/omnipool_liquidity_mining.rs

@@ -130,7 +130,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	impl_name: Cow::Borrowed("hydradx"),
 	authoring_version: 1,
 	spec_version: 425,
-	impl_version: 0,
+	impl_version: 1,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,
 	system_version: 1,
@@ -379,6 +379,7 @@ mod benches {
 		[pallet_omnipool, benchmarking::omnipool::Benchmark]
 		[pallet_route_executor, benchmarking::route_executor::Benchmark]
 		[pallet_dca, benchmarking::dca::Benchmark]
+		[pallet_fee_processor, benchmarking::fee_processor::Benchmark]
 		[pallet_xyk, benchmarking::xyk::Benchmark]
 		[pallet_dynamic_evm_fee, benchmarking::dynamic_evm_fee::Benchmark]
 		[pallet_xyk_liquidity_mining, benchmarking::xyk_liquidity_mining::Benchmark]

runtime/hydradx/src/lib.rs

@@ -16,11 +16,13 @@
 use crate::Runtime;
 
 mod cleanup_hyperbridge;
+mod referrals_accumulator;
 
 // New migrations which need to be cleaned up after every Runtime upgrade
 pub type UnreleasedSingleBlockMigrations = (
 	pallet_ema_oracle::migrations::v2::MigrateV1ToV2<Runtime, crate::assets::BifrostAccount>,
 	cleanup_hyperbridge::CleanupHyperbridge,
+	referrals_accumulator::InitReferralsAccumulator,
 );
 
 // These migrations can run on every runtime upgrade