services/horizon: Batch liquidity pool updates/removals #3944
Conversation
2opremio
force-pushed
the
3931-lpchange-upsert
branch
from
387ae24 to
1b0df11
Compare
2opremio
force-pushed
the
3931-lpchange-upsert
branch
from
1b0df11 to
d5fb5c3
Compare
| lp := p.ledgerEntryToRow(change.Pre) | ||
| lp.Deleted = true | ||
| lp.LastModifiedLedger = p.sequence | ||
| lps = append(lps, lp) |
There was a problem hiding this comment.
I think we should create RemoveLiquidityPools method and hide implementation of deleted objects (Deleted flag) from the caller.
There was a problem hiding this comment.
Can you be more specific? By implementation of deleted objects you mean factoring out the creation of a deleted liquidity pool value?
The deleted field is public anyhow
There was a problem hiding this comment.
By implementation of deleted objects you mean factoring out the creation of a deleted liquidity pool value?
Exactly. If we ever change the implementation of how objects are removed (idk: we also add a deleted time or we will just remove the row) we'll have to change every instance of deletion upstream and fix a bunch of tests. I agree that Deleted field is public. We should probably also change this (not in this PR).
There was a problem hiding this comment.
I have given it a try, but I don't think it makes sense to attempt that sort of encapsulation without removing the Deleted field (which I think is out of the context of this PR).
Can we handle this separately?
Fixes #3931