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

[BREAKING] Refactor: remove vault collateral interface #781

Merged
merged 2 commits into from
Nov 29, 2022

Conversation

sander2
Copy link
Member

@sander2 sander2 commented Nov 18, 2022

Make vaults use the nomination interface instead of a dedicated vault-collateral interface. In draft since this will require a migration that I still have to write after further consideration, this shouldn't require a migration since we're still using the same storage items.

For downstream consumers, the required changes are:

  • replace all calls to vault_registry.deposit_collateral and vault_registry.withdraw_collateral with nomination.deposit_collateral and nomination.withdraw_collateral respectively.

@sander2 sander2 force-pushed the refactor/remove-vault-collateral branch from 47750de to 140335e Compare November 18, 2022 14:11
@sander2 sander2 marked this pull request as ready for review November 18, 2022 14:12
@sander2 sander2 changed the title Refactor: remove vault collateral interface [BREAKING] Refactor: remove vault collateral interface Nov 18, 2022
Copy link
Member

@gregdhill gregdhill left a comment

Choose a reason for hiding this comment

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

This removes a bunch of duplication so I like it :)

@@ -31,6 +31,7 @@ fee = { path = "../fee", default-features = false }
security = { path = "../security", default-features = false }
currency = { path = "../currency", default-features = false }
vault-registry = { path = "../vault-registry", default-features = false }
nomination = { path = "../nomination", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

This can be a dev-dependency right?

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.

2 participants