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

Audit all Penumbra-related todo items #27

Merged
merged 1 commit into from
May 14, 2024
Merged

Audit all Penumbra-related todo items #27

merged 1 commit into from
May 14, 2024

Conversation

zbuc
Copy link
Member

@zbuc zbuc commented May 1, 2024

We audited all existing todo items, replacing them with unimplemented when appropriate and adding more context to the outstanding todo items.

We still have some outstanding questions, for example:

  • Should we implement a trusting period in the Penumbra Hermes configs?
  • The evidence command is unimplemented and expects a CosmosSDK chain; is this command important?
  • Do we need to support genesis reset parameters?
  • Do we need to implement IBC channel/client state queries in Penumbra?
  • Do we need to implement cross-chain queries?

Closes #19

@conorsch
Copy link

conorsch commented May 6, 2024

Should we implement a trusting period in the Penumbra Hermes configs?

We at least need to make a decision about this (see discord discussion in https://discord.com/channels/824484045370818580/930154881040404480/1235729085515436092), and preferably document it somewhere, and I don't see a better place than the code right now. So my vote is yes.

Do we need to support genesis reset parameters?

Sounds related to, but distinct from, #22, but I lack context.

@avahowell it'd be lovely to have your input on @zbuc's questions!

@avahowell
Copy link

Should we implement a trusting period in the Penumbra Hermes configs?

This should be derived from the chain's reported unbonding period, as it's security critical. That's what's currently done in the hermes impl (trusting period = 2/3 of the unbonding period), I think we should not add an ability in the config to override this

The evidence command is unimplemented and expects a CosmosSDK chain; is this command important?

Yes, we should have the ability to automatically detect and submit evidence of equivocation in hermes.

Do we need to support genesis reset parameters?

Yes, this is required in order to support preserving channels after an upgrade. @erwanor and I implemented and exercised this functionality in #22 on one of the recent testnet upgrades

Do we need to implement IBC channel/client state queries in Penumbra?

We have at least some of these implemented already IIRC (that's how the pcli q ibc functionality works). Are there specific ones that are missing?

Do we need to implement cross-chain queries?

Inferring from the PR that this refers to ICS-31; I'm not sure what parts of this we need to support.

@zbuc
Copy link
Member Author

zbuc commented May 14, 2024

@avahowell currently unimplemented IBC state queries:

  • query_upgraded_client_state
  • query_upgraded_consensus_state
  • query_host_consensus_state
  • maybe_register_counterparty_payee
  • query_incentivized_packet
  • query_consumer_chains

@zbuc zbuc merged commit 7213f1d into main May 14, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Fix tests and audit TODO items
3 participants