protos(view): add a subaccount filter to the OwnedPositionIds rpc
#4992
Conversation
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
6 tasks
|
This PR implements the changes requested in #4985 (review). |
2 tasks
|
Downgraded this PR to draft, so that #4993 can land first. Once that's done, I'll need to rebase this and regenerate the proto files, to ensure no conflicts. |
conorsch
added a commit
that referenced
this pull request
Jan 17, 2025
) ## Describe your changes This reverts commit 64c32ef, which constitutes the squash-merge of PR #4980. We're backing this change out strictly to simplify release engineering: we want the `main` branch to remain fully compatible with the `0.81.x` series, and we'll continue QA of significant version changes in a parallel release branch, `release/v0.82.x`. I'll handle preparing the latter shortly. ## Issue ticket number and link See related discussion in #4988 & #4991. ## Testing and review This is a programmatic change, in that I simply ran `git revert 64c32ef`, wrote some notes into the commit message, and pushed it up. I also made sure to rerun `just proto` to regenerate the protos, and confirmed there are no changes. That's good, that's precisely what we wanted to see. Preferably this change would land before #4992, since #4992 changes protos. I'll regenerate protos in 4992 on top of this once it lands on main. ## Checklist before requesting a review - [x] I have added guiding text to explain how a reviewer should test these changes. - [x] If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: > This commit is expressly intended to preserve protocol compatibility with 0.81.x. Future work on QA to ensure compat with 0.82 will happen in a separate branch.
conorsch
force-pushed
the
4985-rebased-view_subaccount_index_owned_positions
branch
from
311f304 to
4c3c816
Compare
erwanor
approved these changes
Jan 17, 2025
cronokirby
reviewed
Jan 17, 2025
cronokirby
approved these changes
Jan 17, 2025
Co-authored-by: Lúcás Meier <[email protected]> Signed-off-by: Erwan Or <[email protected]>
conorsch
deleted the
4985-rebased-view_subaccount_index_owned_positions
branch
This was referenced Jan 21, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Describe your changes
This PR resubmits the unmerged changes from #4985, targeting
mainbranch. Below is the original submission text for 4985, which is still accurate here.This PR:
AddressIndexfield to theOwnedPositionIdsrpc request and responseIt doesn't seem important to do the extra work to support filtering in the rust view server. The natural consumers of that information are web interfaces. Market makers using the rust implementation would probably simply integrate it in their own OMS rather than relying on the view service RPC.
One question I have is whether the buf publish action will be able to pickup the change to the protos.
Checklist before requesting a review
I have added guiding text to explain how a reviewer should test these changes.
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: