Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

a User instance should always have a stream Address #268

Open
arnauorriols opened this issue Jul 6, 2022 · 2 comments
Open

a User instance should always have a stream Address #268

arnauorriols opened this issue Jul 6, 2022 · 2 comments

Comments

@arnauorriols
Copy link
Contributor

arnauorriols commented Jul 6, 2022

Currently, the User structure contains a field stream_address, whose type is Option<Address>. When a User is instantiated, this field is set to None, and when either User::create_stream() or User::receive_announcement() are called, it is populated with the address of the stream.

If we take a step back, we can realize that a User without stream_address is pretty much useless. In fact, any user must call either User::create_stream() or User::receive_announcement() to actually do anything else. This connascence of execution might seem harmless in trivial examples, but in use cases where the creation of the user becomes more dynamic and complex, it can become the source of hidden runtime bugs. These use cases would greatly benefit from this being a compile error instead of a runtime one.

In practice, this means converting the stream_address field to Address instead of Option<Address>. This also means that the constructor must take a stream_address: Address parameter, and create_stream() or receive_announcement() should have been called before the User instance is created.

If we take a step back again, we can realize that receive_announcement() is the main procedure to connect to a stream, regardless of being the author of a subscriber. create_stream() will only be called on new streams, in a one-off execution path. However, there's a distinction between author's and subscribers with regard of the additional information that they are supposed to provide to connect to a stream. Technically, an author only needs the base topic of the stream in order to infer the stream's Address, because it already knows its own identity. All these factors led me to conclude in the following API proposal:

    let mut author = User::builder()
        .with_identity(Ed25519::from_seed(author_seed))
        .with_transport(transport.clone())
        .with_psk(psk.to_pskid(), psk)
        .as_author(BASE_BRANCH)  // <-- addition
        .build().await?;  // gets the announcement message or creates it if it does not exist yet

    let mut subscriber = User::builder()
        .with_identity(Ed25519::from_seed(subscriber_seed))
        .with_transport(transport.clone())
        .as_subscriber(author.identifier(), BASE_BRANCH)  // <-- addition
        .build().await?;  // gets the announcement message or fails if it does not exist yet

This proposal has 2 main benefits, apart from addressing the main issue:

  • For each kind of user, asks only what is truly necessary, no duplication prone to potential inconsistencies
  • Abstracts the logic of creating an stream if it does not exist yet.

The only weirdness of this proposal is on what is the intuitive behaviour when the user for some reason uses both builder methods. In my opinion, the last one should prevail:

    let mut subscriber = User::builder()
        .with_identity(Ed25519::from_seed(subscriber_seed))
        .with_transport(transport.clone())
        .as_author(BASE_BRANCH)
        .as_subscriber(author.identifier(), "another base topic") 
        .build().await?;  // connects as subscriber to author's "another base topic" stream
@arnauorriols
Copy link
Contributor Author

One thing I'm particularly not convinced: what is more usable for subscribers: provide the author's identifier and the stream topic, or just the stream address? The former lets parties exchange the author's identifier once and benefit from the human-readability of topics. The later is less coupled to the current implementation of the address generation, and is only one string that needs to be shared, instead of two.

@arnauorriols
Copy link
Contributor Author

After some experimentation, I'm putting the idea on ice, for several reasons:

  • It moves the creation/fetching of the announcement message at the time of instantiation of the User, which has its own usability drawbacks
  • It requires the duplication or indirection of the fetching logic, muddying the implementation for questionable benefits.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant