base/counter_offer_id not ingested when buy offer not exist (synthetic)

[bartekn]

What version are you using?

release-horizon-v2.9.0 branch

What did you do?

Run verify-range tests for range: [13494463, 13494591].

What did you expect to see?

No diff when comparing to 2.8.3 (DB dump contains offer_id column removed in 2.9.0 - I manually removed it, will add a PR tomorrow).

What did you see instead?

Diff:

1,2c1,2
< 57958466240647169,0,2017-09-14 22:50:10,12536407,10000000,121914,4669644484668035073,GB7JKG66CJN3ACX5DX43FOZTTSOI7GZUP547I3BSXIJVUX3NRYUXHE6W,GC6APVH2HCFB7QLSTG3U55IYSW7ZRNSCTOZZYZJCNHWX2FONCNJNULYN,native,,,credit_alphanum4,JPY,GBVAOIACNSB7OVUXJYC5UE2D4YK2F7A24T7EE5YOMN4CE6GCHUTOUQXM
< 57958470535614465,0,2017-09-14 22:50:18,74221311,5000000,121685,4669644488963002369,GDDNNC4SIEDN64YKJWN5VJH3OFMV4R3RWXRPT25BDR3T2HQWZMOWD6QJ,GB7JKG66CJN3ACX5DX43FOZTTSOI7GZUP547I3BSXIJVUX3NRYUXHE6W,credit_alphanum4,XRP,GA7FCCMTTSUIC37PODEL6EOOSPDRILP6OQI5FWCWDDVDBLJV72W6RINZ,native,,
---
> 57958466240647169,0,2017-09-14 22:50:10,12536407,10000000,121914,,GB7JKG66CJN3ACX5DX43FOZTTSOI7GZUP547I3BSXIJVUX3NRYUXHE6W,GC6APVH2HCFB7QLSTG3U55IYSW7ZRNSCTOZZYZJCNHWX2FONCNJNULYN,native,,,credit_alphanum4,JPY,GBVAOIACNSB7OVUXJYC5UE2D4YK2F7A24T7EE5YOMN4CE6GCHUTOUQXM
> 57958470535614465,0,2017-09-14 22:50:18,74221311,5000000,121685,,GDDNNC4SIEDN64YKJWN5VJH3OFMV4R3RWXRPT25BDR3T2HQWZMOWD6QJ,GB7JKG66CJN3ACX5DX43FOZTTSOI7GZUP547I3BSXIJVUX3NRYUXHE6W,credit_alphanum4,XRP,GA7FCCMTTSUIC37PODEL6EOOSPDRILP6OQI5FWCWDDVDBLJV72W6RINZ,native,,

The missing field is base_offer_id or counter_offer_id. I think the bug (if it is a bug or me missing something) was introduced in #3857 which removed synthetic offer ID. The new code is missing the else branch:

go/services/horizon/internal/ingest/processors/trades_processor.go

Lines 292 to 294 in e55e42a

	if buyOfferExists {
	row.CounterOfferID = null.IntFrom(int64(buyOffer.OfferId))
	}

present in the old code:

go/services/horizon/internal/db2/history/trade_batch_insert_builder.go

Lines 65 to 69 in e6f322c

	if entry.BuyOfferExists {
	buyOfferID = EncodeOfferId(uint64(entry.BuyOfferID), CoreOfferIDType)
	} else {
	buyOfferID = EncodeOfferId(uint64(entry.HistoryOperationID), TOIDType)
	}

@tamirms can you check it out? Was it removed accidentally? I can't see any AMM issue mentioning removal of synthetic offer IDs.

[tamirms]

@bartekn I allowed base_offer_id and counter_offer_id to be NULL when there is no offer involved in the trade (e.g. liquidity pool trades). What is the purpose of having synthetic offer ids?

[bartekn]

Shouldn't we have this still for non-LP trades? AFAIR, synthetic offer IDs are there for offers which are immediately traded so aren't assigned a specific ID.

[bartekn]

Synthetic offer IDs were added in #667 and motivation was:

This should allow easier audit trails for offers and trades. When offer ids are not created in the protocol level (fully consumed offers and path payments) a synthetic offer id will be created (...).