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

[aip-55] Generalize Transaction Authentication and Support Arbitrary K-of-N MultiKey Accounts #263

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

davidiw
Copy link
Contributor

@davidiw davidiw commented Oct 16, 2023

No description provided.

aips/aip-55.md Outdated
---
aip: 55
title: Generalize Transaction Authentication and Support Arbitrary K-of-N MultiKey Accounts
author: davidiw, hariri
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hariria :)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix it

aips/aip-55.md Outdated Show resolved Hide resolved
Copy link
Contributor

@hariria hariria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

## Summary

Transactions submitted to Aptos contain a `RawTransaction` and a `TransactionAuthenticator`. The `TransactionAuthenticator` authorizes the execution of the transaction by the set of senders or approvers of the accounts within the transaction. The `TransactionAuthenticator` contains a mixture of general purpose authenticators like fee payer and multiagent but also some very specific types for single sender transactions such as Ed25519, MultiEd25519, and Secp256k1. Thus adding a new cryptographic proof for authorizing a transaction requires a new `TransactionAuthenticator` and `AccountAuthenticator` variant to support both the single and multiparty transaction authenticaton.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never had time to understand this: why do we have both TransactionAuthenticator and AccountAuthenticator?

aips/aip-55.md Outdated

## Alternative solutions

This work could have focused purely on multi-key solutions that work across the board. This would have eliminated the optimization for single-sender applications. Doing so increases the burden for creating adding cryptographic algorithms for authorization.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "creating adding" should have been just "adding"?

}

pub struct MultiKeyAuthenticator {
public_keys: MultiKey,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to define the MultiKey struct.

Also, why not replace MultiKey by a Vec<AnyKey>?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we use MultiKey in the AuthenticationKey generation, will add context

aips/aip-55.md Outdated

New cryptographic algorithms can be added by adding an appropriate data structure and verify method under `AnyKey` and `AnySignature`.

The `MultiKeyAuthenticator` supports up to 255 keys and up to 32 signatures, as 32 signatures are the maximum allowed in a single transaction.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you clarify why is 32 the maximum? Is it because you assume signatures are, say 32 bytes, so $32 \times 32$ bytes is $960$ bytes which gets close to a potential 1KiB limit in the code?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, does this mean in a $t$-out-of-$n$ multikey address cannot have $t &gt; 32$?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a legacy component of the code, will clarify


Instead of doing this at an account level, we could have used Multisigv2. In Multisigv2, multiple on-chain accounts share a single account. This is similar to the model here, where multiple keys share the same account. The caveat being that if keys are misplaced in a Multisigv2, the assets within those accounts are lost. Those accounts require minimally gas fees, so there would be inevitable loss.

## Specification
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see nothing mentioned here about how the address (a.k.a., authentication key) of such "multikey" accounts is computed...

Copy link
Contributor

@alinush alinush left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Explain the MultiKey struct
  2. Explain what multikey addresses look like (and how to deal with complications when wallets lose some of the PKs underneath a multikey address and, as a result, are not able to reconstruct the address)
  3. Maybe take the opportunity to explain the difference between TransactionAuthenticator and AccountAuthenticator
  4. Writing is incredibly difficult to follow for me due to poor use of terminology / undefined terms

Copy link
Contributor Author

@davidiw davidiw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

feedback addressed

}

pub struct MultiKeyAuthenticator {
public_keys: MultiKey,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because we use MultiKey in the AuthenticationKey generation, will add context

aips/aip-55.md Outdated

New cryptographic algorithms can be added by adding an appropriate data structure and verify method under `AnyKey` and `AnySignature`.

The `MultiKeyAuthenticator` supports up to 255 keys and up to 32 signatures, as 32 signatures are the maximum allowed in a single transaction.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is a legacy component of the code, will clarify

@thepomeranian thepomeranian requested a review from alinush October 18, 2023 22:14
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

Successfully merging this pull request may close these issues.

4 participants