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

Use truly free balance (liquid) in chain extension #501

Merged

Conversation

gianfra-t
Copy link
Contributor

Closes #483

Uses the reducible_balance fn from the Inspect trait. This is the truly liquid balance see here of orml_tokens and here for pallet_balances. Here frozen has a different meaning but the meaning of liquid is the same judging by how both use it in their transfer_all function.

There is no need to substract from reserved since these free and reserved are already mutually exclusive (see the reserve extrinsic).

Why not use MultiCurrency

Sadly, this trait does not implement the notion of a reducible balance nor of a frozen balance. It does implement this notion in ensure_can_withdraw but this won't actually tell us the amount, so the Inspect trait seems to be the only choice (that I could find).
But this means we need to match between the native and token variants, which is not very ergonomic. Also, the return is encoded to avoid mismatches of Balance type.

@gianfra-t gianfra-t linked an issue Oct 11, 2024 that may be closed by this pull request
@gianfra-t gianfra-t changed the title Use truly free balance Use truly free balance in chain extension Oct 11, 2024
@ebma
Copy link
Member

ebma commented Oct 14, 2024

@gianfra-t is this ready for review? You didn't add reviewers.

@gianfra-t
Copy link
Contributor Author

Yes @ebma it is, but tests are failing (I was waiting for them). I will check if it is related.

@gianfra-t gianfra-t force-pushed the 483-change-chain-extensions-to-return-transferable-balance branch from 2752d4d to 1b7d4fa Compare October 14, 2024 12:04
@gianfra-t
Copy link
Contributor Author

I will ran format afterwards since it will as always complicate the review of actual changes.

@gianfra-t gianfra-t changed the title Use truly free balance in chain extension Use truly free balance (liquid) in chain extension Oct 14, 2024
Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Interesting, I didn't know about the reducible_balance function. Makes sense to use it but unfortunate that it's not implemented on the MultiCurrency trait.

Looks good to me overall 👍

@gianfra-t gianfra-t merged commit 3f45706 into main Oct 15, 2024
1 of 2 checks passed
@gianfra-t gianfra-t deleted the 483-change-chain-extensions-to-return-transferable-balance branch October 15, 2024 14:42
@gianfra-t
Copy link
Contributor Author

I merged this even if the CI showed a failure, because locally when running the last clippy commands they all work. It may have been a one time thing.

@TorstenStueber
Copy link
Member

Is the failing CI due to clippy? Isn't it the Test job that fails with the following error?

Oct 14 13:51:18.446 ERROR parachain_staking::pallet: 💥 keeping old session because of empty collator set!

@gianfra-t
Copy link
Contributor Author

But that has been a very common log we see almost always we start the chain in tests. I doubt it will make the CI fail.

It almost seems like it didn't run the last 3 jobs.

@gianfra-t
Copy link
Contributor Author

Ohhh I checked the logs of the CI again and I see:

System.IO.IOException: No space left on device : '/home/runner/runners/2.320.0/_diag/Worker_20241014-124125-utc.log

So that's it.

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.

Change chain extensions to return transferable balance
3 participants