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

Create Query for ProofSpecs #962

Open
AdityaSripal opened this issue Feb 21, 2022 · 4 comments
Open

Create Query for ProofSpecs #962

AdityaSripal opened this issue Feb 21, 2022 · 4 comments

Comments

@AdityaSripal
Copy link
Member

We must create an RPC endpoint for relayers so that they can query the proofspecs for a given chain. These proofspecs can then be used to create clients on counterparty chains.

Ref: #559 (comment)

@colin-axner
Copy link
Contributor

Essentially we need an gRPC so that relayers can pass in chain specific arguments that pass this function

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Mar 21, 2023

Proposal for gRPC query endpoint for Tendermint:

  • In proto/ibc/core/client/v1/query.proto:
// ClientSpec queries the parameters universal to all light clients
rpc ClientSpec(QueryClientSpecRequest) returns (QueryClientSpecResponse) {
  option (google.api.http).get = "/ibc/core/client/v1/client_spec";
}

message QueryClientSpecRequest {
  // request does not have any fields
}

message QueryClientSpecResponse {
  google.protobuf.Any client_spec = 1;
}
  • In proto/ibc/lightclients/tendermint/v1/tendermint.proto:
message ClientSpec {
  option (gogoproto.goproto_getters) = false;

  string chain_id = 1;
  google.protobuf.Duration unbonding_period = 2 
    [ (gogoproto.nullable) = false, (gogoproto.stdduration) = true ];
  ibc.core.client.v1.Height latest_height = 3
    [ (gogoproto.nullable) = false ];

  // Proof specifications used in verifying counterparty state
  repeated cosmos.ics23.v1.ProofSpec proof_specs = 4 [(gogoproto.moretags) = "yaml:\"proof_specs\""];

  // Path at which next upgraded client will be committed.
  // Each element corresponds to the key for a single CommitmentProof in the
  // chained proof. NOTE: ClientState must stored under
  // `{upgradePath}/{upgradeHeight}/clientState` ConsensusState must be stored
  // under `{upgradepath}/{upgradeHeight}/consensusState` For SDK chains using
  // the default upgrade module, upgrade_path should be []string{"upgrade",
  // "upgradedIBCState"}`
  repeated string upgrade_path = 5 [(gogoproto.moretags) = "yaml:\"upgrade_path\""];
}

I basically took Tendermint's ClientState and took out all the fields with custom values for different light clients (trust_level, trusting_period, max_clock_drift, allow_update_after_expiry, allow_update_after_misbehaviour) + frozen_height.

  • I am open to different naming for ClientSpec (I thought of ClientParams but maybe that's a bit confusing with the usage we already do of the params word).
  • I doubt if we should keep latest_height.

@AdityaSripal
Copy link
Member Author

Thanks Carlos
I think the name could be improved since you're returning a clientState
so it could be confusing if you call the grpc query ClientSpec i would assume it returns the proofspecs
i would suggest ClientStateTemplate as the name

and i would also suggest adding another endpoint:
DefaultClientState where we allow the chain to return a default client with the custom parameters filled in with some defaults that the chain chooses
99% of the time, the relayer doesn't care about making some custom modifications to the custom fields
they will just pick a sensible default
so it would be nice to just offer that as an endpoint as well if the chain can provide it for them

@colin-axner
Copy link
Contributor

I agree with @AdityaSripal's suggestions. In addition, I would propose having the ClientStateTemplate return the DefaultClientState, the relayer can modify fields as it desires. The response should include the tendermint ClientState instead of a separate type

Eventually the proof specs themselves should be queried from the SDK. This would allow the SDK flexibility in adjusting its DB layer without disrupting IBC

CosmosCar pushed a commit to caelus-labs/ibc-go that referenced this issue Nov 6, 2023
Fixes cosmos#846 

As per the discussion in cosmos#846, the aggregator node only need to
initialize the store with genesis header, after that it only needs to
broadcast the header without directly trying to append to store. The
append is take care by the broadcast api which along with broadcasting
also puts the header in the local append queue which then appends to the
local store.

Co-authored-by: Ganesha Upadhyaya <gupadhyaya@Ganeshas-MacBook-Pro-2.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

3 participants