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

Fungible and fungibles adapters should allow account death like burns and mints #7039

Open
franciscoaguirre opened this issue Jan 3, 2025 · 6 comments · May be fixed by #7243
Open

Fungible and fungibles adapters should allow account death like burns and mints #7039

franciscoaguirre opened this issue Jan 3, 2025 · 6 comments · May be fixed by #7243
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T6-XCM This PR/Issue is related to XCM.

Comments

@franciscoaguirre
Copy link
Contributor

The fungible and fungibles adapters set the preservation of the burn operation to Expendable: here and here.
This allows accounts to transfer all their funds (and getting reaped because of it) out of their account.
However, this only works if they're using the WithdrawAsset and DepositAsset instructions, which correspond to burn_from and mint_into.
If TransferAsset is used, as is the case with limited_reserve_assets_transfer, then the transfer function will be called with Preserve.

This distinction is arbitrary and we should change the way TransferAsset is handled to also allow transferring all the funds of an account. It results in confusing errors for users.
If needed, we could add a Hint for changing the default of ALL operations from Expendable to Preserve.

@franciscoaguirre franciscoaguirre added the T6-XCM This PR/Issue is related to XCM. label Jan 3, 2025
@antonkhvorov
Copy link

Very much needed

@franciscoaguirre franciscoaguirre added C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. labels Jan 8, 2025
@dhirajs0
Copy link

dhirajs0 commented Jan 13, 2025

I would like to work on this one.

@franciscoaguirre
Copy link
Contributor Author

@dhirajs0 Thank you for wanting to work on this! Let me know if everything is understood or if you have any questions. I'm here to help.

@dhirajs0
Copy link

@franciscoaguirre

This distinction is arbitrary and we should change the way TransferAsset is handled to also allow transferring all the funds of an account. It results in confusing errors for users.

We can change the Preservation of transfer function from Preserve to Expendable so that the behavior will be consistent with the WithdrawAsset function, as in fungible and fungibles adapter.

If needed, we could add a Hint for changing the default of ALL operations from Expendable to Preserve.

If we add PreservationType { preservation: Preservation } to the Hint and import use frame_support::traits::tokens::Preservation; in the same file(mod.rs), and then while building xcm instruction the set_hints(vec![PreservationType { preservation: Preserve}]) method is called, is this good enough to change the default of ALL operations from Expendable to Preserve or do we need to make more changes for the PreservationType hint to works?

@franciscoaguirre
Copy link
Contributor Author

We can change the Preservation of transfer function from Preserve to Expendable so that the behavior will be consistent with the WithdrawAsset function, as in fungible and fungibles adapter.

Yes, we should do exactly this.

If we add PreservationType { preservation: Preservation } to the Hint and import use frame_support::traits::tokens::Preservation; in the same file(mod.rs), and then while building xcm instruction the set_hints(vec![PreservationType { preservation: Preserve}]) method is called, is this good enough to change the default of ALL operations from Expendable to Preserve or do we need to make more changes for the PreservationType hint to works?

We can't add a new Hint now because XCMv5 is already frozen, released in stable2412. But for XCMv6 for example, we can add a hint PreserveAccounts: bool that just lets you specify whether you want to make sure to preserve accounts. I think it's much simpler if it's just a bool.
But for now, don't worry about the hint, we can add it later.

@dhirajs0
Copy link

Hi @franciscoaguirre,

Thanks for the clarification!

I’ve raised a PR to address this issue. Would greatly appreciate it if you could review it and share your feedback when you have a moment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C1-mentor A task where a mentor is available. Please indicate in the issue who the mentor could be. C2-good-first-issue A task for a first time contributor to become familiar with the Polkadot-SDK. T6-XCM This PR/Issue is related to XCM.
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

3 participants