ADR034: Account Rekeying

[sunnya97]

Description

This PR includes an ADR for a new MsgType to allow the updating of the public key for an account.

Corresponding implementation in #7452

---

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

[clevinson]

Thanks @sunnya97!

Can you update the # of this to ADR034 ? We currently have WIP ADR's up to ADR033 already as active pull requests.

[robert-zaremba]

We can still make it possible to prune accounts by introducing a new message: MsgNukeAccount. When an author will delete HIS account, then the deposit is released.

[sunnya97]

I implemented the "deposit" as an extra Gas charge rather than an actual monetary deposit. With a gas charge, we could provide a gasrefund when an account is nuked, but that could potentially turn into GasToken shenanigans like on Ethereum.

[robert-zaremba]

I really like this ADR.

I also started drafting a similar thing 2 weeks ago: to decouple account address from a public key and proposed it as one of the goals for SDK. I call it re-keying (this is in fact a term used by Algorand). During the review of #7086 I noted we need to support accounts with native addresses, specifically:

• multisigs with native public key
• accounts which will enable dynamic keys.

@aaronc in #7086 (comment) pointed to a Group Module we are designing at Regen Network, which solves the latter point using a separation of an account object from the policies.

What's good, all this ideas will play together very well:

• we can use the Address algorithm from ADR-028 to compute an initial address.
• we can update all account structures to add a fixed address field, which will be computed using the aforementioned Address function.
• All accounts will support re-keying.

[aaronc]

Concept ACK

Can you add a boolean EnableChangePubKey param?

[aaronc]

ACK

One question: do we allow two accounts with the same pubkey but different addresses?

[zmanian]

I'm mostly comfortable with this ADR.

here is my reasoning.

1. It's presumably not a mandatory for a chain to implement this message type.

2. I'd oppose implementing it on the hub without other fee improvements like consensus min fee but all of those things are coming.

[sunnya97]

@aaronc Yeah, I think that should be allowed. I can't think of a reason it shouldn't be, as msgs explicitly declare the originating address, and don't depend on the signature/pubkey for that

[ethanfrey]

Robert, I don't say Ethereum is perfect, and the points you raise have little to do with key rotation.

What I am saying is this separation of concepts has allowed people to reason about behavior - especially on the off-chain / wallet / layer 2 side. I do not see any clear analysis of how one could build off-chain infrastructure with this key rotations.

e.g. if I open a state channel with an account, I would only open it with a pubkey tied to an EOA. I cannot open it with some on-chain multisig contract as one party. So, I design a state channel solution with that understanding. Suddenly, even these stable addresses may switch out pubkeys while packets are in transit in Layer 2. How can I build a state channel that is secure in that situation?

I think this ADR should have detailed analysis and proposed solutions for a few such problems. Basically any layer 2 solution will face these issue.

[ethanfrey]

I think this can cause lots of unintended and undiscovered impacts.

Please take a look at https://github.com/CosmWasm/cosmwasm-plus/tree/master/contracts/cw1-whitelist which is a super minimal contract. If you give tokens to this, it provides one clear address and you can swap under that. However, the account address never has a pubkey associated with it. And it actually provides more options that what you describe here and is easily extended in eg cw1-subkeys

I think you could do something very similar in a native module, making a lot of "sub-module addresses", which may have a rotating pubkey (or keys) behind them. But since the address never has a pubkey directly when querying x/auth, then you never create confusion. Either an account is controlled by an off-chain pubkey or controlled by on-chain logic.

[ethanfrey]

Flexible on chain multisigs are done in CosmWasm and coming soon in SDK. I don't see this point as a reason to add the pubkey rotation which has many negative impacts on client code.

[jackzampolin]

Tend to agree here @ethanfrey

[ethanfrey]

validator operators typically use multisigs. They could just move to new on-chain multisig as soon as they are available.

[ethanfrey]

It is far more than a UI issue. If you have any intelligent client node, like a long-living program, especially if it does any caching or rollup. This will make life harder, possibly impossible to resolve.

[jackzampolin]

I tend to agree with @ethanfrey here, this is pretty huge change and it seems like the usecases we are envisioning are taken care of by the group module. Breaking a bunch of client side assumptions right as we are getting to stable client side libs seems bad.

[jackzampolin]

Tend to agree here @ethanfrey

[robert-zaremba]

During the call I mentioned that I made a comment with few ideas how to support verifiable off-chain signatures:

1. Attach blockchain transaction ID to the MsgSignData (as a required field). Verifier checks with the blockchain that the transaction exists and is signed by the same key.
2. Similarly to the above one: instead of attaching a transaction ID, we can attach a merkle proof + block height, proving that at that time, an account has assigned correct public key.
3. Update this ADR (ADR-34) to store the past public keys. If someone will rekey, we will have a stack of the previous keys. Dapp can query that stack to search for a public key.

I think the first one is the easiest one. Note it's also sound, because whenever a user will rekey, he will need to do a transaction with his old key. So for every users' public key, if the key was associated with a blockchain account, there will be always an associated blockchain transaction.

[aaronc]

In the call it seemed like people thought this EnableChangePubKey param was superfluous... but I'm also not sure it hurts to have it.

[sunnya97]

@ethanfrey

There is a nice separation in Ethereum from "external accounts" - which are tied 1:1 with a pubkey for authorization and "contract accounts' - which are controlled by logic.

There is a push in the Ethereum to actually remove EOAs and have only contract accounts (see EIP 2938 - Account Abstraction)

Client tooling. One big hope was coordination off-chain - things like authenticating as in ADR 036, but also wallet connect, creating multisigs, etc.

By designing client tooling to be focused on working with accounts that cannot change pubkeys, this will likely leave accounts that can (on-chain multisigs, smart contract wallets, etc) out in the dust. This is not desirable (this happens already on some off-chain coordination tools like Snapshot). Instead, we should make it clear that it is expected that in the general-case, the authentication logic of accounts is mutable and composed of logic. And for applications where this does not work, these applications should explicitly special case fixed pubkeys in their modules. See next section:

especially on the off-chain / wallet / layer 2 side. I do not see any clear analysis of how one could build off-chain infrastructure with this key rotations. ... e.g. if I open a state channel with an account, I would only open it with a pubkey tied to an EOA.

For things that need long term stable keys, their modules should explicitly lock users into such keys. For example, when designing a payment channel module, the payment channel shouldn't be based on the public key of the accounts, but rather a new public key should be stored in the Payment channel struct. This could be the same public key as the account's public key, but it would not change if the account's public key changes.

[sunnya97]

I've added the pruning details to the ADR requested by @robert-zaremba and @alexanderbez

I've also added to the ADR the addition of option 3 from @robert-zaremba's list of proposals on how to support verifiable off-chain signatures, as its the one I find the simplest for clients to implement.

Update this ADR (ADR-34) to store the past public keys. If someone will rekey, we will have a stack of the previous keys. Dapp can query that stack to search for a public key.

[robert-zaremba]

This could be the same public key as the account's public key, but it would not change if the account's public key changes.

I would coin a term: Special Purpose Account :)

[sunnya97]

This could be the same public key as the account's public key, but it would not change if the account's public key changes.

I would coin a term: Special Purpose Account :)

I was thinking it wouldn't even be an Account. The struct for a Payment channel would look like:

struct PayChan {
    user1Addr        sdk.Address
    user1PubKey   crypto.PubKey

    user2Addr       sdk.Address
    user2Pubkey   crypto.PubKey

    ...
    challengePeriod
    amount
    lastState
    ...
}

It uses user1Addr and user2Addr as the source of funds and payout addresses, but uses user1PubKey and user2Pubkey as the pubkeys involved with the payment channel update signing

[alexanderbez]

@sunnya97 let's remove the EnableChangePubKey section. Also, what arguments do you have for this feature that CosmWasm would not address.

I think we can keep and merge this ADR as proposed but I think we need more discussion around it, especially if CosmWasm already buys us this functionality.

[alexanderbez]

We could instead devise a way to provide a means of one-time "migrating".

[sunnya97]

@alexanderbez

While you could use CosmWasm to do rotating keys, there are many SDK-based chains that will probably not want to integrate CosmWasm functionality, due to variety of reasons. And it will likely be a much while longer before CosmWasm is included on major chains like the Cosmos Hub. This is a much simpler feature that can be integrated very quickly

[robert-zaremba]

Thanks for updating the description and discussing the outcomes.

Addressed

[alexanderbez]

Why just create your own module @sunnya97 and publish it to Atlas? Developers/chains that need such functionality w/o CosmWasm can simple use your module.

[sunnya97]

@alexanderbez well that's why I proposed having a param that can toggle it on and off, to effectively do that without needing to maintain two different modules