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

Fix backwards compatibility for tx fee #2257

Merged
merged 12 commits into from
Sep 2, 2020

Conversation

SebastianMarian
Copy link
Contributor

  • Fixed backwards compatibility for calculation of tx fee in "checkTxValues" method from baseTxProcessor and "Fee" method from InterceptedTransaction

…Values" method from baseTxProcessor and "Fee" method from InterceptedTransaction
@SebastianMarian SebastianMarian self-assigned this Aug 30, 2020
@SebastianMarian SebastianMarian changed the title Fix backward compatibility for tx fee Fix backwards compatibility for tx fee Aug 30, 2020
@SebastianMarian SebastianMarian added type:feature New feature or request and removed type:needs-investigation labels Aug 31, 2020
@SebastianMarian SebastianMarian marked this pull request as ready for review August 31, 2020 17:57
@sasurobert sasurobert self-requested a review August 31, 2020 18:00
cmd/node/main.go Outdated
@@ -833,7 +833,11 @@ func startNode(ctx *cli.Context, log logger.Logger, version string) error {
time.Sleep(secondsToWaitForP2PBootstrap * time.Second)

log.Trace("creating economics data components")
economicsData, err := economics.NewEconomicsData(economicsConfig)
economicsData, err := economics.NewEconomicsData(
economicsConfig,
Copy link
Contributor

Choose a reason for hiding this comment

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

add arguments structure instead of list of arguments

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


return gasPrice.Mul(gasPrice, gasLimit)
// EstimateMoveBalanceFee estimates the provided transaction's fee
func (ed *EconomicsData) EstimateMoveBalanceFee(tx process.TransactionWithFeeHandler) *big.Int {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong function name - we will get into the same problem later on.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -137,7 +137,7 @@ func (txProc *baseTxProcessor) checkTxValues(
return process.ErrWrongTypeAssertion
}

txFee := core.SafeMul(tx.GasLimit, tx.GasPrice)
txFee := txProc.economicsFee.EstimateMoveBalanceFee(tx)
Copy link
Contributor

Choose a reason for hiding this comment

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

ComputeTxFee

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

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

System tests passed.

@LucianMincu LucianMincu merged commit 4e62a24 into development Sep 2, 2020
@LucianMincu LucianMincu deleted the Fix-backwards-compatibility-for-tx-fee branch September 2, 2020 07:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants