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

Localhost client audit concerns #3193

Closed
10 tasks done
Tracked by #3034
chatton opened this issue Feb 23, 2023 · 5 comments
Closed
10 tasks done
Tracked by #3034

Localhost client audit concerns #3193

chatton opened this issue Feb 23, 2023 · 5 comments
Assignees
Milestone

Comments

@chatton
Copy link
Contributor

chatton commented Feb 23, 2023

  • Replace the store.Get usage in verifyNonMembership with store.Has
  • Document in verifyMembership and verifyNonMembership that the store passed is the ibc store.
  • In verifyMembership and verifyNonMembership use _ as variable name for unused parameters.
  • In GetTimestampAtHeight Add a comment that this function is not doing exactly the same of what it does for other client types and that it just returns the latest context timestamp.
  • Create a sentinel proof for localhost and re-add validation in ValidateBasic. #3194
  • Move types.LocalhostID to exported and have two variables LocalhostClientId and LocahostConnectionId.
  • Add migration docs.
  • Update DefaultAllowedClients godoc
  • Rename getClientStateAndKVStore to getClientStateAndVerificationStore. Use clientID instead of clientState.ClientType in if check (to make sure that the whole ibc store is only returned for the sentinel localhost client).
  • Use AttributeKeyConnectionId in all events in 04-channel/keeper/events
@chatton chatton changed the title localhost client audit concerns Localhost client audit concerns Feb 23, 2023
@damiannolan
Copy link
Member

I'll pick up the sentinel proof work and create a separate PR for it.

@colin-axner
Copy link
Contributor

We need to add migrations docs for the upgrade handler as well?

@damiannolan
Copy link
Member

Addressing points 1-4 in #3199

@colin-axner
Copy link
Contributor

Addressing 6, 8, 9, 10

@colin-axner
Copy link
Contributor

Nice one y'all!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

4 participants