-
Notifications
You must be signed in to change notification settings - Fork 499
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
services/horizon: Allow filtering accounts by liquidity pool participation. #3873
Merged
Merged
Changes from 11 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
465d309
Update API definition to include pool ID in balances
Shaptic 16bed52
Add skeleton code enabling filtering accounts by pool participation
Shaptic b0d3e42
Add query for selecting accounts by liquidity pool
Shaptic 5015351
Add changelog entry
Shaptic 3a260bd
Populate account balances with pool shares
Shaptic 1f6c2b6
Separate changelog entries
Shaptic 5d893dc
Add tests for filling out account balances
Shaptic 583b943
Test non-existent liabilities properly
Shaptic c0df714
Simplify liabilities check
Shaptic ebcdf9c
Amend query parameter tests to include liquidity_pool filter
Shaptic f42e1f2
Simplify asset type parsing for filling balance entries
Shaptic 1b8eeb9
review feedback
bartekn 7b119c4
fix tests
bartekn 12b52db
fix shadow
bartekn File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we already have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 1b8eeb9. |
||
return nil, errors.New("filtering by liquidity pool is not implemented") | ||
|
||
sql := sq. | ||
Select("accounts.*"). | ||
From("accounts"). | ||
Join("trust_lines ON accounts.account_id = trust_lines.account_id"). | ||
Where(map[string]interface{}{ | ||
"trust_lines.liquidity_pool_id": poolId, | ||
}) | ||
|
||
sql, err := page.ApplyToUsingCursor(sql, "trust_lines.liquidity_pool_id", page.Cursor) | ||
if err != nil { | ||
return nil, errors.Wrap(err, "could not apply query to page") | ||
} | ||
|
||
var results []AccountEntry | ||
if err := q.Select(ctx, &results, sql); err != nil { | ||
return nil, errors.Wrap(err, "could not run select query") | ||
} | ||
|
||
return results, nil | ||
} | ||
|
||
var selectAccounts = sq.Select(` | ||
account_id, | ||
balance, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.