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

Fix IBC Query cmds #7648

Merged
merged 4 commits into from
Oct 23, 2020
Merged

Fix IBC Query cmds #7648

merged 4 commits into from
Oct 23, 2020

Conversation

colin-axner
Copy link
Contributor

Description

Tested:

 gaiad q ibc client state thestargate --height 0  --chain-id slippy-0 --node http://159.65.118.186:26657
client_state:
  '@type': /ibc.lightclients.tendermint.v1.ClientState
  allow_update_after_expiry: false
  allow_update_after_misbehaviour: false
  chain_id: stargate-4
  consensus_params:
    block:
      max_bytes: "22020096"
      max_gas: "-1"
    evidence:
      max_age_duration: 172800s
      max_age_num_blocks: "100000"
      max_num: 50
    validator:
      pub_key_types:
      - ed25519
    version: null
  frozen_height:
    version_height: "0"
    version_number: "0"
  latest_height:
    version_height: "109280"
    version_number: "4"
  max_clock_drift: 600s
  proof_specs:
  - inner_spec:
      child_order:
      - 0
      - 1
      child_size: 33
      empty_child: null
      hash: SHA256
      max_prefix_length: 12
      min_prefix_length: 4
    leaf_spec:
      hash: SHA256
      length: VAR_PROTO
      prefix: AA==
      prehash_key: NO_HASH
      prehash_value: SHA256
    max_depth: 0
    min_depth: 0
  - inner_spec:
      child_order:
      - 0
      - 1
      child_size: 32
      empty_child: null
      hash: SHA256
      max_prefix_length: 1
      min_prefix_length: 1
    leaf_spec:
      hash: SHA256
      length: VAR_PROTO
      prefix: AA==
      prehash_key: NO_HASH
      prehash_value: SHA256
    max_depth: 0
    min_depth: 0
  trust_level:
    denominator: "3"
    numerator: "1"
  trusting_period: 1209600s
  unbonding_period: 1814400s
  upgrade_path: upgrade/upgradedClient
proof: CqgHCrcFCgppY3MyMzppYXZsEh9jbGllbnRzL3RoZXN0YXJnYXRlL2NsaWVudFN0YXRlGocFCoQFCh9jbGllbnRzL3RoZXN0YXJnYXRlL2NsaWVudFN0YXRlEtgBCisvaWJjLmxpZ2h0Y2xpZW50cy50ZW5kZXJtaW50LnYxLkNsaWVudFN0YXRlEqgBCgpzdGFyZ2F0ZS00EgQIARADGgQIgOpJIgQIgN9uKgMI2AQyADoGCAQQ4NUGQisKEAiAgMAKEP///////////wESDAigjQYSBAiAxgoYMhoJCgdlZDI1NTE5ShkKCQgBGAEgASoBABIMCgIAARAhGAQgDDABShkKCQgBGAEgASoBABIMCgIAARAgGAEgATABUhZ1cGdyYWRlL3VwZ3JhZGVkQ2xpZW50Gg0IARgBIAEqBQAC1toOIi0IARIGAgTW2g4gGiEgvxsRbh1rKMGqG6wsqws10sabU6FYAg4QTYe77D6gpXIiLQgBEgYECNbaDiAaISClPR3Gae/cF05GsSbwJ1N/T//JBnP0qRitA15JvrPYnSItCAESBgYM1toOIBohIHyPYRpgPLAvDINZw3KhY1k77ClNRcPKCuVHOwdwFwNdIi0IARIGCijW2g4gGiEgm3+XAxvaAvWabC9YZxeqEqWsLGvCxkm0+I3h8tMxnVciLQgBEgYMOtbaDiAaISAeU5bs1id9T0AHX7jZLOnyV0XZKJg+u7gJnCLmlYV5tCIuCAESBxCwAY7QECAaISC07buDGd+PYIU6vrsAKPx0ixmulOxtSjytDhAhBmQTgyIuCAESBxK4Ao7QECAaISDMTGQjYqhPqd+mc7owDg5vrRMRSQz91o091n94Jv9zWyIsCAESKBT2A6rYECBZf4eYsj2RADn8qjIlqjrztOuA95xpn02tteFhUEDINiAK6wEKDGljczIzOnNpbXBsZRIDaWJjGtUBCtIBCgNpYmMSIOVw0YpSPfAyr8yOjXwNQcg5R8ne5dtb7nSVYt+47PBzGgkIARgBIAEqAQAiJwgBEgEBGiD5Vt5/BBmvQQnMGXTh8TcjCr0rjLn1J3de7MoaiBcPIiIlCAESIQHE9fPjQgNd+1LyG5frMQ5vVjJeKlhk4wlb8lEMwdZLMSIlCAESIQG+OsjH8NDvZ5lp2ZiBsA1YL5VNmYKVgCTUOuVJ4nvbOSInCAESAQEaIL+5Th3bDcRtedzpMNM8j+9uOByL4m+WEqv9kg1jC6c8
proof_height:
  version_height: "136726"
  version_number: "0"
proof_path: /thestargate/clientState

closes: #7583


Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

  • Targeted PR against correct branch (see CONTRIBUTING.md)
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Code follows the module structure standards.
  • Wrote unit and integration tests
  • Updated relevant documentation (docs/) or specification (x/<module>/spec/)
  • Added relevant godoc comments.
  • Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
  • Re-reviewed Files changed in the Github PR explorer
  • Review Codecov Report in the comment section below once CI passes

// Issue: https://github.com/cosmos/cosmos-sdk/issues/6567
func QueryTendermintProof(clientCtx client.Context, key []byte) ([]byte, []byte, clienttypes.Height, error) {
height := clientCtx.Height

// ABCI queries at height less than or equal to 2 are not supported.
// Base app does not support queries for height less than or equal to 1.
// Therefore, a query at height 2 would be equivalent to a query at height 3
if clientCtx.Height <= 2 {
if height != 0 && height <= 2 {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

turns out, somewhere down the line, a height of 0 is interpreted as latest height

Copy link
Collaborator

Choose a reason for hiding this comment

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

can you update the comment too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated, I left the error message as is

@codecov
Copy link

codecov bot commented Oct 23, 2020

Codecov Report

Merging #7648 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #7648      +/-   ##
==========================================
- Coverage   54.20%   54.15%   -0.05%     
==========================================
  Files         454      611     +157     
  Lines       30638    38556    +7918     
==========================================
+ Hits        16606    20881    +4275     
- Misses      12392    15543    +3151     
- Partials     1640     2132     +492     

// Issue: https://github.com/cosmos/cosmos-sdk/issues/6567
func QueryTendermintProof(clientCtx client.Context, key []byte) ([]byte, []byte, clienttypes.Height, error) {
height := clientCtx.Height

// ABCI queries at height less than or equal to 2 are not supported.
// Base app does not support queries for height less than or equal to 1.
// Therefore, a query at height 2 would be equivalent to a query at height 3
if clientCtx.Height <= 2 {
if height != 0 && height <= 2 {
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you update the comment too?

@colin-axner colin-axner added the A:automerge Automatically merge PR once all prerequisites pass. label Oct 23, 2020
Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

Ah - nice catch, ACK

@mergify mergify bot merged commit c6cbe3a into master Oct 23, 2020
@mergify mergify bot deleted the colin/fix-q-cmds-7583 branch October 23, 2020 16:31
clevinson pushed a commit that referenced this pull request Oct 29, 2020
* fix query

* update comment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A:automerge Automatically merge PR once all prerequisites pass. C:CLI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix IBC query cmds
3 participants