feat: use swapping queue when swapping network fee for burn

[j4m1ef0rd]

Pull Request

Closes: PRO-1214

Checklist

Please conduct a thorough self-review before opening the PR.

• I am confident that the code works.
• I have updated documentation where appropriate.

Summary

• Created a SwapQueueApi so the pools pallet can add the swap to the queue
• Created a new swap type that is only used for this network fee swap.
• Moved the FlipToBurn storage item to the swapping pallet.
  • Added migration for this and tested it against perseverance.

[codecov]

Codecov Report

Attention: Patch coverage is 69.23077% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 72%. Comparing base (4540356) to head (9f1cdd0).
Report is 1 commits behind head on main.

Files	Patch %	Lines
state-chain/runtime/src/lib.rs	0%	21 Missing ⚠️
state-chain/traits/src/liquidity.rs	8%	8 Missing and 4 partials ⚠️
state-chain/pallets/cf-swapping/src/lib.rs	94%	3 Missing ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #4584    +/-   ##
======================================
- Coverage     73%     72%    -0%     
======================================
  Files        402     402            
  Lines      67087   66987   -100     
  Branches   67087   66987   -100     
======================================
- Hits       48664   48493   -171     
- Misses     16083   16157    +74     
+ Partials    2340    2337     -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kylezs]

I don't think we want to do the MaximumSwapAmount checks for a network fee swap

[kylezs]

if we add it to the SwapQueue and use Self::swap_into_stable_taking_network_fee(&mut swaps)?; which is what's happening now, then we'll be taking a network fee for the network fee swap. which we don't want either.
We should add a test for this case - after the network swap goes through then we should have nothing left to swap in the Network fee storage

[kylezs]

I'm wondering if it makes sense to have a second queue here. What matters isn't that it's in the SwapQueue specifically - but that it is queued by the delay and it shows in the subscription. I think this is probably the cleaner solution than trying to retrofit this into the existing flow which takes network fees etc. Also means we get the ability to emit a separate event for the network swap (if we wanted to).

CC: @dandanlen ^

[dandanlen]

I don't think we really care that much if the network fee swap also pays a fee. Adding another queue raises more questions than it answers (do bundle it with other swaps? why/why not? what about delays, slippage limits etc...)

Much simpler to treat it like a normal swap - notify the LPs, bundle it with everything else, recover like everything else.

[kylezs]

Yeah I guess it's not a big deal

[dandanlen]

If we really care about this, we can address it in the swap bundling / unbundling logic. Fee Swaps are always USDC->FLIP, so they happen on the second leg. We could just add the network fee swap to the bundle after we take the network fee.

This would be less invasive than adding a whole new queue.

[msgmaxim]

Is it not as simple as just checking swap_type in swap_into_stable_taking_network_fee to see if the fee should be taken? Something like:

fn swap_into_stable_taking_network_fee(
	swaps: &mut Vec<Swap>,
) -> Result<(), BatchExecutionError> {
	Self::do_group_and_swap(swaps, SwapLeg::ToStable)?;

	// Take NetworkFee for all swaps
	for swap in swaps.iter_mut() {
                 // ***** NEW CODE BEGINS *****
		if swap.swap_type == SwapType::NetworkFee {
			// Don't take network fee for network fee swaps
			continue;
		}
                 // ***** NEW CODE END *****
		debug_assert!(
			swap.stable_amount.is_some(),
			"All swaps should have Stable amount set here"
		);
		let stable_amount = swap.stable_amount.get_or_insert_with(Default::default);
		*stable_amount = T::SwappingApi::take_network_fee(*stable_amount);
	}

	Ok(())
}

[dandanlen]

It might be easier/cleaner to just move the FLipToBurn storage into the Swapping pallet rather than extend the SwappingApi with this arbitrary add_flip_to_burn method.

[dandanlen]

This looks fine to me, will leave final approval to @kylezs / @msgmaxim

FWIW (and this is out of scope of this PR) I think we should consider moving all of the network fee stuff into the swapping pallet.

We now seem to have two redundant interfaces:

• SwapQueueApi: only used for network fee swaps?
• SwappingApi::take_network_fee: used to calculate the network fee.

If we make it the responsibility of the swapping pallet to trigger the fee swaps, we can remove both of the above: the swapping pallet can calculate the USDC fee, store it, and then schedule a swap (or even schedule an immediate swap every time we process a batch).

It's not quite trivial because we have swap_with_network_fee which is used in some places for price estimation, and this would need a bit of a refactor. But IMO is the right direction in the longer term. Taking the network fee is something we do for swaps, it should be in the swapping pallet. It's not really a concern of the pools. (@AlastairHolmes maybe you have an opinion here)