Skip to content

Check existential deposit when creating an account during swap#694

Merged
dmitrylavrenov merged 11 commits intomasterfrom
check-existential-deposit
Jul 13, 2023
Merged

Check existential deposit when creating an account during swap#694
dmitrylavrenov merged 11 commits intomasterfrom
check-existential-deposit

Conversation

@dmitrylavrenov
Copy link
Contributor

Closes #692

@dmitrylavrenov dmitrylavrenov force-pushed the check-existential-deposit branch from 36da198 to 68a996f Compare July 4, 2023 13:36
Copy link
Contributor

@MOZGIII MOZGIII left a comment

Choose a reason for hiding this comment

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

@dmitrylavrenov
Copy link
Contributor Author

Actually, it seems we don't need to do any checks on the calls:

https://github.com/humanode-network/substrate/blob/master/frame/balances/src/lib.rs#L284-L300

I am pretty sure the currency is supposed to handle that:

https://github.com/humanode-network/substrate/blob/master/frame/balances/src/lib.rs#L1501-L1563 https://github.com/humanode-network/frontier/blob/locked/polkadot-v0.9.38/frame/evm-balances/src/lib.rs#L439-L497

The question is why doesn't it work as expected?

As we don't use the actual transfer, we use default resolve_creating.

@MOZGIII
Copy link
Contributor

MOZGIII commented Jul 4, 2023

Actually, it seems we don't need to do any checks on the calls:
humanode-network/substrate@master/frame/balances/src/lib.rs#L284-L300
I am pretty sure the currency is supposed to handle that:
humanode-network/substrate@master/frame/balances/src/lib.rs#L1501-L1563 humanode-network/frontier@locked/polkadot-v0.9.38/frame/evm-balances/src/lib.rs#L439-L497
The question is why doesn't it work as expected?

As we don't use the actual transfer, we use default resolve_creating.

Ah, got it! This must be the reason it doesn't do those checks. Ok then!

Copy link
Contributor

@MOZGIII MOZGIII left a comment

Choose a reason for hiding this comment

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

Looks good so far

@dmitrylavrenov
Copy link
Contributor Author

Looks good so far

Will add some tests later.

@MOZGIII
Copy link
Contributor

MOZGIII commented Jul 4, 2023

But then, if we follow the trail of calls, we invoke the resolve_creating, which calls deposit_creating, which, in turn, does have the check: https://github.com/humanode-network/frontier/blob/07429a3b559c7277fa20024587f67ae809556054/frame/evm-balances/src/lib.rs#L608C36-L608C36

Copy link
Contributor

@MOZGIII MOZGIII left a comment

Choose a reason for hiding this comment

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

Hold on, we need to figure out exactly what's going on

Copy link
Contributor

@MOZGIII MOZGIII left a comment

Choose a reason for hiding this comment

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

I think we can merge this for now, and clean up this later once we have switched to fungible traits.

@dmitrylavrenov dmitrylavrenov marked this pull request as ready for review July 12, 2023 13:47
@dmitrylavrenov dmitrylavrenov added this pull request to the merge queue Jul 13, 2023
Merged via the queue into master with commit aeb2ab6 Jul 13, 2023
@dmitrylavrenov dmitrylavrenov deleted the check-existential-deposit branch July 13, 2023 06:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Check existential deposit when creating an account during swap

2 participants