services/horizon: Allow filtering accounts by liquidity pool participation. #3873
Conversation
Shaptic
force-pushed
the
pools-in-accounts
branch
from
bd4104b to
9f0e19d
Compare
Shaptic
force-pushed
the
pools-in-accounts
branch
from
9f0e19d to
3a260bd
Compare
While this is technically less defensive, it's still strictly correct, because it covers all of the possible cases for this code block: only pool shares / asset4 / asset12 can be a part of the switch statement.
| @@ -366,6 +366,32 @@ func (q *Q) AccountEntriesForSigner(ctx context.Context, signer string, page db2 | |||
| return results, nil | |||
| } | |||
|
|
|||
| // AccountEntriesForLiquidityPool returns a list of `AccountEntry` rows that | |||
| // have trustlines established with a given liquidity pool ID. | |||
| func (q *Q) AccountEntriesForLiquidityPool(ctx context.Context, poolId string, page db2.PageQuery) ([]AccountEntry, error) { | |||
There was a problem hiding this comment.
we already have AccountsForLiquidityPool(), do we need to add this function?
| "native": xdr.AssetTypeAssetTypeNative, | ||
| "credit_alphanum4": xdr.AssetTypeAssetTypeCreditAlphanum4, | ||
| "credit_alphanum12": xdr.AssetTypeAssetTypeCreditAlphanum12, | ||
| "liquidity_pool_shares": xdr.AssetTypeAssetTypePoolShare, |
There was a problem hiding this comment.
AssetTypeMap is used to validate asset query parameters for a bunch of different endpoints in horizon including the trades and path finding endpoints. Not all horizon endpoints should accept pool share asset types. For example pool shares are not transferrable therefore it doesn't make sense to do path payments where the source or destination asset is a pool share.
I think we should treat the endpoints which accept pool shares asset parameters as a special case rather than making all asset parameters validate pool shares as ok
There was a problem hiding this comment.
I removed it in 1b8eeb9 however I think AssetTypeMap should be a general usage map that's mapping all types. If there are actions that doesn't allow liquidity pools we should probably add an extra validation there. We should probably do it after future net deploy.
|
thanks @bartekn , looks good! |
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
needed with deprecations, added features, breaking changes, and DB schema changes.
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
GET /accounts?liquidity_pool=[...some id...]).Why
Resolves #3832.
Known limitations
Still needs to be tested, hence why it's in draft.