extend ensure_account_can_withdraw and fix transfer check

[gui1117]

related to #1962

as pointed by @xlc here we don't check if account can withdraw the amount for the TransactionPayment reason.

so it fix it, improve ensure_account_can_withdraw syntax and remove _amount unused parameter from it.

I am not very sure of this, I may be wrong if TransactionPayment reason is considered a subset of Transfer reason.

[xlc]

I never encounter this syntax before. Can I know where I can read more about this syntax?

[gui1117]

in parameter as far as I know it is same as if I have a generic in function T: Into<WithdrawReasons> and then reasons: T I don't have that much material maybe try to google impl trait

[gavofyork]

Incorrect - there should be just one WithdrawReason for any single withdraw,

[xlc]

Let's say I want to create a runtime module that allow user to reserve and transfer in a single runtime call. How can do it? Call ensure_account_can_withdraw twice with different reasons before all the withdraws will not give the correct result.

[gui1117]

actually calling

ensure_account_can_withdraw(who, Reserve | Transfer, new_free_balance)

is equivalent to

ensure_account_can_withdraw(who, Transfer, new_free_balance)
ensure_account_can_withdraw(who, Reserve, new_free_balance)

[gavofyork]

This is not a transaction payment. Transaction payments happen specifically for inclusion of a transaction in the chain.

[gavofyork]

Removal of _value parameter is reasonable. However the rest of changes are in error.

[xlc]

Seems like me and @thiolliere have some misunderstanding of the meaning of transaction payment. @gavofyork can you provide some definitions of it?

To my understanding, transaction payment, is the transaction fee user have to pay to have the transaction included in the chain.
This can be made up with two parts:

1. fee of inclusion of the the transaction, which is calculated with a base fee + transaction size * byte fee
2. fee of additional storage / computation usage, in balance transfer case is account creation fee (addition storage usage) and transfer fee (storage read and write). In smart contract case, it is gas fee.

If my understanding is wrong, that only (1) is counted as transaction payment, then what is the (2) part? It is a fee charged to user. It is not value transferred to another account. Should there be another WithdrawReason?

[gui1117]

Yes that was thought indeed, then maybe (2) is actually just a Transfer reason

(I've been to fast into impl, this should have been discussed in the issue)

[gavofyork]

So WithdrawReason::Transfer is to be read "withdrawing from an account in the duty of executing the transfer function".

It doesn't relate to the balance of the total withdrawal that lands in the transfer destination's account.