Add fee fields; formatting for warnings #240
Conversation
chowbao
changed the title
| @@ -68,7 +68,12 @@ func GetLedgers(start, end uint32, limit int64, env utils.EnvironmentDetails, us | |||
| }, | |||
| } | |||
|
|
|||
| ledgerSlice = append(ledgerSlice, ledger) | |||
| ledgerLCM := utils.HistoryArchiveLedgerAndLCM{ | |||
There was a problem hiding this comment.
Because of the new LCM fee field we need to pass in the historyarchive.Ledger as well as the LedgerCloseMeta. This should be eventually refactored in the future to where input/ledger and input/assets don't use historyarchive.Ledger and just use LCM
| extV1, ok := meta.SorobanMeta.Ext.GetV1() | ||
| if ok { | ||
| outputTotalNonRefundableResourceFeeCharged = int64(extV1.TotalNonRefundableResourceFeeCharged) | ||
| outputTotalRefundableResourceFeeCharged = int64(extV1.TotalRefundableResourceFeeCharged) | ||
| outputRentFeeCharged = int64(extV1.RentFeeCharged) | ||
| } |
internal/transform/ledger.go
Outdated
| var outputSorobanFeeWrite1Kb int64 | ||
| lcmV1, ok := lcm.GetV1() | ||
| if ok { | ||
| outputSorobanFeeWrite1Kb = int64(lcmV1.Ext.V1.SorobanFeeWrite1Kb) |
There was a problem hiding this comment.
may be worthwhile to use the safety getter method of LedgerCloseMetaExt.GetV1() rather than direct reference to nested V1 struct, to avoid potential nil pointer, i.e. check ok from lcmv1.Ext.GetV1()
There was a problem hiding this comment.
Makes sense. Done in d43e728
There was a problem hiding this comment.
The HistoryArchiveLedgerAndLCM is definitely a hack that I want to address and refactor sometime this quarter. Probably a good opportunity to pair program with a new team member and refactor that code. Like you noted, if someone runs the codebase using Captive Core, they will run into errors getting the write fee since LCM will be empty.
Also, do you think there is any merit in keeping the code you commented out, or should that just be deleted entirely? A lot of those functions predate me and were original attempts to maintain parity between Horizon/Hubble.
| TotalNonRefundableResourceFeeCharged int64 `json:"non_refundable_resource_fee_charged"` | ||
| TotalRefundableResourceFeeCharged int64 `json:"refundable_resource_fee_charged"` |
There was a problem hiding this comment.
do these differ from resource_fee_refund and resource_fee?
There was a problem hiding this comment.
From the xdr itself they are different fields. Functionally equivalent I'm not sure. We can probably compare them when there is data emitted and then determine if we only need to keep one or the other
Nit: Should we also update the Captive core configs to use those flags within |
Yeah we can delete them. Worse case we can find them again in the commit history |
Note: The
EMIT_SOROBAN_TRANSACTION_META_EXT_V1EMIT_LEDGER_CLOSE_META_EXT_V1flags for core are set to true for ledgerexporter testnet txmeta files