feat: minimum chunk size setting #5314

j4m1ef0rd · 2024-10-07T02:17:16Z

Pull Request

Closes: PRO-1703

Checklist

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

I am confident that the code works.
I have written sufficient tests.
I have written and tested required migrations.
I have updated documentation where appropriate.

Summary

Added a storage item for MinimumChunkSize
- Added it to the pallet config so we can update the setting for each asset.
  - Added the new setting to the pallet config unit test.
Added logic in the init_swap_request function that lowers the number of chunks so it will always be >= to the minimum.
- Added a unit test to cover this new logic
Added a minimum_chunk_size AssetMap to the SwappingEnvironment
- Updated the test snapshot

state-chain/pallets/cf-swapping/src/lib.rs

codecov · 2024-10-07T02:29:46Z

Codecov Report

Attention: Patch coverage is 68.88889% with 14 lines in your changes missing coverage. Please review.

Project coverage is 70%. Comparing base (839eb60) to head (9a22c81).
Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
state-chain/custom-rpc/src/lib.rs	67%	6 Missing ⚠️
state-chain/pallets/cf-swapping/src/lib.rs	82%	4 Missing ⚠️
state-chain/runtime/src/lib.rs	0%	0 Missing and 4 partials ⚠️

Additional details and impacted files

@@          Coverage Diff           @@
##            main   #5314    +/-   ##
======================================
- Coverage     70%     70%    -0%     
======================================
  Files        489     489            
  Lines      87874   87776    -98     
  Branches   87874   87776    -98     
======================================
- Hits       61939   61772   -167     
- Misses     22630   22694    +64     
- Partials    3305    3310     +5

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

msgmaxim · 2024-10-07T03:22:33Z

state-chain/pallets/cf-swapping/src/lib.rs

@@ -1964,6 +1983,18 @@ pub mod pallet {
 				swap_amount
 			};

+			// Restrict the number of chunks based on the minimum chunk size.
+			let dca_params = dca_params.map(|mut dca_params| {


@zoheb391 just double checking that the frontend will be able to handle this (changing the number of dca chunks based on the amount). I imagine if you are reading the number of chunks from SwapRequested rather than by looking at the channel, this shouldn't be a problem?

@msgmaxim The front-end does not have an option to change dca interval params. It is our quoting endpoint that decides on the dca params. if the swap amount is under the configuration amount (~3000usd) we only use 1 chunk so it won't be an issue on our front-end or in our sdk if they use it like we recommend.

@zoheb391 The issue here is that the protocol might change the parameters: if the chosen number of chunks would result in chunks that are smaller than some minimum, we reduce the number of chunks accordingly. This can still happen with our FE. Example: User requests a quote for 10BTC and gets a dca quote for 1000 chunks. But then they only send 0.1BTC. In this case, the protocol would adjust the number of chunks to something smaller than 1000 to ensure we don't waste resources on 1000 tiny swaps.

ah i see now. yeah we're using the channel for dca params. So that might be a problem for us. We will need to make that switch.

i'm guessing interval remains unchanged though?

anyways will track the issue and make the necessary changes 👍

i'm guessing interval remains unchanged though?

Yes, no change to the interval but I would still recommend using the swap request details rather than the channel details for this.

msgmaxim

LGTM, but let's see whether or not we want a migration here to set the initial limits.

state-chain/pallets/cf-swapping/src/lib.rs

dandanlen · 2024-10-07T08:09:29Z

state-chain/pallets/cf-swapping/src/lib.rs

+	#[pallet::storage]
+	#[pallet::getter(fn minimum_chunk_size)]
+	pub type MinimumChunkSize<T: Config> =
+		StorageMap<_, Twox64Concat, Asset, AssetAmount, ValueQuery>;


Doesn't really need to be stored as an AssetAmount. u32 is plenty, and then we don't have the confusion of returning a NumberOrHex from the rpc.

Some assets like ETH have 18 decimal places. Even if we set this so something small like 0.001 ETH, it would still be a large enough number that it won't fit in u32, right?

Yes, ignore me. For some reason I was confusing chunk size with number of chunks. AssetAmount makes total sense.

state-chain/custom-rpc/src/lib.rs

state-chain/runtime/src/runtime_apis.rs

…lana-ccm * origin/main: chore: update polkadot sdk 1.15.2, rust to 2024-07-31 (#5306) chore: add sol min_prio_fee to ingress and egress estimation (#5323) docs: fix log filtering in readme (#5319) feat: cleanup aborted broadcasts (#5301) fix: increase init timeout for backup test (#5317) feat: minimum chunk size setting (#5314) chore: add egress fee for token account rent (#5315) fix: ensure unused sol deposit channels are closed (#5312)

feat: minimum chunk size setting

9a22c81

j4m1ef0rd added the State Chain label Oct 7, 2024

j4m1ef0rd requested a review from msgmaxim October 7, 2024 02:17

j4m1ef0rd self-assigned this Oct 7, 2024

j4m1ef0rd requested a review from dandanlen as a code owner October 7, 2024 02:17

j4m1ef0rd commented Oct 7, 2024

View reviewed changes

state-chain/pallets/cf-swapping/src/lib.rs Show resolved Hide resolved

msgmaxim reviewed Oct 7, 2024

View reviewed changes

msgmaxim approved these changes Oct 7, 2024

View reviewed changes

dandanlen reviewed Oct 7, 2024

View reviewed changes

dandanlen approved these changes Oct 8, 2024

View reviewed changes

dandanlen added this pull request to the merge queue Oct 8, 2024

Merged via the queue into main with commit 6cd25ce Oct 8, 2024
49 checks passed

dandanlen deleted the feat/minimum-chunk-size branch October 8, 2024 11:31

dandanlen added a commit that referenced this pull request Oct 11, 2024

feat: minimum chunk size setting (#5314)

8ad9eda

dandanlen added a commit that referenced this pull request Oct 14, 2024

feat: minimum chunk size setting (#5314)

5199580

dandanlen added a commit that referenced this pull request Oct 18, 2024

feat: minimum chunk size setting (#5314)

554595e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: minimum chunk size setting #5314

feat: minimum chunk size setting #5314

j4m1ef0rd commented Oct 7, 2024 •

edited by msgmaxim

Loading

codecov bot commented Oct 7, 2024 •

edited

Loading

msgmaxim Oct 7, 2024 •

edited

Loading

zoheb391 Oct 7, 2024

dandanlen Oct 7, 2024 •

edited

Loading

zoheb391 Oct 8, 2024

zoheb391 Oct 8, 2024

zoheb391 Oct 8, 2024

dandanlen Oct 8, 2024

msgmaxim left a comment •

edited

Loading

dandanlen Oct 7, 2024

msgmaxim Oct 7, 2024

dandanlen Oct 8, 2024

feat: minimum chunk size setting #5314

feat: minimum chunk size setting #5314

Conversation

j4m1ef0rd commented Oct 7, 2024 • edited by msgmaxim Loading

Pull Request

Checklist

Summary

codecov bot commented Oct 7, 2024 • edited Loading

Codecov Report

msgmaxim Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dandanlen Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msgmaxim left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j4m1ef0rd commented Oct 7, 2024 •

edited by msgmaxim

Loading

codecov bot commented Oct 7, 2024 •

edited

Loading

msgmaxim Oct 7, 2024 •

edited

Loading

dandanlen Oct 7, 2024 •

edited

Loading

msgmaxim left a comment •

edited

Loading