Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support assets with xyk dot pools as fee asset #852

Merged
merged 108 commits into from
Sep 23, 2024

Conversation

dmoka
Copy link
Contributor

@dmoka dmoka commented Jun 27, 2024

This PR is about:

*A non-fee payment asset is any asset whose price is not calculated within the multi-payment pallet. For example we also want to support WUD which is a sufficient asset but not tracked in multi pallets.

TODO:

  • adjust benchmark
  • rebench on reference machine
  • maual test
  • check if dca works with buynig insuff, so HDX should be payed, check itnegration test about it
  • add support for insufficient general fee payment currency
  • refactor so the pallets don't depend on XYK and AssetReg
  • rebenchmark
  • manual test
  • modify the check and if check if asset is feee asset, if not then we check for the XYK pool
  • adjust ExtrinsicBaseWeight
  • rebenchmark withdraw_fee (used for ExtrinsicBaseWeight) on reference machine
  • manual test the fee value

(Benchmark withdraw_fee and check if it is not bigger than ExtrinsicBaseWeight. If so, then adjust. "The base weight contains all the shit for validating tx and doing all the on initialize and other stuff for every transaction so we should probably increase it anyway... But there should be a guide on how to bench this from shawn somewhere I think")

@dmoka dmoka self-assigned this Jun 27, 2024
Copy link

github-actions bot commented Jun 27, 2024

Crate versions that have been updated:

  • runtime-integration-tests: v1.23.4 -> v1.23.5
  • pallet-dca: v1.5.1 -> v1.6.0
  • pallet-democracy: v4.3.0 -> v4.3.1
  • pallet-lbp: v4.8.4 -> v4.8.5
  • pallet-liquidity-mining: v4.4.0 -> v4.4.1
  • pallet-transaction-multi-payment: v10.0.4 -> v10.1.0
  • pallet-xyk: v6.4.5 -> v6.5.1
  • pallet-xyk-liquidity-mining: v1.1.12 -> v1.1.13
  • hydradx-runtime: v257.0.0 -> v258.0.0
  • hydradx-traits: v3.6.0 -> v3.7.0

Runtime version has been increased.

@dmoka dmoka marked this pull request as ready for review July 8, 2024 07:05
@dmoka dmoka changed the title feat: support insufficient asset as fee in DCA feat: support insufficient asset as fee Jul 9, 2024
@dmoka dmoka changed the title feat: support insufficient asset as fee feat: support insufficient asset as fee (DCA + general) Jul 12, 2024
@mrq1911 mrq1911 marked this pull request as draft August 23, 2024 13:45
@dmoka dmoka marked this pull request as ready for review August 30, 2024 19:08
…icient-asset-in-dca

# Conflicts:
#	Cargo.lock
#	integration-tests/Cargo.toml
#	traits/Cargo.toml
…icient-asset-in-dca

# Conflicts:
#	Cargo.lock
#	integration-tests/Cargo.toml
#	pallets/liquidity-mining/Cargo.toml
#	pallets/xyk/Cargo.toml
.ok_or(ArithmeticError::Overflow)?;
Self::unallocate_amount(schedule_id, schedule, effective_amount_in)?;

let fee_in_dot = Self::convert_to_polkadot_native_asset(Self::weight_to_fee(weight_to_charge))?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need to do this again ?

should not this be equal to fee_amount_in_sold_asset?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fee_amount_in_sold_asset is different from fee_in_dot.

Because the amount_in (so DCA fee asset) is not necessarly DOT.

But we need to calculate the fee_in_dot, so we know how much we should do a buy trade for, within the FeeAsset-DOT xyk pool

pallets/dca/src/lib.rs Outdated Show resolved Hide resolved
@dmoka dmoka merged commit c048260 into master Sep 23, 2024
7 checks passed
@dmoka dmoka deleted the feat/support-insufficient-asset-in-dca branch September 23, 2024 14:07
@dmoka dmoka mentioned this pull request Sep 27, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow fee payment with insufficient asset
2 participants