services/horizon: Exclude trades with >10% rounding slippage from trade aggregations

[paulbellamy]

PR Checklist

PR Structure

• This PR has reasonably narrow scope (if not, break it down into smaller PRs).
• This PR avoids mixing refactoring changes with feature changes (split into two PRs
  otherwise).
• This PR's title starts with name of package that is most changed in the PR, ex.
  services/friendbot, or all or doc if the changes are broad or impact many
  packages.

Thoroughness

• This PR adds tests for the most critical parts of the new functionality or fixes.
• I've updated any docs (developer docs, .md
  files, etc... affected by this change). Take a look in the docs folder for a given service,
  like this one.

Release planning

• I've updated the relevant CHANGELOG (here for Horizon) if
  needed with deprecations, added features, breaking changes, and DB schema changes.
• I've decided if this PR requires a new major/minor version according to
  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

Track rounding slippage on trades, and exclude any >10% from trade aggregations.

Why

Fixes #4160

Todo

• Make the filter configurable
• Check it won't affect any other trades
• Check if we need an index on the new column

[2opremio]

I wonder whether it would be feasible to compute the slippage on the fly instead of storing it in the DB.

[paulbellamy]

The issue with on-the-fly, is that you need the ledger meta, as it contains the pool reserves.

[bartekn]

The code tested in stg (commit 8f924e1781dfc0bccc7e8c7e1287944008db37f9 which isn't part of this PR but letting you know anyway) was crashing on ledger 39270855 with:

panic: division by zero
goroutine 121 [running]:
math/big.(*Rat).Quo(0xc0004eaf28, 0xc0004eb0a8, 0xc0004eafe8)
        math/big/rat.go:535 +0x136
github.com/stellar/go/services/horizon/internal/ingest/processors.(*TradeProcessor).roundingSlippage(0xbf2f077a6b1a7e7, {0x7a,>
        github.com/stellar/go/services/horizon/internal/ingest/processors/trades_processor.go:233 +0x7dd
github.com/stellar/go/services/horizon/internal/ingest/processors.(*TradeProcessor).extractTrades(_, {{0xca, 0x5f, 0x24, 0xc8,>
        github.com/stellar/go/services/horizon/internal/ingest/processors/trades_processor.go:375 +0x188d
github.com/stellar/go/services/horizon/internal/ingest/processors.(*TradeProcessor).ProcessTransaction(0xc012db65a0, {0x12a75b>
        github.com/stellar/go/services/horizon/internal/ingest/processors/trades_processor.go:53 +0x158
github.com/stellar/go/services/horizon/internal/ingest.groupTransactionProcessors.ProcessTransaction({{0xc0043c3950, 0x6d6d030>
        github.com/stellar/go/services/horizon/internal/ingest/group_processors.go:67 +0x133
github.com/stellar/go/services/horizon/internal/ingest/processors.StreamLedgerTransactions({0x12a75b0, 0xc000648640}, {0x128e5>
        github.com/stellar/go/services/horizon/internal/ingest/processors/change_processors.go:32 +0x245
github.com/stellar/go/services/horizon/internal/ingest.(*ProcessorRunner).RunTransactionProcessorsOnLedger(_, {_, _})
        github.com/stellar/go/services/horizon/internal/ingest/processor_runner.go:296 +0x328
github.com/stellar/go/services/horizon/internal/ingest.(*ProcessorRunner).RunAllProcessorsOnLedger(_, {_, _})
        github.com/stellar/go/services/horizon/internal/ingest/processor_runner.go:335 +0x406
github.com/stellar/go/services/horizon/internal/ingest.resumeState.run({0xce0d8f}, 0xc000032480)
        github.com/stellar/go/services/horizon/internal/ingest/fsm.go:471 +0x13fe
github.com/stellar/go/services/horizon/internal/ingest.(*system).runStateMachine(0xc000032480, {0x129c730, 0x19adaa0})
        github.com/stellar/go/services/horizon/internal/ingest/main.go:580 +0x364
github.com/stellar/go/services/horizon/internal/ingest.(*system).Run(0x0)
        github.com/stellar/go/services/horizon/internal/ingest/main.go:478 +0x45
github.com/stellar/go/services/horizon/internal.(*App).Serve.func1()
        github.com/stellar/go/services/horizon/internal/app.go:102 +0x35
created by github.com/stellar/go/services/horizon/internal.(*App).Serve
        github.com/stellar/go/services/horizon/internal/app.go:101 +0x291

[Shaptic]

Added some minor notes, and an important one re: performance. Seeing the benchmark (exp/orderbook/graph_benchmark_test.go) would help alleviate my concern 👍

[paulbellamy]

With orderbook dumped to ledger 39339519...

master as of 4920b27104df0ec7631eae28b5295e220fb59be7

$ go test ./exp/orderbook -bench '.*' -offers offers.dump -pools pools.dump
goos: darwin
goarch: arm64
pkg: github.com/stellar/go/exp/orderbook
BenchmarkVibrantPath-8                               160           7442436 ns/op         2868812 B/op         44 allocs/op
BenchmarkMultipleDestinationAssets-8                 100          10807999 ns/op         2871080 B/op         70 allocs/op
BenchmarkTestData-8                                    2         735662062 ns/op        286842320 B/op      2299 allocs/op
BenchmarkSingleLiquidityPoolExchange/deposit-8          1000000000               0.0000003 ns/op
BenchmarkSingleLiquidityPoolExchange/exchange-8         1000000000               0.0000001 ns/op
BenchmarkLiquidityPoolDeposits-8                        17120752                69.89 ns/op            0 B/op          0 allocs/op
BenchmarkLiquidityPoolExpectations-8                    71820445                16.11 ns/op            0 B/op          0 allocs/op
PASS
ok      github.com/stellar/go/exp/orderbook     32.008s

This branch as of 2af05facc58b59e955ae0e5fc59aa89aab190cea

$ go test ./exp/orderbook -bench '.*' -offers offers.dump -pools pools.dump
goos: darwin
goarch: arm64
pkg: github.com/stellar/go/exp/orderbook
BenchmarkVibrantPath-8                               153           7868738 ns/op         2868812 B/op         44 allocs/op
BenchmarkMultipleDestinationAssets-8                 100          10674957 ns/op         2871083 B/op         70 allocs/op
BenchmarkTestData-8                                    2         745304916 ns/op        286842392 B/op      2299 allocs/op
BenchmarkSingleLiquidityPoolExchange/deposit-8          1000000000               0.0000004 ns/op
BenchmarkSingleLiquidityPoolExchange/exchange-8         1000000000               0.0000001 ns/op
BenchmarkLiquidityPoolDeposits-8                         7928089               152.8 ns/op             0 B/op          0 allocs/op
BenchmarkLiquidityPoolExpectations-8                    71751022                16.35 ns/op            0 B/op          0 allocs/op
PASS
ok      github.com/stellar/go/exp/orderbook     32.745s

So, the main difference is in BenchmarkLiquidityPoolDeposits. (Could be that the Expectations scenario isn't hitting the rounding slippage), which is ~2x slower. Not ideal. I'm not really sure how to write the math faster than it is, so maybe someone else knows, or the other option would be to only store the reserves, and calculate it as part of rebuilding trade aggregations. Which would be less efficient, but not in the hot path.

I dont' particularly believe the 0 allocs/op thing, as there are definitely allocations.

[paulbellamy]

Uncovered a bug in effect ingestion while testing this, so this is blocked on #4203

[paulbellamy]

Ok. Did a bunch of analysis, and came to the conclusion that 10% is a "good enough" spot to set the filter default. There's a long-tail of >10%, but beyond that the graphs get ugly. We can clean the graphs up more by going down to 5%, or even 1%, but then we risk cutting out some legitimate trades.

[bartekn]

Nit: looks like ok is always true before this line.

Suggested change

	ok = ok && S.IsUint64() && roundingSlippageBips >= 0
	ok = S.IsUint64() && roundingSlippageBips >= 0

[bartekn]

Leftover because the new config value was a float64 at some point?