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

Introduce OnNewTokenAccount and OnKilledTokenAccount Callbacks #754

Merged
merged 12 commits into from
Jun 7, 2022

Conversation

apopiak
Copy link
Contributor

@apopiak apopiak commented May 29, 2022

It would be useful for some features to be able to react to account creation in the tokens pallet that includes the currency for which an account was created.
This PR thus introduces an OnNewTokenAccount (and corresponding OnKilledTokenAccount) callback to the pallet for other pallets to integrate with.

@apopiak
Copy link
Contributor Author

apopiak commented May 29, 2022

Aside: I was confused I had to include the changes in 34f50fc as they did not seem to have anything to do with this PR. Broken master?

@xlc
Copy link
Member

xlc commented May 29, 2022

Aside: I was confused I had to include the changes in 34f50fc as they did not seem to have anything to do with this PR. Broken master?

Update your Cargo.toml. That's due to bumping Substrate version

@@ -643,6 +643,24 @@ impl<AccountId, CurrencyId, Balance> OnDust<AccountId, CurrencyId, Balance> for
fn on_dust(_: &AccountId, _: CurrencyId, _: Balance) {}
}

/// Handler for a newly created account
pub trait OnNewTokenAccount<AccountId, CurrencyId> {
Copy link
Member

Choose a reason for hiding this comment

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

I don't really like the idea of bunch different traits for each individual purposes.

I think we can just reuse Happened<(&AccountId, CurrencyId)> instead.

Otherwise we can have trait OnAccountUpdated { fn updated }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have a look. Not sure how I would get around the who.clone() induced by the change to Happened

@xlc xlc requested review from zqhxuyuan and shaunxw May 30, 2022 10:08
@xlc
Copy link
Member

xlc commented Jun 6, 2022

can you revert the can_deposit change and fix the build?

@zjb0807
Copy link
Contributor

zjb0807 commented Jun 7, 2022

CI failed, Blocked: paritytech/substrate#11608

@zjb0807
Copy link
Contributor

zjb0807 commented Jun 7, 2022

@apopiak please merge master to fix the CI.

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.

5 participants