Extend transaction structure new fields #4930
Conversation
Codecov ReportBase: 70.80% // Head: 70.79% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## rc/v1.4.0 #4930 +/- ##
=============================================
- Coverage 70.80% 70.79% -0.01%
=============================================
Files 648 649 +1
Lines 85262 85320 +58
=============================================
+ Hits 60366 60406 +40
- Misses 20366 20379 +13
- Partials 4530 4535 +5
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. |
go.mod
Outdated
| @@ -14,7 +14,7 @@ require ( | |||
| github.com/google/gops v0.3.18 | |||
| github.com/gorilla/websocket v1.5.0 | |||
| github.com/mitchellh/mapstructure v1.5.0 | |||
| github.com/multiversx/mx-chain-core-go v1.1.31 | |||
| github.com/multiversx/mx-chain-core-go v1.1.32-0.20230131083243-ebccba0c240f | |||
There was a problem hiding this comment.
proper tag before merging
|
|
||
| hasRefund := false | ||
| for _, scr := range tx.SmartContractResults { | ||
| if scr.IsRefund { |
There was a problem hiding this comment.
might refactor:
if !scr.IsRefund {
continue
}
....
break
}
There was a problem hiding this comment.
Right, or even better: split in a new function
| ) | ||
|
|
||
| type gasUsedAndFeeProcessor struct { | ||
| txFeeCalculator feeComputer |
There was a problem hiding this comment.
Calculator & Computer ? Let's rename to even-out the naming
|
|
||
| hasRefund := false | ||
| for _, scr := range tx.SmartContractResults { | ||
| if scr.IsRefund { |
There was a problem hiding this comment.
Right, or even better: split in a new function
| } | ||
|
|
||
| for _, event := range tx.Logs.Events { | ||
| if core.WriteLogIdentifier == event.Identifier && !hasRefund { |
There was a problem hiding this comment.
split in more functions?
| @@ -56,6 +56,39 @@ func NewFeeComputer(args ArgsNewFeeComputer) (*feeComputer, error) { | |||
| return computer, nil | |||
| } | |||
|
|
|||
| // ComputeGasUsedAndFeeBasedOnRefundValue computes gas used and fee based on the refund value, at a given epoch | |||
| func (computer *feeComputer) ComputeGasUsedAndFeeBasedOnRefundValue(tx *transaction.ApiTransactionResult, refundValue *big.Int) (uint64, *big.Int) { | |||
There was a problem hiding this comment.
unit tests for the new functions?
miiu96
added
type:feature
| if core.WriteLogIdentifier == event.Identifier && !hasRefund { | ||
| gasUsed, fee := gfp.feeComputer.ComputeGasUsedAndFeeBasedOnRefundValue(tx, big.NewInt(0)) | ||
| tx.GasUsed = gasUsed | ||
| tx.Fee = fee.String() |
There was a problem hiding this comment.
missing return here?
| @@ -14,7 +14,7 @@ require ( | |||
| github.com/google/gops v0.3.18 | |||
| github.com/gorilla/websocket v1.5.0 | |||
| github.com/mitchellh/mapstructure v1.5.0 | |||
| github.com/multiversx/mx-chain-core-go v1.1.32-0.20230131083243-ebccba0c240f | |||
| github.com/multiversx/mx-chain-core-go v1.1.32 | |||
Reasoning behind the pull request
Proposed changes
Transactionstructure that is returned on the/transactionendpoint with 2 extra fields:gasUsedandfeemx-chain-core-gomodule.mx-chain-core-gorepository: Extend api transaction structure mx-chain-core-go#161Testing procedure
gasUsedand thefeefield if are returned by thetransactionendpoint of the node and proxyPre-requisites
Based on the Contributing Guidelines the PR author and the reviewers must check the following requirements are met:
featbranch created?featbranch merging, do all satellite projects have a proper tag insidego.mod?