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

Investigate improvements to support interface subscriptions #14067

Closed
3 tasks done
stefanobaghino-da opened this issue Jun 2, 2022 · 8 comments
Closed
3 tasks done

Investigate improvements to support interface subscriptions #14067

stefanobaghino-da opened this issue Jun 2, 2022 · 8 comments
Assignees
Labels
component/java-ecosystem Java development experience component/js-ecosystem TypeScript and React.js bindings component/json-api HTTP JSON API discussion Things to be discussed and decided roadmap/interfaces https://digitalasset.atlassian.net/browse/DAML-56 team/ledger-clients Related to the Ledger Clients team's components.

Comments

@stefanobaghino-da
Copy link
Contributor

stefanobaghino-da commented Jun 2, 2022

Interface subscription are currently being designed in this document.

They are an important piece of enabling upgradable applications using interfaces. As such, having them as part of the first stable release is important. We should understand how those could be implemented on top of our components, in particular:

Engage with the design document and report the changes that are deemed necessary to make our components support interface subscriptions.

@stefanobaghino-da stefanobaghino-da added component/json-api HTTP JSON API component/js-ecosystem TypeScript and React.js bindings team/ledger-clients Related to the Ledger Clients team's components. labels Jun 2, 2022
@stefanobaghino-da stefanobaghino-da added the component/java-ecosystem Java development experience label Jun 2, 2022
@stefanobaghino-da stefanobaghino-da added the roadmap/interfaces https://digitalasset.atlassian.net/browse/DAML-56 label Jun 2, 2022
@S11001001 S11001001 added the discussion Things to be discussed and decided label Jun 3, 2022
@S11001001
Copy link
Contributor

S11001001 commented Jun 7, 2022

LF Interface library

It's not clear whether subscriptions will make it possible to universally treat an interface type as an alias or "pun" for its associated view type. For the purposes of these designs, I'm assuming that it will not. So interfaces will remain separate from the serializable type decls.

An ast interface will include a reference to the identifier of the associated view type, a serializable type. We copy that to DefInterface in the InterfaceReader.

Assuming that LF will require that view types are records with no type parameters, we include utilities

// EnvironmentInterface
def resolveInterfaceViewType(n: Ref.TypeConName): Option[Record.FWT]
// Interface
def resolveInterfaceViewType(n: Ref.QualifiedName): Option[Record.FWT]

// Interface companion
def interfaceViewResolver(findInterface: PartialFunction[PackageId, Interface])
    : PartialFunction[Ref.TypeConName, Record.FWT]

Will break

Anything downstream that depends on the structure of DefInterface.

@S11001001
Copy link
Contributor

S11001001 commented Jun 7, 2022

JSON API

Introduce 3 subtypes for TemplateId.RequiredPkgs that have been resolved by lookup with the internal PackageService as a template ID, interface ID, or "either". This is because we currently assume that a fully-qualified template ID can simply be turned into a template filter for the ledger API. The interface subscriptions feature invalidates that assumption, and probably some significantly more insidious cases (as that is just the one I spotted last I was examining the query code), so it seems worth doing away with the assumption statically.

For the purposes of the DB cache, we will consider interface IDs exactly like template IDs, and their payload types to be exactly their interface view types. So the contracts table will continue to be partitioned by template ID: updates for a template ID won't affect an interface ID's partition, even if the same contract ID is present in both.

  • permit multiple rows per contract ID

On the contract table, remove the contract_k primary key constraint on contract_id, replacing it with a primary key constraint on (contract_id, tpid). We must advance the schemaVersion as a result.

In Queries#deleteContracts, include the template IDs as arguments (I think we only need one, but adjust as needed), and when selecting rows for deletion. It sounds appealing to delete every view associated with a contract ID at once, but this entails race conditions more easily dealt with by not doing this.

When assembling a transaction or ACS request, we always set include_create_arguments_blob = false, include_interface_view = true.

Permitted multiqueries

  • coerce interface subscriptions to present JSON API request/response model

When building InsertDeleteSteps for the database, we essentially want one create event per matching {contract ID, contract type ID} pair, and likewise for archives. For MVP, we will also apply that restriction to multiquery responses.

Given the set of contract type IDs (template IDs or interface IDs) CTIDS = {ctid₁, ctid₂, …, ctidₙ} in a query, if there exist ctidⱼ, ctidₖ such that ctidⱼ ≠ ctidₖ and ctidⱼ is an interface ID, then the request will be rejected immediately.

This is because this condition may require the response to account for the violation of one of these invariants, which would be incompatible with our current response format:

  1. There exists at most one create event per contract ID.
  2. Each create event has one payload format, fully determined by the contract type ID.

The caveat is that this forbids arbitrary multiplexing of query streams.

Request/response format

The above restriction means that the request and response format do not change; they merely gain additional semantics. In particular, templateId accepts interface IDs as well for the /v1/query, /v1/stream/query, /v1/fetch endpoints, just as is currently supported for /v1/exercise.

In any of these new cases, if the interface ID does not have an associated interface view, the request will be rejected with HTTP 400. (NB: /v1/exercise does not require an interface view, so this restriction should not be universally or retroactively applied to all endpoints.)

Transforming transaction streams to responses

The above restriction squeezes the possible transaction streams into what can fit in current response formats. That will be done as follows for create events, with respect to the set of CTIDS from the multiquery.

  1. If template_id ∈ CTIDS, use create_arguments as the payload, and ignore any interface views.
  2. Otherwise, interface_views should contain exactly one element whose interface_id ∈ CTIDS.
    1. If its view_status is not ok, treat that as a server error similarly to how JSON conversion failures on payloads are currently treated (i.e. it doesn't break the stream, but gets reported opaquely in the response block).
    2. Otherwise, use the view_value as the payload, and discard the template_id and create_arguments.

Archive events in WS streams must also be transformed, because the template_id when matching interface ID cases will be irrelevant to the request.

  1. If template_id ∈ CTIDS, use that as the templateId for the JSON archive event.
  2. Otherwise, check the (JSON API) package service for every interface that template_id implements. Exactly one of these should be ∈ CTIDS; report that interface ID as the templateId.
    1. If 0 or >1 such interface IDs match, report that as a stream element error in the usual fashion.

Will break

Nothing.

@S11001001 S11001001 removed their assignment Jun 8, 2022
@stefanobaghino-da
Copy link
Contributor Author

@S11001001 @ray-roestenburg-da I don't see anything here about what the API would look like neither for the HTTP JSON API Service nor for the Java bindings. Is there something that is blocking you here in some respect? Have you reached out to the Participant team to understand their current PoC and how they plan to move forward?

@S11001001 S11001001 self-assigned this Jun 23, 2022
@S11001001
Copy link
Contributor

S11001001 commented Jun 23, 2022

Java bindings

(Largely with respect to #14033 as of this writing.)

In com.daml.ledger.javaapi.data, extend the following classes to reflect changes to their protobuf counterparts:

  • add Filter.Interface, the pair of bools from proto InterfaceFilter
  • InclusiveFilter: add a interfaceIds: @NotNull Map<@NotNull Identifier, @NotNull Filter.Interface>. Add a static method ofTemplateIds to replace the current single-arg constructor, now deprecated.
  • CreatedEvent: add interfaceViews: @NotNull Map<@NotNull Identifier, @NotNull DamlRecord> and failedInterfaceViews: @NotNull Map<@NotNull Identifier, @NotNull Status>. NB: Status might not be the best choice; if we can narrow it down, we should.

Existing constructors should default these to empty maps, and be deprecated with a porting note, a version note, and fresh GitHub issue reference similar to #14039.

Include proto to/from conversion changes; beware, this is not fully type-checked.

Will break

Users of transaction streams will likely encounter the deprecation warning on new InclusiveFilter, whose Javadoc directs them to InclusiveFilter.ofTemplateIds. If we don't do this, because we don't have a type-level distinction between template IDs and interface IDs, users are apt to try adding interface Identifiers to this existing set, which will not work.

@cocreature
Copy link
Contributor

Two comments/suggestions on the JSON API changes:

  1. Your query changes are slightly ill-defined: You can check that at the beginning of the query but a package upload for a running query can break this for a running query. In the participant, we were very explicit that it’s a requirement to never serve stale data meaning a package upload needs to take effect immediately. If I understand your proposal correctly, you check the restriction once at the beginning leaving you with the question of what you do if that invariant gets violated, e.g., you then get a create event that implements two interfaces. You could abort the stream or something like that. Alternatively, you could also enforce a much stricter rule: If there is an interface in CTIDS, then CTIDS must be a singleton set containing only that interface. I think for the typescript bindings (without multiplexing which is I believe still disabled), that would be sufficient and it’s easy to understand and resilient against later package uploads.
  2. I recommend to not exploit the restrictions you impose on the queries in the output format and instead have an output format that is sufficiently general that you can support other queries later without changing the format just for those. That leads to a more complex output format right now but it’s much easier to extend it without breaking backwards compatibility or ending up with weird inconsistencies. One option would be to follow what the gRPC does and introduce an object with the keys being interface ids and the values being the the interface value.

@cocreature
Copy link
Contributor

Java bindings changes look very reasonable.

@S11001001
Copy link
Contributor

S11001001 commented Jun 30, 2022

Alternatively, you could also enforce a much stricter rule: If there is an interface in CTIDS, then CTIDS must be a singleton set containing only that interface.

@cocreature Thanks for pointing out the invalidation case. I will use this rule instead.

2. I recommend to not exploit the restrictions you impose on the queries in the output format and instead have an output format that is sufficiently general that you can support other queries later without changing the format just for those.

The simple format is one of the goals of applying these restrictions. "The payload type of an interface ID is its interface view" is a simple metaphor to extend existing query uses. The more general format, such as any gRPC-inspired one, penalizes the simple single-query use case to support full multiquery generality.

To this point, the multiquery cases have imposed no additional complexity on single-query cases; it has always been possible to treat them as "one or list-of-n" functions as common in Python et al. The only artifact is matchedQueries, which can simply be ignored by single-query users. With this new feature, something's gotta give, and I do not think it should be the simple, more intuitive interface enabled by the single-query restrictions. Instead, the complex, multi-query use case should yield.

From discussions elsewhere arose the suggestion from @garyverhaegen-da that we can implement the restricted form with the very nice-to-use "payload of interface ID is interface view" extension, and leave a more general multiquery result type to a new endpoint in a later release. This is the plan I currently have in mind.

@S11001001
Copy link
Contributor

S11001001 commented Jul 1, 2022

JavaScript bindings

(This somewhat depends on #14134. Consider some form of interface contract ID from that proposal to be a prerequisite of this design.)

An interface contract ID type for interface I will be ContractId<I>, such that

type I = Interface<"pkgid:mod:I"> & IV

where IV is the interface view type. Depending on the chosen semantics for the final LF implementation, this can probably be a reference, and the interface view should be included in the module dependencies of an interface. As interface views must be serializable records, there are no possible IVs such that Interface<x> & IV = never.

Generate a Serializer<I> and include it in the interface companion. Ensure that the companion is generated for interface types as the JsSerializerConRef by genTypeCon in TsCodeGenMain.hs. The serializer should be trivially derivable from IV's serializer. Additionally, the type of the companion should include Template or a supertype thereof, that can be used as a typed argument to the query and fetch functions in @daml/ledger.

@daml/ledger functions

CreateEvents contain a ContractId<T> and payload T. This matches the above type declarations for interfaces with views, and the responses of JSON API.

The fetch and various query functions take a Template<T, K, I> argument. If the interface companions cannot include Template in their intersection, but instead a new supertype of Template, the argument types of these functions should change to this new supertype.

Query (the typed query language) will work as-is, because the extends object case, which effectively removes the "branding" of Interface<I>, always applies.

Will break

This design breaks nothing itself, but #14134 or a similar required design breaks some current interface choice invocation expressions. Neither that nor this design breaks any pre-interface features.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/java-ecosystem Java development experience component/js-ecosystem TypeScript and React.js bindings component/json-api HTTP JSON API discussion Things to be discussed and decided roadmap/interfaces https://digitalasset.atlassian.net/browse/DAML-56 team/ledger-clients Related to the Ledger Clients team's components.
Projects
None yet
Development

No branches or pull requests

5 participants