Skip to content
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

pd: add --enable-expensive-rpc flag #3300

Merged
merged 3 commits into from
Nov 9, 2023
Merged

Conversation

erwanor
Copy link
Member

@erwanor erwanor commented Nov 9, 2023

This PR adds a --simulate-trade-service flag to pd that activates the DEX trade simulation service, which allow users to send SimulateTrade RPCs. The service is disabled by default, because it can be onerous to run it against a rich orderbook, and therefore is a potential DoS vector.

@conorsch
Copy link
Contributor

conorsch commented Nov 9, 2023

@erwanor mind if I tack on the necessary deployment changes to this PR, so that we can use this on preview once it's merged? I can also follow up in a separate PR if you'd prefer.

@conorsch conorsch self-requested a review November 9, 2023 01:16
@erwanor
Copy link
Member Author

erwanor commented Nov 9, 2023

Not at all, thanks actually

@hdevalence
Copy link
Member

Not to bikeshed, but I'm wondering if this could be --enable-expensive-rpc or something similar, so that we could just have one flag and if we add some other similar RPC later we could have it all grouped into one setting.

@erwanor erwanor changed the title pd: add --simulate-trade-service flag pd: add --enable-expensive-rpc flag Nov 9, 2023
Here we unconditionally enable serving expensive gRPC services via pd
for the fullnodes. We could make this option configurable in the
deployment logic, but our use case is that we want the expensive options
to be available to the dev team, so just forcing them all on. Notably we
don't expose the RPC services for the genesis validators, and we're not
enabling the flag on pd for the gen vals, so we'll monitor performance
on the load-balanced fullnode endpoints and adjust if necessary.
@conorsch conorsch merged commit acf79c7 into main Nov 9, 2023
6 checks passed
@conorsch conorsch deleted the erwan/simulate_service_flag branch November 9, 2023 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants