Skip to content
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

R4R: fix metatx estimateGas #33

Merged
merged 4 commits into from
Jan 23, 2024
Merged

R4R: fix metatx estimateGas #33

merged 4 commits into from
Jan 23, 2024

Conversation

wwqicode-blkchain
Copy link

if metatx not nil, sponsorAmount is set by gasCap(price), but should be similar to gasPrice * gasUsed. this causes estimateGas failed if gasUsed larger than 1.(100percent and 0 fromBalance)
e.g.: 100% sponsor, and msg.sender has no MNT. sponsorAmount will be set as feeCap. Balance in total is 0 + feeCap, and will also set to "avaliable". Then allowance is set by avaliable div feeCap, so allowance will be 1. If only the gasUsed larger than 1, error msg "gas required exceeds allowance (1)" will be returned.

@wwqicode-blkchain wwqicode-blkchain changed the title fix(eth api): metatx estimateGas R4R: fix metatx estimateGas Jan 22, 2024
abelliumnt
abelliumnt previously approved these changes Jan 22, 2024
if sponsorAmount.Cmp(sponsorBalance) < 0 {
available.Add(available, sponsorAmount)
} else {
available.Add(available, sponsorBalance)

Choose a reason for hiding this comment

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

sponser & user's balance are added to available, but not all their balance are used to afford gasFee.

  • in a case, sponser's balance is used to afford, and user's balance should not be involved.

Copy link
Author

Choose a reason for hiding this comment

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

Because of we can not get the correct gasLimit before EstimateGas,we can only calculate sponsorAmount and selfAmount by kind of blockGasLimit feeCap * hi. The results of two amount may be much larger than balance. Such judgments may be inappropriate.


if metaTxParams != nil {
sponsorAmount, _ := types.CalculateSponsorPercentAmount(metaTxParams, new(big.Int).Mul(feeCap, new(big.Int).SetUint64(hi)))
sponsorBalance := state.GetBalance(metaTxParams.GasFeeSponsor)

Choose a reason for hiding this comment

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

sponsorAmount, selfAmount := types.CalculateSponsorPercentAmount(metaTxParams, new(big.Int).Mul(feeCap, new(big.Int).SetUint64(hi)))

available should be no less than selfAmount. Please add this check.

Copy link
Author

@wwqicode-blkchain wwqicode-blkchain Jan 23, 2024

Choose a reason for hiding this comment

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

sponsorAmount and selfAmount will be a large number because we get them by CalculateSponsorPercentAmount with input feeCap * hi. The hi is kind of blockGasLimit, a huge number.
Therefore, making judgments based on such results is inaccurate.

Copy link
Author

Choose a reason for hiding this comment

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

calculate amount inside estimateGas function is merely to assign a value to "available" but can not verify whether the balance is sufficient.
Or whether we can estimateGas with payload without metatx prefix pureGasLimit,then add a const number to the gasLimit metaTxGasLimit.
CalculateSponsorPercentAmount use metaTxGasLimit as input, sponsorAmount & selfAmount can be more accurate. Checking the balance would also be more meaningful.

@abelliumnt abelliumnt merged commit f76ba4b into release/v0.5.0 Jan 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants