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

Domain type constructors and getters #661

Open
greg-szabo opened this issue Nov 6, 2020 · 2 comments
Open

Domain type constructors and getters #661

greg-szabo opened this issue Nov 6, 2020 · 2 comments
Labels
domain-types Anything relating to the creation, modification or removal of domain types enhancement New feature or request

Comments

@greg-szabo
Copy link
Member

greg-szabo commented Nov 6, 2020

Issue description

With the introduction of domain type restrictions, we need a consistent way of generating domain-type structs. In this issue we describe the current status, the expected end-status and the ongoing work that gets us there.

Current status

Currently all domain types are expected to be valid-by-construction. The evolution of fuzzy testing might reveal cases where this is not true. In those cases a simple issue ticket with the description of the domain knowledge should be enough to update any of the structs.

There are three types of implementation for domain-type structures. (These evolved through time, hence the discrepancy.)

Type 0: open struct

For some structs, there were no domain knowledge restrictions identified or the restrictions are mirrored in the chosen primitive types already (like "unsigned int" <==> "no negative numbers").

The easiest way to recognize these structs is that they have public fields. This makes them easy to construct natively.

If any domain knowledge is revealed about these types, they should be upgraded to Type 2 structs.

Type 1: restricted through the TryFrom trait

These are simple domain types that have their corresponding protobuf type conversion implemented through the TryFrom/Into traits. These domain types can be generated using only one input parameter (the protobuf type).

The easiest way to spot these structs is that they have private fields but usually no new constructor implemented. (Construction is implemented through the TryFrom trait.) They usually have the Protobuf trait implemented too, and at least one TryFrom/Into trait pairs.

Type 2: constructors and getters

These domain types are the fully formed types we would like to get to. They have a constructor that implements the domain knowledge restrictions and its fields can be reached through getter functions. All TryFrom traits use the constructor to convert from other types.

The easiest way to spot these structs is that they have a new constructor with (usually) multiple input fields.

In general, they have private fields, a new constructor and field getter functions implemented. You can find an example with the SignedHeader type.

Definition of done

When all public structs are Type 2 structs, this issue is done.

Ongoing work that gets us there

  • Taking on this issue all at once is counter-productive: Type 0 structs are convenient for developers and Type 1 structs are less code to implement. There is no problem with the code base as-is.

  • In the long term, we expect that our provided structs implement similar functions so the developers have the same experience no matter what struct they start using.

Based on the above, the proposal is to keep this issue as a recommended way of implementing structs but not make it a focus of a single release. It can be closed and referred back to when new issues are implemented.

@greg-szabo greg-szabo added this to the v0.17.0 milestone Nov 6, 2020
@thanethomson thanethomson removed this from the v0.17.0 milestone Dec 4, 2020
@thanethomson thanethomson added the enhancement New feature or request label Dec 4, 2020
@thanethomson
Copy link
Contributor

One of the challenges with the approach of providing a constructor that validates its input is that our more complex domain types rely on other domain types (e.g. the Block domain type needs a Header, transaction::Data, evidence::Data and an Option<Commit> to construct). One could simply continue to use the current constructor, which looks as follows:

impl Block {
    pub fn new(
        header: Header,
        data: transaction::Data,
        evidence: evidence::Data,
        last_commit: Option<Commit>,
    ) -> Result<Self, Error> {
        // ... validation logic ...
        Ok(Block {
            header,
            data,
            evidence,
            last_commit,
        })
    }
}

The problem with this is that you need to do all the TryInto conversions for all of these related types, which includes error handling/mapping, in the outer context (e.g. within the TryFrom<RawBlock> for Block implementation). If you have TryFrom implementations for multiple different source/serialization types (e.g. Protobuf and JSON), then you need to handle these field conversion failures in each and every TryFrom implementation.

Compare this to an approach where we require dynamic input parameters:

impl Block {
    pub fn new(
        header: impl TryInto<Header>,
        data: impl TryInto<transaction::Data>,
        evidence: impl TryInto<evidence::Data>,
        last_commit: Option<impl TryInto<Commit>>,
    ) -> Result<Self, Error> {
        let header = header.try_into()?;
        let data = data.try_into()?;
        // ... etc.
        // ... validation logic ...
        Ok(Block {
            header,
            data,
            evidence,
            last_commit,
        })
    }
}

In such a case, the conversion attempts for all sub-types takes place within the constructor.

It can also improve developer ergonomics, because you can supply any type for a constructor parameter whose TryInto implementation facilitates conversion.

Are there other such approaches that could facilitate better developer ergonomics, and cut down on the required amount of code to achieve what we're looking to achieve here? Or better yet, are there perhaps libraries that can help to facilitate this with a minimal amount of coding from our side?

@greg-szabo
Copy link
Member Author

I'm going to add here Romain's example of hiding fields and validating them: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=389500b442c9ee697dc8c2511b8a14bb
Might not be the solution we're looking for but it's an interesting concept.

One note to the above use of dynamic input parameters: I think it's great idea. I see the benefit that it would cut down on transformation code among JSON/Protobuf/domain-type. I'm not sure, but if I recall you can always execute TryFrom in place, so this dynamic input implementation might even work for any current use-case too. (I think you can put a regular Header/Data/Data/Commit in there and it will work.)

@mzabaluev mzabaluev added the domain-types Anything relating to the creation, modification or removal of domain types label Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain-types Anything relating to the creation, modification or removal of domain types enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants