Allow COA.withdraw call to run when there is a possible rounding error
#6877
Conversation
|
cc @sisyphusSmiling Can you take a lot at this approach, and the added test cases? |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6877 +/- ##
==========================================
+ Coverage 41.11% 41.13% +0.01%
==========================================
Files 2116 2116
Lines 185749 185759 +10
==========================================
+ Hits 76378 76413 +35
+ Misses 102954 102935 -19
+ Partials 6417 6411 -6
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
m-Peter
force-pushed
the
mpeter/withdraw-balance-with-rounding
branch
2 times, most recently
from
9d80c61 to
1f5fbae
Compare
fvm/evm/impl/impl.go
Outdated
| panic(types.ErrInvalidBalance) | ||
| } | ||
|
|
||
| if types.BalanceInAttFlowValidForFlowVault(amountValue.BigInt) { |
There was a problem hiding this comment.
Based on the name, I would expect true here to be valid. But it seems the check results in a panic so I'm probably misunderstanding the meaning of BalanceInAttFlowValidForFlowVault. What is being checked here?
There was a problem hiding this comment.
@sisyphusSmiling My bad 😅 The name and the logic didn't match the intention.
I have updated it in f0e23ff.
This method basically checks if the requested withdrawal amount in attoFlow, can be represented in a FlowToken vault. The smallest allowed withdrawal amount is 1e^10 attoFlow, which is 1e^-8 Flow (or 0.00000001).
I have chosen to panic on such a case, as there's no reason to silently allow such cases.
|
Thanks for the changes @m-Peter! The test cases and behavior look good to me, just left one question. |
In this case, we perform the withdrawal, by simply truncating the remainder that cannot fit in a Flow token vault.
m-Peter
force-pushed
the
mpeter/withdraw-balance-with-rounding
branch
from
1f5fbae to
f0e23ff
Compare
m-Peter
requested review from
janezpodhostnik,
zhangchiqing and
a team
as code owners
Closes #6856
In this case, we perform the withdrawal, by simply truncating the remainder that cannot fit in a Flow token vault.
When the attoFlow balance is too small to fit in a Flow vault, we panic with a dedicated error message.