romac
commented
1 year ago Thanks so much for taking the time to write this up 🙏
Overall, I fully agree with your assessment that exposing the underlying signing key type is a layering violation and prevents non-Cosmos chains from integrating with Hermes.
Your proposal sounds good as well, but before we can move forward with it, we need to figure out how to deal with the hermes keys command, which I guess would either:
a) need to be adapted to work with both Cosmos and Penumbra keys
b) be moved under a new cosmos subcommand, leaving it up to you to add a penumbra keys subcommand should you find the need for it
c) left as-is
Option (a) might be the most satisfying for both of us, provided m we manage to make it work for both types of chain without changing the interface of these commands too much.
Option (b) would be a breaking change of the CLIs, which we try to keep as infrequent as possible. But if (a) proves not to be doable or desirable, I am not completely moving everything Cosmos-specific under an aptly named subcommand for clarity now that we will have support for at least one non-Cosmos SDK chain.
Option (c) might be a good option in case you don't ever plan to add Penumbra-specific commands for managing keys.
What do you think?
Summary
The current ChainEndpoint trait makes too restrictive assumptions about the shape and role of signing keys. This makes it difficult to properly add support for chains like Penumbra with different authorization behaviors.
Problem Definition
The ChainEndpoint trait used to create pluggable, chain-specific backends has a SigningKeyPair associated type.
This type must implement the SigningKeyPair trait, which demands the following behavior:
This is a layering violation, because it bakes in assumptions from the Cosmos SDK that may not apply to other chains. In fact, none of these properties obtain for Penumbra keys:
Penumbra signing keys are never used on arbitrary byte slices, because they do not sign the encoding of the transaction data. Instead, they sign over the transaction's effect hash, similar to Bitcoin's SIGHASH mechanism, and there is one signature per spend, rather than a single signer for the transaction. And each signature is produced with a fresh randomization of the verification key. There is no API to sign arbitrary byte slices. Instead, a complete TransactionPlan is passed to the custodian, who returns AuthorizationData used to build the transaction.
Penumbra signing keys control many (2^32) logical accounts simultaneously. Accounts are not disclosed to anyone other than the user who controls them. Each account has multiple addresses, a stable "default address" as well as 2^96 ephemeral addresses. Neither accounts nor addresses are disclosed by shielded transactions. Addresses (representing payment capability) are only disclosed by the user to desired counterparties, but are not published on-chain.
Penumbra's key heirarchy does not have a pair of keys. Instead it has a tree of keys, with variously attenuated capability: a spending key, representing spending capability, a viewing key, representing viewing capability, and many publicly unlinkable addresses, representing the capability to pay specific accounts controlled by that user's spending key.
However, although these assumptions are all violated by Penumbra's design, there is no real problem with relaying. There's just a different authorization mechanism. The only problem here arises from the layering violation of lifting chain-specific authorization mechanisms into the common ChainEndpoint trait.
Proposal
Transaction authorization should move out of the ChainEndpoint trait into the endpoint implementation itself.
Rather than demanding an opinionated KeyPair, the ChainEndpoint should only require the endpoint to provide the Signer field that should be used when building related packets.
Signing transactions should be an opaque and internal responsibility of the endpoint implementation.
Acceptance Criteria
For Admin Use