Fix eth_estimateGas for hollow account creation #844
Conversation
Nana-EC
left a comment
Nana-EC
left a comment
There was a problem hiding this comment.
LG.
Do the current images support testing this E2E in an acceptance test?
If so let's add one for hollow account creation when the appropriate gas for account creation is provided in a transfer transaction
Nana-EC
requested review from
AlfredoG87,
ebadiere,
georgi-l95 and
lukelee-sl
|
Yes, the current image support is allowing us to write some E2E tests. I think it's a good idea. |
Codecov ReportBase: 73.76% // Head: 73.79% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #844 +/- ##
==========================================
+ Coverage 73.76% 73.79% +0.02%
==========================================
Files 29 29
Lines 1776 1778 +2
Branches 324 325 +1
==========================================
+ Hits 1310 1312 +2
Misses 372 372
Partials 94 94
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
My guess is that there is a small chance of a race condition where we will give the wrong gas estimate for a toAccount that is being create. |
Nana-EC
left a comment
Nana-EC
left a comment
There was a problem hiding this comment.
LG. You can take this out of draft.
Minor nits
Should be fine as network will charge the lower gas amount if the it determines the account is existent by the time one of the relay submitted accounts hits the network. |
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
Signed-off-by: nikolay <[email protected]>
Signed-off-by: georgi-l95 <[email protected]>
georgi-l95
force-pushed
the
fix-estimateGas-for-hollow-account-creation
branch
from
319b26b to
1ef2a60
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
Hollow account creation requires an accurate gas amount response - add a `TX_HOLLOW_ACCOUNT_CREATION_GAS` to capture the necessary gas amount - Update `eth_estimateGas` to check in the case of a transfer transaction if the recipient exists or not. If so return update gas, if not default to current flow - Update unit and acceptance tests Signed-off-by: nikolay <[email protected]> Signed-off-by: georgi-l95 <[email protected]> Co-authored-by: georgi-l95 <[email protected]>
Hollow account creation requires an accurate gas amount response - add a `TX_HOLLOW_ACCOUNT_CREATION_GAS` to capture the necessary gas amount - Update `eth_estimateGas` to check in the case of a transfer transaction if the recipient exists or not. If so return update gas, if not default to current flow - Update unit and acceptance tests Signed-off-by: nikolay <[email protected]> Signed-off-by: georgi-l95 <[email protected]> Co-authored-by: georgi-l95 <[email protected]> Signed-off-by: Nana Essilfie-Conduah <[email protected]>
Hollow account creation requires an accurate gas amount response - add a `TX_HOLLOW_ACCOUNT_CREATION_GAS` to capture the necessary gas amount - Update `eth_estimateGas` to check in the case of a transfer transaction if the recipient exists or not. If so return update gas, if not default to current flow - Update unit and acceptance tests Signed-off-by: nikolay <[email protected]> Signed-off-by: georgi-l95 <[email protected]> Signed-off-by: Nana Essilfie-Conduah <[email protected]> Co-authored-by: Nikolay Atanasow <[email protected]> Co-authored-by: georgi-l95 <[email protected]>
Signed-off-by: nikolay [email protected]
Description:
The basic transfer needs 21k gas but hollow account creation needs 587k gas
Related issue(s):
Fixes #
Notes for reviewer:
Checklist