ingest: Add support for CAP-63 #5615
Conversation
| Post: change.Post, | ||
| ChangeType: existingChange.ChangeType, | ||
| case xdr.LedgerEntryChangeTypeLedgerEntryCreated, | ||
| xdr.LedgerEntryChangeTypeLedgerEntryUpdated, |
There was a problem hiding this comment.
LedgerEntryUpdated's are now cached into the compactor all the time, before it was dropped, don't see any changes in existing tests that may have broken related to this, I do see the new coverage on TTL's, is there potential edge case where LedgerEntryUpdated should still be dropped or are TTL's the only source of an LedgerEntryUpdated?
There was a problem hiding this comment.
LedgerEntryUpdated's are now cached into the compactor all the time, before it was dropped
the behavior is actually the same. The fallthrough keyword means it should match the case below. In other words,
case xdr.LedgerEntryChangeTypeLedgerEntryUpdated:
fallthrough
case xdr.LedgerEntryChangeTypeLedgerEntryRestored:is equivalent to:
case xdr.LedgerEntryChangeTypeLedgerEntryUpdated, xdr.LedgerEntryChangeTypeLedgerEntryRestored:
| @@ -58,8 +58,21 @@ func (l LedgerCloseMeta) TransactionEnvelopes() []TransactionEnvelope { | |||
| case 1: | |||
| var envelopes = make([]TransactionEnvelope, 0, l.CountTransactions()) | |||
| for _, phase := range l.MustV1().TxSet.V1TxSet.Phases { | |||
| for _, component := range *phase.V0Components { | |||
| envelopes = append(envelopes, component.TxsMaybeDiscountedFee.Txs...) | |||
| switch phase.V { | |||
There was a problem hiding this comment.
should the new v1, and panic cases be exercised through unit tests? don't see any coverage directly against this method, or could those branches be asserted indirectly through LedgerTransactionReader tests or other?
There was a problem hiding this comment.
I added a test case
| @@ -163,6 +163,14 @@ func (c *ChangeCompactor) getLedgerKey(ledgerEntry *xdr.LedgerEntry) ([]byte, er | |||
| return ledgerKey, nil | |||
| } | |||
|
|
|||
| // maxTTL returns the ttl entry with the highest LiveUntilLedgerSeq | |||
| func maxTTL(a, b xdr.TtlEntry) xdr.TtlEntry { | |||
There was a problem hiding this comment.
Acknowledged for the maxTTL
| case 1: | ||
| for _, stage := range phase.ParallelTxsComponent.ExecutionStages { | ||
| for _, cluster := range stage { | ||
| for _, envelope := range cluster { | ||
| envelopes = append(envelopes, envelope) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
@chowbao this change might need to be ported to stellar-etl. I updated ingest/processors/ledger_processor/ledger.go to extract the new source of transaction envelopes but I'm not sure if there are other places in the etl processors code which need to be updated
There was a problem hiding this comment.
Yeah we will make the changes in stellar-etl. TBH I'm not sure exactly where the logic is used in stellar-etl but a quick grep will fix that when we make the update
PR Checklist
PR Structure
otherwise).
services/friendbot, orallordocif the changes are broad or impact manypackages.
Thoroughness
.mdfiles, etc... affected by this change). Take a look in the
docsfolder for a given service,like this one.
Release planning
CHANGELOG.mdwithin the component folder structure. For example, if I changed horizon, then I updated (services/horizon/CHANGELOG.md. I add a new line item describing the change and reference to this PR. If I don't update a CHANGELOG, I acknowledge this PR's change may not be mentioned in future release notes.semver, or if it's mainly a patch change. The PR is targeted at the next
release branch if it's not a patch change.
What
Fixes #5614
This PR implements support for CAP-63 see #5614 for more details.
The failing tests are fixed in #5611 . Once #5611 is merged , I will rebase this PR and the tests should pass.
Known limitations
[N/A]