thanethomson
commented
3 years ago 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
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
newconstructor 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
newconstructor with (usually) multiple input fields.In general, they have private fields, a
newconstructor and field getter functions implemented. You can find an example with theSignedHeadertype.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.