Feedback on Concerto 1.0 note

Summary

Looks good. To me it sounds like a more detailed documentation for basically the design we had agreed on for Concerto 1.0 so I'm happy with it. A few comments below.

Things that were not so clear in the original design but are clearer here:

• assets and participants are identified but without a timestampe
• events and transactions are not identified (I think the code has to be changed for that) but with a timestamp
• some changes are proposed to serialization for $identifier (although this was probably underspecified in our original notes)

Notes

NB: One of the basic properties we want to get out of the new type system is that if C2 extends C1 we want to be able to use any object of class C2 in a place where an object of class C1 is expected. Major comments below are fundamentally related to guaranteeing this propery.

Major comments

1. inheritancee for identifiers

Within a hierarchy of identified types, a given type may redefine its identifying field.

Do we want to support ^^^^? It may make the code more complex, as to determine whether a field is identifying for a type you need to look at not just at the type declaration, but at the type of the instance.

I really think we want to avoid this.

If we have

concept C1 identified by cId {
  o String cId
  o String name
}
concept C2 identified by otherId {
  o String otherId
}

Then what would be the identifier for:

{
  $class: 'C2',
  cId: '123',
  otherId' 234',
  name: 'foo'
}

If we allow identifiers redefinitions, we do not know which of those fields is the identifier. It will be especially confusing in the context where an object of class C2 is passed to e.g., and Ergo function with a C1 parameter (whose body might expect cId to be an identifier).

2. inheritance for asset/participant/asset etc

Concerto enforces that types that are declared as asset or participant must ultimately extend concerto.Asset or concerto.Participant respectively. This ensures that the modeller can denote the role of a type when declaring it, and document/enforce its ultimate super type.

Not clear that we want to enforce this ^^^^ restriction?

How is that a restriction? I think we want all assets to be assets and all participants to be participants or it will get very confusing. The same way we want all classes to have one super type (Concept) I think we want all assets to have one super class Asset and all participants to have on super class Participant.

Minor comments

1. inheritance for assets

In the way we word things we have to be a little careful when we say things like:

The keyword asset is semantic sugar to denote that a type extends concerto.Asset:

asset Truck identified by vin {
    o String vin
    o String make
    o String model
}

Because we can also write things like:

> asset Truck identified by vin extends Vehicle {
>     o String vin
>     o String make
>     o String model
> }

I think the implementation and intent is clear, but we may want to word it along those lines:

The keyword asset can be used to declare classes that extend concerto.Asset. When using extends the super type should also be an asset, when extends is not use, the super type is concerto.Asset.

2. system v user defined identifiers

I quite liked the currrent implemeentation which always have a $identifier field for all identifiable classes. This ensures you can write code that uses $identifier even if you don't know the name of the identifier without having to inspect the type hierarchy.

3. A superficial reading of the note gives the impression that timestamp is under user control, but I really think it should be treated like systems identifiers? (i.e., added by the validation as we do in Concerto 0.82). See also: https://github.com/accordproject/concerto/issues/242

The kind of issue that will come up with this is what do we do with Ergo code like this:

declare function f() {
  return MyTransaction{
    name: "foo"
  };
}

I don't think we want users to have to write:

declare function f() {
  return MyTransaction{
    timestamp: now(),
    name: "foo"
  };
}

(This is essentially a variant of part of Fletch's issue here: https://github.com/accordproject/ergo/issues/734#issuecomment-584843505)