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

FI v2: refactor without states #1698

Merged
merged 78 commits into from
Jan 29, 2024
Merged

FI v2: refactor without states #1698

merged 78 commits into from
Jan 29, 2024

Conversation

lemunozm
Copy link
Contributor

@lemunozm lemunozm commented Jan 22, 2024

Description

The main idea of this PR is to remove states to easier the adoption of #1674.

Crate organization

  • swaps.rs: It only focuses on making swaps. Doesn't know about any Info type or info Storage. It only reads the storage related to the swap ids. Actually, this file could be moved to its own pallet with the storage Ids if we need swapping for other purposes in the future.
  • entities.rs: Defines the InvestmentInfo and RedemptionInfo with some methods that mutate their fields. Only these methods can mutate the internal states of the Info types. Higher layers should only read the content and never mutate Info fields directly. This is the low-level logic.
  • impls.rs: The interface to handle FI, through the pallet or through the Hooks. The higher logic level "easier to follow"

Pending work

@lemunozm lemunozm self-assigned this Jan 22, 2024
@lemunozm
Copy link
Contributor Author

Currently compiling with cargo check -p pallet-foreign-investments

@lemunozm lemunozm changed the title PoC: FI: without states FI: refactor without states Jan 26, 2024
@lemunozm lemunozm added P7-asap Issue should be addressed in the next days. I6-refactoring Code needs refactoring. labels Jan 26, 2024
@lemunozm lemunozm changed the title FI: refactor without states FI v2: refactor without states Jan 26, 2024
@lemunozm lemunozm marked this pull request as ready for review January 26, 2024 08:19
wischli
wischli previously approved these changes Jan 29, 2024
Copy link
Contributor

@wischli wischli left a comment

Choose a reason for hiding this comment

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

Nothing to complain, not even a single nit. Looks really good and clean. Thanks so much for all your great work!

Comment on lines +77 to +85
pub type SwapOf<T> = Swap<<T as Config>::Balance, <T as Config>::CurrencyId>;
pub type TrancheIdOf<T> = <<T as Config>::PoolInspect as cfg_traits::PoolInspect<
<T as frame_system::Config>::AccountId,
<T as Config>::CurrencyId,
>>::TrancheId;
pub type PoolIdOf<T> = <<T as Config>::PoolInspect as cfg_traits::PoolInspect<
<T as frame_system::Config>::AccountId,
<T as Config>::CurrencyId,
>>::PoolId;
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for trimming down the pallet even more!

* tests: fix foreign investments v2

* chore: expose ForeignIdToSwapId storage

* fix: remove events

* chore: add swap_id_from

* refactor: use give_pool_role
@lemunozm lemunozm mentioned this pull request Jan 29, 2024
4 tasks
@lemunozm lemunozm enabled auto-merge (squash) January 29, 2024 15:26
@lemunozm lemunozm merged commit d781a10 into main Jan 29, 2024
9 checks passed
@lemunozm
Copy link
Contributor Author

Thanks again @wischli for all your work, help, support, and eyes here, and for taking care of all border cases in the previous implementation! 🙌🏻

@lemunozm
Copy link
Contributor Author

Let's keeping the previous branch by now 🙏🏻

wischli added a commit that referenced this pull request Jan 29, 2024
* increase & decrease investments

* add rest of methods

* remove unused code

* generalize swapping

* Create DecreaseForeignInvestment event

* ForeignInvestment trait fully implemented

* notify status for fulfilling swaps

* simplify storages

* implements collected part

* minor cleanings and renamings

* minor rename

* collection refactors

* protect agains 0 investment changes

* Events computed in both swap directions

* protect againts different foreign currency

* minor comment

* add some notes

* fix notify_swap_done

* BaseInfo to extract common parts

* minor rename

* Remove inverse swap optimization

* William suggestions

* fix redemption tranche tokens

* fix collected redemption

* minor notes

* simplify and collected redemption

* fix decrease and fix error in amount denominations

* fix redemption condition

* minor fixes to mirror legacy implementation

* total to pending tranche tokens

* determine pool currency from investment id

* apply William review

* Use apply swap to cancel inverse orders

* fix decrement issue

* fix apply_swap issue

* minor fix

* minor foreign trait simplification

* minor simplifications. mock compiling

* add testing for swaps

* add some tests. Fix issue when decreasing after increasing

* add tests, remove increment/decrement check

* fix amount_reminder for decrease msng, adding the pending increasing amount

* add error to handle too much decrease

* fulfill test working

* decrease after fulfill works

* remove inverse swap notifications

* Extract notifications from mutable closures

* full investment test

* collect investment working

* minor refactorizations

* logic moved to Info structures

* split in files

* add complex decrease test

* correct pending redemption calculation

* better swap done split

* Improve readability for notifications/swaps

* minor renames

* add some docs

* better naming SwapDone

* remove field from InvestmentInfo, computed now from the state

* bypassing swapping for the same currencies

* test collect redemptions with swaps

* nit

* chore: enable runtime compilation

* chore: expose FI hook impls

* clean unused dependencies

* remove old benchmark

* reduce Config associated types

* adapt other tests to last trait changes

* fix investment mock

* fix clippy issues

* fix complex type clippy issues

* avoid failing the notification when the investment is not found

* fix redemption collection denomination

* fix clippy

* tests: fix FI v2 integration  (#1700)

* tests: fix foreign investments v2

* chore: expose ForeignIdToSwapId storage

* fix: remove events

* chore: add swap_id_from

* refactor: use give_pool_role

---------

Co-authored-by: William Freudenberger <w.freude@icloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I6-refactoring Code needs refactoring. P7-asap Issue should be addressed in the next days.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants