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

improve error message for identifier validation #152

Open
1 of 5 tasks
colin-axner opened this issue May 4, 2021 · 3 comments
Open
1 of 5 tasks

improve error message for identifier validation #152

colin-axner opened this issue May 4, 2021 · 3 comments

Comments

@colin-axner
Copy link
Contributor

colin-axner commented May 4, 2021

Summary

  • identifier validation does not specify which identifer is empty
 query packet commitments: rpc error: code = InvalidArgument desc = rpc error: code = InvalidArgument desc = identifier cannot be blank: invalid identifier: invalid request 
  • if proof height is > client state height, indicate client should be updated

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added this to the 1.1.0 milestone May 10, 2021
@colin-axner
Copy link
Contributor Author

colin-axner commented May 20, 2021

I'm pretty sure this means a relayer got beat to the relaying process:

error: Caught error:  Error when broadcasting tx FA98E443F1D4CAEE7EFF6B4FF6CEA08329441860B0CA8BA0E42A22CBB842B625 at height 737. Code: 13; Raw log: failed to execute message; message index: 0: acknowledge packet verification failed: commitment bytes are not equal: got ([153 160 69 75 247 52 141 25 227 227 230 148 0 25 2 166 1 226 192 199 63 195 241 114 137 177 88 6 77 114 98 6]), expected ([]): invalid packet
  • The error message should indicate such

@colin-axner colin-axner self-assigned this May 20, 2021
@colin-axner
Copy link
Contributor Author

colin-axner commented May 20, 2021

  • Already relayed packets should have well known associated error codes so relayers don't retry the transaction. See comment

@colin-axner colin-axner removed their assignment Jul 7, 2021
@crodriguezvega crodriguezvega modified the milestones: 1.1.0, vNext Sep 17, 2021
faddat referenced this issue in notional-labs/ibc-go Feb 23, 2022
Convert -tags argument to comma separated list
@crodriguezvega crodriguezvega removed this from the Q2-2022-backlog milestone Mar 31, 2022
@colin-axner colin-axner changed the title Improve error messages improve error message for identifier validation Feb 20, 2023
@colin-axner
Copy link
Contributor Author

To complete this issue we need to improve the error message on the identifier validation. Two solutions exist:

  • update all calls to ClientIdentifierValidator, ConnectionIdentifierValidator, ChannelIdentifierValidator to wrap with the type of identifier which failed validation, like so
  • Wrap the error from within ClientIdentifierValidator etc. We still likely need to wrap from the caller, for example when validating counterparty identifiers

It probably makes sense to wrap from within the ClientIdentifierValidator, and do additional wrapping whenever the identifier is also something more naunced like a counterparty client ID or a counterparty connection ID

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

No branches or pull requests

2 participants