[Object Ownership #1] Add Authenticator #292

lxfind · 2022-01-28T18:11:31Z

This is the first step to implement #99.
It adds a new Authenticator type that represent either a user account address or an object ID.
The owner of an object is now Authenticator.
This PR doesn't use it in anyway, but merely just try to get all code compile with this new type.

sblackshear · 2022-01-29T02:37:41Z

fastx_types/src/base_types.rs

@@ -84,6 +84,21 @@ pub type AuthorityName = PublicKeyBytes;
 pub type ObjectID = AccountAddress;
 pub type ObjectRef = (ObjectID, SequenceNumber, ObjectDigest);

+#[derive(Eq, PartialEq, Debug, Clone, Copy, Deserialize, PartialOrd, Ord, Serialize, Hash)]
+pub enum Authenticator {


Wondering: should we call this type Address and the Address variant Ed25519PublicKey? E.g., say we support multiple signing schemes via extensions to the enum (e.g., ECDSA for compatibility with Eth/Bitcoin wallets, BLS to support tricks with aggregation), there will be several variants of the enum that look like addresses authenticated by a public key.

My mental model of an address (which may be wrong/different from what others think--please LMK!) is:

Something you can send an object to

Something that can (possibly) be the signer of a transaction

It seems like all current and future variants of Authenticator probably satisfy both of these conditions (with the exception of ObjectID and (2), which is somewhat of a special case).

The Address type is already taken though: https://github.com/MystenLabs/fastnft/blob/main/fastx_types/src/messages.rs#L20
It represents either an address on FastX or an external address on primary chain.

So maybe we could name this one FastXAddress?

E.g., say we support multiple signing schemes via extensions to the enum, there will be several variants of the enum that look like addresses authenticated by a public key.

While there may be several signing schemes at play eventually, quoting from #99:

In the future, an address should be the hash of an authenticator, e.g.

If that's our north start, there are advantages (in terms of validity, UX, feasibility of error correction, ...) to having that has be normalized and universal, i.e. one single fixed-length byte string format - not that there aren't ways of embedding variants in this type.

IOW, I think even with several key variants, we may get away with a single variant for the Address concept.

The Address type is already taken though: https://github.com/MystenLabs/fastnft/blob/main/fastx_types/src/messages.rs#L20
It represents either an address on FastX or an external address on primary chain.
So maybe we could name this one FastXAddress?

Makes sense to me. And reminds me: I think the notion of "primary" is a fastpay artifact and we should kill anything related to it.

#310 for this

Lets not block this for the sake of future potentially features. I think calling it Address in this context is fine.

huitseeker

LGTM

huitseeker · 2022-01-29T16:25:58Z

fastx_types/src/base_types.rs

@@ -84,6 +84,21 @@ pub type AuthorityName = PublicKeyBytes;
 pub type ObjectID = AccountAddress;
 pub type ObjectRef = (ObjectID, SequenceNumber, ObjectDigest);

+#[derive(Eq, PartialEq, Debug, Clone, Copy, Deserialize, PartialOrd, Ord, Serialize, Hash)]
+pub enum Authenticator {


E.g., say we support multiple signing schemes via extensions to the enum, there will be several variants of the enum that look like addresses authenticated by a public key.

While there may be several signing schemes at play eventually, quoting from #99:

In the future, an address should be the hash of an authenticator, e.g.

If that's our north start, there are advantages (in terms of validity, UX, feasibility of error correction, ...) to having that has be normalized and universal, i.e. one single fixed-length byte string format - not that there aren't ways of embedding variants in this type.

IOW, I think even with several key variants, we may get away with a single variant for the Address concept.

sblackshear · 2022-01-29T17:01:55Z

If that's our north start, there are advantages (in terms of validity, UX, feasibility of error correction, ...) to having that has be normalized and universal, i.e. one single fixed-length byte string format - not that there aren't ways of embedding variants in this type.

I think stating all the goals like this is helpful--we want a concept of "address" that:

supports multiple authentication schemes (e.g., different signature schemes, auth via object ID, multisig, ...)
is fixed length
is used as the signer of a transaction
is the thing that you transfer Objects to in Move code

The path I imagined to this was defining an Authenticator just as @lxfind has here, and then using the hash of this Authenticator as "address". But this PR seems to be going toward using Authenticator directly as an address (instead of its hash). I would be delighted by from a code cleanliness perspective, but I don't know how to go forward with this while satisfying the "fixed length" condition).

Curious what you all think of these goals + different paths toward hitting all of them.

gdanezis

I like the flexibility this change gives us with direct applicability to the next 3 PRs. Lets land it.

gdanezis · 2022-01-31T12:32:58Z

fastx_types/src/base_types.rs

@@ -84,6 +84,21 @@ pub type AuthorityName = PublicKeyBytes;
 pub type ObjectID = AccountAddress;
 pub type ObjectRef = (ObjectID, SequenceNumber, ObjectDigest);

+#[derive(Eq, PartialEq, Debug, Clone, Copy, Deserialize, PartialOrd, Ord, Serialize, Hash)]
+pub enum Authenticator {


Lets not block this for the sake of future potentially features. I think calling it Address in this context is fine.

huitseeker · 2022-01-31T12:51:24Z

The path I imagined to this was defining an Authenticator just as @lxfind has here, and then using the hash of this Authenticator as "address". But this PR seems to be going toward using Authenticator directly as an address (instead of its hash). I would be delighted by from a code cleanliness perspective, but I don't know how to go forward with this while satisfying the "fixed length" condition).

I think it's a relatively straightforward refactor to go from Authenticator-as-key to Authenticator-as-hash-of-key. Typically, we'd :

pick one hash function, and its length (this is one place where I'd advise against genericity, for once),
write a sealed trait for what we expect a key to do (e.g. signing, having a well-defined over-the-wire byte format),
use the former to hash the later, store the output in Authenticator,
for now, have just one implementation of the public key trait.

sblackshear · 2022-01-31T14:58:32Z

The path I imagined to this was defining an Authenticator just as @lxfind has here, and then using the hash of this Authenticator as "address". But this PR seems to be going toward using Authenticator directly as an address (instead of its hash). I would be delighted by from a code cleanliness perspective, but I don't know how to go forward with this while satisfying the "fixed length" condition).

I think it's a relatively straightforward refactor to go from Authenticator-as-key to Authenticator-as-hash-of-key. Typically, we'd :

pick one hash function, and its length (this is one place where I'd advise against genericity, for once),

write a sealed trait for what we expect a key to do (e.g. signing, having a well-defined over-the-wire byte format),

use the former to hash the later, store the output in Authenticator,

for now, have just one implementation of the public key trait.

Sounds like we're on the same page + thanks for the detailed plan!

huitseeker · 2022-01-31T16:15:12Z

@sblackshear opened #318

lxfind · 2022-01-31T18:31:46Z

Committing this as it is with Authenticator + Address variant. Added TODO to rename/extend it to support more.

lxfind requested review from huitseeker, patrickkuo, sblackshear, gdanezis and oxade January 28, 2022 18:11

lxfind changed the title ~~Add Authenticator~~ [Object Ownership #1] Add Authenticator Jan 28, 2022

sblackshear reviewed Jan 29, 2022

View reviewed changes

huitseeker approved these changes Jan 29, 2022

View reviewed changes

gdanezis approved these changes Jan 31, 2022

View reviewed changes

huitseeker mentioned this pull request Jan 31, 2022

[Refactor] use hashes of Public Keys in Authenticator #318

Closed

Add Authenticator

b8f6ea7

lxfind force-pushed the add-authenticator branch from 7f779f7 to b8f6ea7 Compare January 31, 2022 18:13

lxfind merged commit 7f7c115 into main Jan 31, 2022

lxfind deleted the add-authenticator branch January 31, 2022 18:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Object Ownership #1] Add Authenticator #292

[Object Ownership #1] Add Authenticator #292

lxfind commented Jan 28, 2022

sblackshear Jan 29, 2022

lxfind Jan 29, 2022

huitseeker Jan 29, 2022

sblackshear Jan 29, 2022

sblackshear Jan 29, 2022

gdanezis Jan 31, 2022

huitseeker left a comment

huitseeker Jan 29, 2022

sblackshear commented Jan 29, 2022

gdanezis left a comment

gdanezis Jan 31, 2022

huitseeker commented Jan 31, 2022

sblackshear commented Jan 31, 2022

huitseeker commented Jan 31, 2022

lxfind commented Jan 31, 2022

[Object Ownership #1] Add Authenticator #292

[Object Ownership #1] Add Authenticator #292

Conversation

lxfind commented Jan 28, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huitseeker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sblackshear commented Jan 29, 2022

gdanezis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huitseeker commented Jan 31, 2022

sblackshear commented Jan 31, 2022

huitseeker commented Jan 31, 2022

lxfind commented Jan 31, 2022