-
Notifications
You must be signed in to change notification settings - Fork 536
Add client side check of withdraw-ability #1546
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
base: master
Are you sure you want to change the base?
Add client side check of withdraw-ability #1546
Conversation
| } | ||
| } | ||
|
|
||
| pub fn max_withdraw_amount(&self) -> Result<U256, E> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does with_balance_for in CheckEvmTransaction meet your needs?
|
|
||
| use pallet_evm::OnChargeEVMTransaction; | ||
| let max_withdraw = check_transaction.max_withdraw_amount().map_err(|e| e.0)?; | ||
| <T as pallet_evm::Config>::OnChargeTransaction::can_withdraw(&origin, max_withdraw) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What additional validation does this offer compared to the with_balance_for mentioned above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm rechecking this change we have at Moonbeam's frontier fork to see if the issue was fixed with paritytech/polkadot-sdk#2292. Basically it meant to make a consistent check for the withdraw amount to prevent a corner case where the TX would be included in the pool even if it did not have enough balance to pay the fees.
|
I spent some time evaluating whether this change is still necessary, and the answer is yes.
Here are my notes comparing the behavior of both implementations: Upstream (reducible_balance): Our fork (can_withdraw): As you can see, the behavior and constraints differ significantly. |
This pull request introduces changes to the
frame/ethereumandframe/evmmodules to improve transaction validation and add a new method for checking the maximum withdrawable amount on the client side.