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

Service provider directory support in nym-api, nym-cli, validator-client #3332

Merged
merged 8 commits into from
Apr 27, 2023

Conversation

octol
Copy link
Contributor

@octol octol commented Apr 20, 2023

Description

Closes: #3296
Closes: #3241
Closes: #3242
Closes: #3334

  • Add sp directory contract traits and methods to nyxd client
  • Make the contract optional
  • nym-api: add service provider endpoint and caching
  • nym-cli: add query, announce and delete service provider

@octol octol changed the base branch from develop to release/v1.1.16 April 20, 2023 10:58
@octol
Copy link
Contributor Author

octol commented Apr 20, 2023

The way we should test this IMHO is the following:

  1. Deploy nym-api from this branch, and make sure it works with qwerty where the service provider contract is already deployed, but crucially also that it works with mainnet where there is no contract yet. The service provider contract is supposed to be optional so we don't want anything breaking for networks where it's missing.
    1. For qwerty I'd expect the endpoints for the nym-api to be able to query announced services
    2. For mainnet, the endpoint should be there but it should return an empty list
    3. Announcing and deleting services is done via the nym-cli
  2. Once the above is cleared, I think we should merge it into the release branch
  3. When it's merged into the release branch we can start doing further more extensive testing of the service-provider contract in itself (which can be a follow up task)

Copy link
Contributor

@jstuczyn jstuczyn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm


/// Deposit to be made to the service provider directory, in curent DENOMINATION (e.g. 'unym')
#[clap(long)]
pub deposit: u128,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would make sense to change the deposit to be an explicit Coin so people wouldnt be confused by unym vs nym. But it's more of a note for the future

@octol octol changed the base branch from release/v1.1.16 to release/v1.1.17 April 26, 2023 08:58
@octol octol merged commit 6fe93bc into release/v1.1.17 Apr 27, 2023
@octol octol deleted the jon/feat/sp-integrations branch April 27, 2023 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants