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

DFX-Interface: Imports canister code (as actor classes) #1549

Closed
wants to merge 1 commit into from

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Jun 2, 2020

In meeting with @crusso and @matthewhammer about dynamic actor creation,
we converged on Claudio’s proposal that a clean, easy and desirable
way to expose dynamic actor creation in Motoko is to get some help from
dfx.

Big plus of this approach: You can, in one dfx, project, have a Motoko
and a Rust canister module, and the Motoko code can “import” the rust
“code” and then dynamically install instances of that code. So this
allows you write high-performance infinitely scaleable Rust backend
canisters together with a nice declarative Motoko canister that
orchestrates things. But thanks to Candid it also works nicely within
Motoko.

Another big approach is that is seems the fasted way towards clean,
hackfree dynamic actor creation.

The rough idea is that in Motoko you can write

import Worker "class:worker";

and have that to be equivalent to actor class Worker(params) body
where params is imported (via a Candid-like file, .params) and the
body is also imported (by reading the actual .wasm file).

The impact on dfx is that it would do more of the stuff that it
currently does with the .did files: Figure out which canisters depend
on which, get build artifacts from moc (now: .did, then: also the
.wasm (which will be used the Motoko to install) and .params (so
that Motoko knows how to type the the imported class)) and making them
available to the using canister.

(The .params may also be useful for dfx to type-check the argument
to canister canister install for parametrized canisters.)

All filenames and other names up for bikeshedding.

This was written together by @crusso and @matthewhammer.

In meeting with @crusso and @matthewhammer about dynamic actor creation,
we converged on Claudio’s proposal that a clean, easy and desirable
way to expose dynamic actor creation in Motoko is to get some help from
`dfx`.

Big plus of this approach: You can, in one `dfx`, project, have a Motoko
and a Rust canister module, and the Motoko code can “import” the rust
“code” and then dynamically install instances of that code. So this
allows you write high-performance infinitely scaleable Rust backend
canisters together with a nice declarative Motoko canister that
orchestrates things. But thanks to Candid it also works nicely within
Motoko.

Another big approach is that is seems the fasted way towards clean,
hackfree dynamic actor creation.

The rough idea is that in Motoko you can write

    import Worker "class:worker";

and have that to be equivalent to `actor class Worker(params) body`
where `params` is imported (via a Candid-like file, `.params`) and the
`body` is also imported (by reading the actual `.wasm` file).

The impact on `dfx` is that it would do more of the stuff that it
currently does with the `.did` files: Figure out which canisters depend
on which, get build artifacts from `moc` (now: `.did`, then: also the
`.wasm` (which will be used the Motoko to install) and `.params` (so
that Motoko knows how to type the the imported class)) and making them
available to the using canister.

(The `.params` may also be useful for `dfx` to type-check the argument
to `canister canister install` for parametrized canisters.)

All filenames and other names up for bikeshedding.

This was written together by @crusso and @matthewhammer.
@nomeata nomeata force-pushed the joachim/canister-code-import branch from a4f630a to 4fcca28 Compare June 2, 2020 17:07
@nomeata nomeata requested review from hansl and rossberg June 2, 2020 17:08
@nomeata nomeata changed the title DFX-Interface: Imports of _code_ of another canister DFX-Interface: Imports canister code (as actor classes) Jun 2, 2020
@dfinity-ci
Copy link

This PR does not affect the produced WebAssembly code.


As the previous point, but passing `--param-idl` to `moc`.

Theis does not issue any warnings.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Theis does not issue any warnings.
This does not issue any warnings.

@chenyan-dfinity
Copy link
Contributor

chenyan-dfinity commented Jun 2, 2020

How do we generate the .params file, by looking at the actor class type definition? What can go wrong if I manually create a .params file that is different from the expected type?

@hansl
Copy link

hansl commented Jun 2, 2020

I'd like to see a concrete example with 4 files: the Motoko code, its generated IDL and Params, and a Motoko canister that uses those.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 2, 2020

Here is a full example

Input for counter:

// src/canisters/counter.mo
actor class(step : Nat) {
  var x := 0;
  public inc() : async () { x += step };
  public query get() : async Nat = step;
}

Output for moc counter.mo -o counter.wasm:

(module …
  (export "canister_init" … some code expecting a candid-encoded Nat as arg …
)

Output for moc --idl counter.mo -o counter.did:

service Counter {
  inc : () -> ()
  get : () -> (nat) query
}

Output for moc --param-idl counter.mo -o counter.param:

(nat)

An actor using this:

// src/canisters/counterFactory.mo
import Counter "class:counter";
// effectively:
// type Counter = { inc : shared () -> (); get : shared query () -> async Nat;  }
// let Counter : Nat -> async Counter = … magic to install counter.wasm …
actor {
  public new_small_counter() : async Counter {
    let c = await Counter(1);
    await c.inc();
    assert(await c.get() == 1);
    return c;
  };
  public big_small_counter() : async Counter {
    let c = await Counter(42);
    await c.inc();
    assert(await c.get() == 42);
    return c;
  };
}

@chenyan-dfinity
Copy link
Contributor

Since we are extending candid syntax, maybe we can make Candid to support service class directly? So there is no need for a separate .param file.

@matthewhammer
Copy link
Contributor

matthewhammer commented Jun 2, 2020

Since we are extending candid syntax, maybe we can make Candid to support service class directly? So there is no need for a separate .param file.

We discussed this, but the rationale for separating them is that there is a phase distinction that gives very little gain from combining these AST fragments into a single file, with a single AST:

  • The .param file is relevant for compiling code that installs this canister (e.g., compiling the Motoko code of the "admin canister" of a "worker canister" that it installs dynamically, as needed by its "admin logic", in Motoko.)
  • The .did file is relevant for client canisters of the installed service. In the admin/worker example, these canisters would likely be a 3rd party, that is neither the admin nor the worker. They do not care about the .param file, since it's already running, and has thus already been installed.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 2, 2020

We can refine the .param file later into a “management interface file”, also useful for the data needed to determine if an upgrade is safe, but as Matthew says – the .did file descibes a (running) service, and the .params+.did file describe a class (canister module, factory, whatever)

@chenyan-dfinity
Copy link
Contributor

Make sense. So if the .param file has the wrong type, I will get a runtime error during installation, but the compiler cannot check for this.

@crusso
Copy link
Contributor

crusso commented Jun 2, 2020

How do we generate the .params file, by looking at the actor class type definition? What can go wrong if I manually create a .params file that is different from the expected type?

The idea would be to generate the .params file only if the program ends in an actor class, from the (sharable) parameter type of that class.

Once we add the ability to deserialize the installation argument passed to a motoko canister, if you forge the .params file, Motoko would most likely defensively fail to deserialize the argument (same as if you forge the IDL of a Motoko canister method).

I guess this wouldn't easily support recursive canister installation unless we add some system method to access the wasm blob of the current actor class.

I guess that leaves the question of how to dynamically create an actor from a Motoko wasm file that produces an actor, not an actor class. and has no .params file. I could imagine importing that as a trivial class with unit argument, so installation still returns a fresh actor as you would expect from installing the same wasm code several times.

It's a bit weird that the client of an imported actor (class) that is mean to be installed dynamically needs to embed the code for that class (rather than somehow link to some existing code) but that seems to be what our platform supports as of now.

@crusso
Copy link
Contributor

crusso commented Jun 2, 2020

To fix the strange linking/code duplication issue, we could move to a system where we can install the wasm module of a canister class and then separately create canister instances of that class with a system call that takes an installed class id and an argument blob. But I guess that's much further down the line.

@hansl
Copy link

hansl commented Jun 2, 2020

But why not make the Candid service defines an actor constructor declaration (or whatever name you want to give it). Isn't that what this is? This way the Candid file is the only thing we need.

Something like (to take @matthewhammer's example above):

service Counter {
  class (nat);
  inc : () -> ();
  get : () -> (nat) query;
}

I'm making up stuff on the actual syntax, but the point of having a single file has advantages; for example, we don't need 2 files in assets to know a canisters' API (with this we need both did and params). Also we can enhance the ecosystem around Candid, not around Motoko. Other languages are already working with Candid for interfaces. Adding a new file just mean more work for dfx for example (which doesn't read outside of copy-pasting them around for moc).

I don't see why having two files is better than having one.

@hansl
Copy link

hansl commented Jun 2, 2020

the rationale for separating them is that there is a phase distinction that prevents very little gain from combining these AST fragments into a single file

They're both needed for code generation or compilation, and they're both declaring types related to the same API. They also both have to live with the canister since we need to be able to query a canister's API...

Please understand that Rust canisters (and soon Assembly Script) need all the files from output of a build, and not only that, but it needs to produce those outputs too, to be able to interop with Motoko. We would need to move those additional file around, produce it somehow, and use it. And error out when it's not found. For Rust we're already asking people to create a did file by hand. If we just add another file we need to have a good reason for it.

This does not seem to hit that threshold.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 3, 2020

They're both needed for code generation or compilation, and they're both declaring types related to the same API. They also both have to live with the canister since we need to be able to query a canister's API...

That is all only half true:

  • The class parameter is not part of the service API. A user of the service interface has no reason to see it. It doesn't play a role in upgrade checking. You can have many canister classes with totally different parameters that still produce a service with the same API. It is roughly as “internal” as the to-come “stable data upgrade type”.
  • Therefore the same file would not be what you'd store with the canister. If we add the parameters here, you'd have to strip them somehow before uploading.

Another reason: we want to support both driving Candid from the Canister code, but also programming canisters against an externally given Candid interface. In that case .did may become an input to the compiler (for type-checking), and you'd only get .params as an output.

It seems that shuffling two files in parallel is less work for dfx than actually having to parse a candid class file (as you propose) to extract the candid service file. (Or we just leave it there as cruft, but that's kinda leaky.)

@rossberg
Copy link
Contributor

rossberg commented Jun 3, 2020

This is interesting. But I'm missing a vital piece of information here, namely how multi-canister apps get deployed. I can guess that you have one of two ways in mind:

  1. DFX somehow combines the multiple canisters into one, producing some wrapper canister Wasm module embedding them. Not sure what exactly that wrapper would look like.
  2. The platform itself supports multiple Wasm (canister) modules somehow and a way to address them. But this hasn't been on the table so far.

I assume you're thinking the former, but can you elaborate on the specifics?

Taking a step back, I can see two coherent approaches to introducing multi-canister apps:

  1. Motoko-specific. In that case, the special imports are only understood by the Motoko compiler, and it would be its job to merge them into a single module. AFAIU, this is not what you are proposing.
  2. Language-agnostic. In that case, this becomes part of the platform and Candid conventions. But then it should be supported comprehensively, and we should pick namings that don't presume particular language mappings. For example, the URI scheme "class:" doesn't make sense in that approach.

Option 1 does not seem particularly valuable. We could just as well support regular modules with actor classes in them without introducing a new ad-hoc kind of import (let's call that alternative Option 0).

Option 2 on the other hand extends the expressiveness of the platform, which of course adds value. However, it seems like substantially more work than Option 0. So is the only motivation cross-language bundling? Is that relevant for Tungsten?

In particular, if we do Option 2 then I think we should do it a little less ad-hoc. This idea effectively brings a new concept to the (Candid-level abstraction of) the platform. So if we go there, then Candid should have a proper way of speaking about it. In particular, I think "actor classes" should be a first-class notion then, with its own first-class Candid type, and a suitable name ("service factory"? Yikes! :) ).

Let's say we introduce such factory types, e.g.:

service ( <argtype>,* ) -> <actortype>

They are semantically similar to a function type ( <argtype>,* ) -> service <actortype>, but with the extra knowledge that they create a new service. And they have a completely different wire representation.

A .did file could then describe a service factory. When dfx is faced with such a .did for a canister to upload, then it effectively performs the "application". That is, it (1) knows that the user has to supply arguments, and (2) it produces a .did describing the result type, i.e., rewrites the .did to a plain service type to be put on chain.

Probably a bit more work than what's described in the PR. But let's avoid half-assed solutions. If we think this is too much work for Tungsten then I prefer we stick to Option 0 for now. ;)

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 3, 2020

The proposal here is actually option 1: moc reads the counter.wasm and embeds it in the resulting module as data - no different than what would do with local actor expressions (and what we did in M1).

There are no new system features proposed or required, and this is of course crucial for something we want to have running in <30 days! Something to keep in mind before we stroll into the territory of what we might envision in an ideal future.

I disagree that this is Motoko-specific. Rust, with str_include or similar, can very easily support these canister class imports. (Very easily = no harder than the existing canister id-and-interface feature)

We could just as well support regular modules with actor classes in them without introducing a new ad-hoc kind of import (let's call that alternative Option 0).

Similar, but not quite the same: you could not include a Rust or raw Wat canister classes then (as you could only import .mo files, if only moc is involved)

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 3, 2020

A .did file could then describe a service factory. ... dfx rewrites the .did to a plain service type to be put on chain.

This just means we bundle the proposed .params with the .did as produced by moc. Something that @hansl also wants. If nobody is worried about the complexity of dfx editing these files (rather than just copying), that variant is fine with me.

@crusso
Copy link
Contributor

crusso commented Jun 3, 2020

The proposal is meant to be language agnostic, so languages can instantiate canisters written in other languages in a strongly typed way. The idea was that you could have a motoko manager canister instantiating Rust worker canisters if you wanted. Or an all motoko solution.

I actually don't see any reason to introduce the service constructor type now unless we also want to pass factories/constructors in message/store them in data structures. Nice to have maybe, but not necessary now.

I sympathise with the view that the idl for a wasm module should be self-contained. Maybe we can leave it to the compilers to project the relevant parts of the idl instead of leaving it to dfx.

A canister import takes the type of service declaraton and codomain of a factory declaration, a factory import takes the domain and range.

We don't need a first class service type, just an optional argument on the final service exported by the IDL.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 3, 2020

Oh, I completely missed that Andreas is proposing to add first-class classes to Candid types…

@rossberg
Copy link
Contributor

rossberg commented Jun 3, 2020

My argument is along these lines:

  1. If we only care about Motoko for now (which seems fine for Tungsten) then Option 0 is the simplest.

  2. If we want cross-lang then it becomes a platform concept. And then you'll eventually want some way of passing these around, e.g., imagine you want to virtualise dfx on-chain. I agree that first-class factories in Candid are not necessary for Tungsten (although they don't seem difficult either), but we should at least have a design that anticipates them as a future feature.

Having the parameters in the IDL is more consistent with that and also nicer for other reasons mentioned. It kind of fills a gap we currently have (I suppose Candid currently has no way of checking whether the user supplies the right parameters?). The only downside is that dfx has to parse and output Candid now, but that would already be the case with the params file. And it avoids questions like whether or how I can use type defs from the .did in the .param file.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 3, 2020

Having the parameters in the IDL is more consistent with that and also nicer for other reasons mentioned.

As you think about extending that format, would you also add, for example, the type information about the stable data to that file? Or would that still be a separate interface file?

@rossberg
Copy link
Contributor

rossberg commented Jun 3, 2020

As you think about extending that format, would you also add, for example, the type information about the stable data to that file? Or would that still be a separate interface file?

Hah, good question. You could make the argument that if Candid rewrites the .did anyway then why not strip private stable vars?

I think the fundamental difference is that stable variables are purely a Motoko concept, not a platform notion. It is not realistic to me that other languages will define binary-compatible representations, or even a similar concept (even if @crusso would like that). Moreover, I believe that the types used to declare stable decls need to be Motoko types, not Candid types -- the latter are likely to be insufficient and too imprecise for this purpose, depending on what we gonna allow in stable vars eventually.

@nomeata
Copy link
Collaborator Author

nomeata commented Jun 4, 2020

We could just as well support regular modules with actor classes in them without introducing a new ad-hoc kind of import (let's call that alternative Option 0).

If we think this is too much work for Tungsten then I prefer we stick to Option 0 for now.

So that settles it?

@rossberg
Copy link
Contributor

rossberg commented Jun 4, 2020

I'm actually intrigued by the factories-in-Candid idea. I'm fine going that direction (even if only half the way for now), if we find somebody willing to do the dfx-side work. Otherwise I'd say let's keep that for after Tungsten.

@chenyan-dfinity
Copy link
Contributor

chenyan-dfinity commented Aug 13, 2020

Why? If this is the official syntax for install arguments then we should support it in dfx too. There's no value in stripping those parameters.

We are talking about the interface change, and it is generic for all languages: When calling dfx install, dfx/ic:00 looks at the candid file with the initialization types and apply those parameters to the canister_install method. After install, we get the real interface for the canister by stripping out the initialization part.

Before installation:

factory Counter : (step : nat) -> {
  inc : () -> ();
  get : () -> (nat) query;
}

After installation:

service Counter : {
  inc : () -> ();
  get : () -> (nat) query;
}

My proposal is to make the initialization as a comment:

// init: (step : nat)
service Counter : {
  inc : () -> ();
  get : () -> (nat) query;
}

@hansl
Copy link

hansl commented Aug 13, 2020

My question remains; Why even change the DID file?

@rossberg
Copy link
Contributor

@hansl, I'm not sure what you're asking. We're introducing parameters to canister instantiation. Dfx needs to know about them and their types, so that they can be supplied and encoded when the canister is installed. This is independent of what language a canister was compiled from.

OTOH, after installation, the parameters have been eliminated. The type that a client of the installed canister sees does not involve them. It's like the difference between a function and its result. It doesn't make sense to associate a type with the running canister that still contains parameters. Besides the domain mismatch, they are in fact a private implementation detail of how that canister was created, and it would be a form of abstraction leak to expose them.

Furthermore, probably at some point after Sodium, we should support dynamic actor creation coherently on the platform and in Candid. That suggests the ability to pass around uninstantiated canisters. With that it becomes even more apparent that there is a fundamental difference between types service {...} and service (...) -> {...} -- the former is a running canister, the latter a Wasm module for a canister (a canister "factory"). So we better be careful to get the type domains straight.

@rossberg
Copy link
Contributor

@chenyan-dfinity:

I think the extension only applies to the main actor, not service types in type definitions.

I think of this as a Sodium limitation. To truly support dynamic canister creation, it is natural to ask for the ability to pass around uninstantiated actors. So I assume this eventually has to become a proper part of the type algebra itself.

If at all possible, I would prefer to avoid hacks like assigning semantics to comments.

The service keyword has to change as well, because service type is , not -> { }.

Well, we could use a different keyword, but it's not necessary. It doesn't seem too bad to reuse the same keyword for both instantiated and uninstantiated services.

@nomeata
Copy link
Collaborator Author

nomeata commented Aug 14, 2020

It doesn't seem too bad to reuse the same keyword for both instantiated and uninstantiated services.

Although having a dedicated, concise, clear keyword, and hence term, for what we have been calling “canister module”, “service factory”, “canister factory”, “uninstantiated service”, “actor class constructor”, etc. would be very helpful… (no solution offered here, though).

@rossberg
Copy link
Contributor

Although having a dedicated, concise, clear keyword, and hence term

Yeah, fair enough. But I don't have a good suggestion either. None of "factory", "constructor", "class", "template", "module" are a good fit. It's becoming apparent that it can only be "functor". :)

@matthewhammer
Copy link
Contributor

matthewhammer commented Aug 14, 2020

It's becoming apparent that it can only be "functor". :)

I am in favor.

But I also wonder: Are we just embracing future confusion, and a lifetime of explaining naming ambiguities? (since, I assume you mean "functor" in the SML sense, not the category theory sense).

@matthewhammer
Copy link
Contributor

matthewhammer commented Aug 14, 2020

Although having a dedicated, concise, clear keyword, and hence term, for what we have been calling ...

Totally agree we need a name for this (important) concept. No strong feelings about it, in particular.

I do like "canister functor" the most, especially if we think of that is being the best matching concept from the ones that are available (Andreas is excluding all other choices, helpfully). Though as I mentioned, I do wonder about confusing people that have some passing familiarity with category theory, but I think we don't need to worry about that population among our target developers (?), or we can just trust them to understand that there are pre-existing name clashes, and we aren't adding new ones (?).

@nomeata
Copy link
Collaborator Author

nomeata commented Aug 14, 2020

I would intuitively expect a “canister functor” to be something that takes a canister, and returns another canister.

@rossberg
Copy link
Contributor

In case the smiley wasn't clear enough: I was joking. Dudes, "functor" would be even worse than most of the other alternatives.

If we have to choose among the options so far, "service constructor" would probably be the least bad option for the Candid type. But "canister constructor" sounds weird as well.

@chenyan-dfinity
Copy link
Contributor

chenyan-dfinity commented Aug 14, 2020

To truly support dynamic canister creation, it is natural to ask for the ability to pass around uninstantiated actors.

Hmm, what happens when these actors get instantiated? Do we want to update all did files mentioning the actor?

It's also unclear how to do the rewriting in the higher order case.

type s = service (nat) -> { f: (s) -> (s) }
service : s

@matthewhammer
Copy link
Contributor

matthewhammer commented Aug 14, 2020

FWIW, I think of these actor classes as being like a "parametric service" or a "parameterized service" or something in that vein, in terms of the word service, e.g., at the level of Candid.

Dudes, "functor" would be even worse than most of the other alternatives.

Show me the lattice. : )

Also, there's still "Cannery." : )


Or perhaps: "generalized service" or "abstracted service" or "abstract service"

@rossberg
Copy link
Contributor

@chenyan-dfinity, rewriting the higher-order case seems doable. Or we could just disallow writing it that way.

Your other question is a good one. Ties in with the whole question about where the Candid is stored and how it is retrieved. And how that would extend to "constructors", if at all.

Hm, maybe there is a point to putting the parameters into a separate file after all. At least until we know the answer to the previous question.

@chenyan-dfinity
Copy link
Contributor

For the higher order case, there are exponentially many possibilities. The main service can be rewritten to any of the following types, depending on what service gets instantiated.

...
type s3 = service { f: (s2) -> (s1) };
type s2 = service (nat) -> { f: (s1) -> (s) };
type s1 = service { f: (s) -> (s) };
type s = service (nat) -> { f: (s) -> (s) };
service : s*

Putting the parameters into a separate file is the same as having a constructor syntax on the main service definition, maybe this is all we need?

service(step: Nat) : { f : () -> (nat) }

@crusso
Copy link
Contributor

crusso commented Aug 19, 2020

@chenyan-dfinity this seems to be precisely the syntax I was suggesting above #1549 (comment)

I don't see any reason for introducing constructor types as a first class notion now, since we can always do it later as an extension of Candid, and later redefine the second-class syntax as sugar for the higher-order one.

I regard the constructor syntax as the interface for the module (i.e code) to be installed, and its co-domain the interface of the installed canister. It doesn't seem unreasonable for dfx to take some module
s constructor interface and derive the interface of its running canisters. Indeed, it needs the module interface in order to execute dfx install using textual arguments, the instantiated, canister interface to provide types for canister imports, and the full constructor/module interface to provide types for class imports.

@chenyan-dfinity
Copy link
Contributor

chenyan-dfinity commented Aug 19, 2020

Ah right. I was thinking of the arrow syntax earlier. With this syntax, I think we don't need rewriting, as @nomeata mentioned:

dfx uploads that .did file unmodified with the canister. Users of the installed service asking for its interface will see the install arguments, even though that is none of their business. Maybe not a problem.

  • dfx never uploads any .did file to the canister. We haven't decided where to put the did file. So nobody can get the interface of a running canister. There is no need for rewriting at least at the moment.
  • To support class import, we should not rewrite the local did file even after installation.
  • Even if the did file comes with the canister, service(int) : { f : () -> (nat) } is still type correct, compared with service (int) -> { f: () -> (nat) }.
  • If we want to support reinstall/upgrade after the canister first gets installed, we will need to know the constructor type even when the canister is running.

@rossberg WDYT?

@hansl
Copy link

hansl commented Aug 19, 2020

dfx uploads that .did file unmodified with the canister. Users of the installed service asking for its interface will see the install arguments, even though that is none of their business. Maybe not a problem.

That was exactly my question; why rewrite the DID and why is it a problem?

dfx never uploads any .did file to the canister

I'm trying to make my case we should (somewhere non related to this question) at least have the interface available.


Sorry I wasn't clear. When I said "why even change the DID file", I meant "why even have DFX rewrite the did output". The format changes are fine (as long as it's backward compatible I guess), but there is no need for DFX to rewrite it.

@chenyan-dfinity
Copy link
Contributor

why even have DFX rewrite the did output

The rewriting is implemented in the candid library, but dfx needs to call the rewriting function after dfx install.

@hansl
Copy link

hansl commented Aug 19, 2020

The rewriting is implemented in the candid library, but dfx needs to call the rewriting function after dfx install.

Why? So that people won't see the init function arguments?

@rossberg
Copy link
Contributor

@chenyan-dfinity:

For the higher order case, there are exponentially many possibilities. The main service can be rewritten to any of the following types, depending on what service gets instantiated.

...
type s3 = service { f: (s2) -> (s1) };
type s2 = service (nat) -> { f: (s1) -> (s) };
type s1 = service { f: (s) -> (s) };
type s = service (nat) -> { f: (s) -> (s) };
service : s*

I'm afraid I don't see what's exponential. The rewriting would just need to insert one type definition:

> ...
> type s' = service { f: (s) -> (s) };
> service : s'

Or, if it wants to avoid the repetition, modify s as well:

> ...
> type s = service (nat) -> s'
> type s' = service { f: (s) -> (s) };
> service : s'

(assuming we'd allow this syntax).

Putting the parameters into a separate file is the same as having a constructor syntax on the main service definition, maybe this is all we need?

service(step: Nat) : { f : () -> (nat) }

I don't see why that is necessary either. If we want to avoid larger rewriting for now, okay, but we can still use the more consistent syntax

service : (step: Nat) -> { f : () -> (nat) }

while not introducing first-class service constructor types. At least that would be forward-compatible and stylistically coherent.

@hansl:

That was exactly my question; why rewrite the DID and why is it a problem?

Because it assigns the wrong kind of type. It's a category error, an abstraction leak, and a potential forward-compatibility problem (what if we eventually want to support both kinds on the platform?).

Compare to OO. If you have

class C(x : Nat) { ... }
let c : T = C(1)

would you suggest that T ought to be Nat -> C? And if it was, what'd be the type of the constructor C itself?

@crusso
Copy link
Contributor

crusso commented Aug 20, 2020

service : (step: Nat) -> { f : () -> (nat) }

I'm ok with that provided you mean a grammar like:

<prog>  ::= <def>;* <actor>;?
<def>   ::= type <id> = <datatype> | import <text>
<actor> ::= service <id>? : (<tuptype> ->?)  (<actortype> | <id>)

without extending <actortype> itself.

However, this a subtly different arrow type (at least at the platform level) than the one for actor methods so I'd argue (not very strongly)
the other syntax is preferable (and familiar to OO programmers).

@rossberg
Copy link
Contributor

Yes, that's what I mean.

The syntax for methods is analogous and probably equally unfamiliar to Joe OO Programmer.

@chenyan-dfinity
Copy link
Contributor

I think we are conflating two different did files:

  • did file generated from Motoko (or other canister code). This file represents the interface of the code, and should not change. This allows us to import actor class. We should be able to import actor class even if the corresponding canister is installed on the network. This is the only did file that dfx generates, and we should not change this did file.
  • did file representing the interface of the canister. This did file comes with the canister, so we need to strip the init arguments from the Motoko generated did file. Since we haven't bundled the did file with the canister in any way, there's nothing to be done here.

I think both files are useful, and we should keep both, not mutating a single did file to satisfy both purposes.

@nomeata
Copy link
Collaborator Author

nomeata commented Aug 31, 2020

We should be able to import actor class even if the corresponding canister is installed on the network.

You want to import an actor class from a running canister? That may be feasable, but kinda odd (OO analoge: Taking an object and getting it’s class’ constructor). If anything, I thought we’d get platform-hosted modules (but not running canister, with state) for that. But you are right, if that becomes a use case, then uploading the unmodified .did is correct.

The use case I am thinkig of the second one you mention, importing an actor from a running canister. This is the use case that I have assumed as agreed upon since over a year, that has one proposed spec in #1510. and it’s a bit embarrassing that we never managed to implement it so far.

I guess I didn’t consider the first use case, sorry if that caused confusion, and thanks for pointing out the two.

@chenyan-dfinity
Copy link
Contributor

chenyan-dfinity commented Aug 31, 2020

You want to import an actor class from a running canister?

hmm, I wasn't thinking of a running canister. I just mean from Motoko side, import "canister:A" and import "class:A" refers to two different did files. Locally, the stripping probably happens during resolving the canister import, which is purely a moc change.

@nomeata
Copy link
Collaborator Author

nomeata commented Sep 1, 2020

import "canister:A" should use the same .did file that importing a runing service would, so that file I would expect to alrady have the parameter list stripped.

import "class:A" would have the file with parameter list (as per the sprit of #1549)

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

Successfully merging this pull request may close these issues.

7 participants