Add support for bound function arguments #291

rossberg · 2021-11-03T08:48:14Z

This specs out the Candid side of the "closure" extension I suggested here.

WDYT, does this work?

Edit: see also #292, for a complementary extension with type dynamic.

matthewhammer · 2021-11-03T15:54:39Z

LGTM!

My only nit is about the other companion PR, and how to compose this PR with that one in a larger motivating example.

Let's say that the wallet call wants to return some data, and we don't know what it is, or even its arity?

How would we adapt this?:

type wallet = service {
  topup : (amount : nat) -> ();
  forward : (call : () -> ()) -> ();
}

I see three options for the non-unit return type of call and forward:

Blob
dynamic (see Add support for type dynamic #292)
a variant of dynamic that I will call dynamics (e.g., see comment Feature Idea: Generic data #245 (comment))

1. Use `Blob`

forward : (call : () -> Blob) -> Blob;

And then assume that this Blob holds a candid response sequence, not a single candid value.

Pros: Blob exists today in Candid and Motoko.
Cons: Blob conversions are explicit, and perhaps not ergonomic yet. They could also have type errors.

2. Use `dynamic` (see #292)

forward : (call : () -> dynamic) -> dynamic;

Pros: Could be a safer option to option 1, since conversions would have dynamic checking and auto trapping (I presume).
Cons: Unfortunately, this API is not quite as "general" as the first, since dynamic cannot encode a sequence of values, but only one value. See option 3.

3. Use `dynamics` (see comments to #292)

forward : (call : () -> dynamics) -> dynamics;

Pros: Actually solves the problem (I think).
Cons: Furtherest of the options from what we have today.

spec/Candid.md

chenyan-dfinity · 2021-11-03T16:51:44Z

spec/Candid.md

+```
+type wallet = service {
+  topup : (amount : nat) -> ();
+  forward : (call : () -> ()) -> ();


Suggested change

forward : (call : () -> ()) -> ();

forward : (call : func () -> ()) -> ();

syntactically, how do we know if the function has bound arguments? Does the closure supports return type?

You mean, know it by the type? We don't, it's always allowed. Do you think that would cause issues for bindings?

I see. The existing example has a callback function, which can have bound arguments as well. You are adding another example to illustrate the use of bound arguments, which makes me think there may be syntactic difference for the bound args.

So any function reference can have bound arguments. The return value will be decoded according to the function signature, right? That means we need an abstraction for argument tuples. And if we use the dynamic type as return type, dynamic represents argument tuples instead of a single argument.

That means we need an abstraction for argument tuples.

Yep. That's my thinking too, and more agreement in the comments for #292

chenyan-dfinity · 2021-11-03T16:55:59Z

spec/Candid.md


 M(ref(r) : principal) = i8(0)
 M(id(v*) : principal) = i8(1) M(v* : vec nat8)
+
+M* : <val>* -> <datatype>* -> i8*
+M*(v^N : <datatype>^N) = leb128(N) M(v : <datatype>)^N


this will be a breaking change? or the bound argument is stored in R which was not used before?

No, R is only used if there are reference types that need it, as before. The type is determined from M.

But I think this is a breaking change in so far that a receiver not understanding the new encoding of closures would choke on it. The only way to avoid this is by indeed introducing a separate closure type as a future type, it seems. Hm, that would be ugly...

A new type seems to be appropriate. Not breaking existing stuff is part of our value proposition, and given the complexity of this maybe it’s good if a service can say that they really only support plain function references, but not closures?

Okay, introduced a new (future) type for closures and made it a supertype of func.

rossberg · 2021-11-03T17:00:43Z

For the time being I think it's fine if this pattern only worked for functions of fixed arity. If we wanted variadic abstraction, maybe we could make a tuple record coercible to a parameter list and vice versa, analogous to what we do in Motoko.

Perhaps n-ary functions as a primitive in Candid were a mistake (as @nomeata always argued, though for different reasons). If we introduced such a coercion, could we retroactively pepper over it?

nomeata

A method call f(x,y) now does not just mean “encode (x,y) via Candid and send”, because f could be a closure. This requires

a heap representation of a Candid closure f, consisting of (likely)
- service reference
- method name
- original type table from the message that carried f (pruned to the actual used types? copied as is? Probably pruning is not possible if the bound values are of a future type we don’t understand)
- the candid encoded value, kept as an opaque blob (no need to decode, and because of future types we probably can’t)
the ability to merge that type table with the type table we’d already generate for the explicitly passed (x,y)

That’s a quite high “implementation complexity price” to be paid, so I am overall quite wairy of this.

nomeata · 2021-11-03T18:08:25Z

spec/Candid.md


 M(ref(r) : principal) = i8(0)
 M(id(v*) : principal) = i8(1) M(v* : vec nat8)
+
+M* : <val>* -> <datatype>* -> i8*
+M*(v^N : <datatype>^N) = leb128(N) M(v : <datatype>)^N


Where is the type of v encoded? I’d expect something like

M*(v^N : <datatype>^N) = leb128(N) I(<datatype>^N)^N M(v : <datatype>)^N

to match what we do in B

Ah, good catch, fixed.

nomeata · 2021-11-03T18:09:52Z

spec/Candid.md


 M(ref(r) : principal) = i8(0)
 M(id(v*) : principal) = i8(1) M(v* : vec nat8)
+
+M* : <val>* -> <datatype>* -> i8*
+M*(v^N : <datatype>^N) = leb128(N) M(v : <datatype>)^N


A new type seems to be appropriate. Not breaking existing stuff is part of our value proposition, and given the complexity of this maybe it’s good if a service can say that they really only support plain function references, but not closures?

nomeata · 2021-11-03T18:12:09Z

spec/Candid.md

@@ -1064,7 +1076,7 @@ Most Candid values are self-explanatory, except for references. There are two fo
 Likewise, there are two forms of Candid values for function references:

 * `ref(r)` indicates an opaque reference, understood only by the underlying system.
-* `pub(s,n)`, indicates the public method name `n` of the service referenced by `s`.
+* `pub(s,n,v*:t*)`, indicates the public method name `n` of the service referenced by `s`, possibly followed by a list of type-annotated bound argument values.


You don’t want to support binding arguments to references?

Why not make it a recursive definition that allows you to bind arguments to any function reference (whether it’s opaque, public, or itself a closure – after all, these are all values of the same type, so binding to them should be allowed).

Suggested change

* `pub(s,n,v*:t*)`, indicates the public method name `n` of the service referenced by `s`, possibly followed by a list of type-annotated bound argument values.

* `closure(f,v*:t*)`, indicates the function reference `f`, followed by a list of type-annotated bound argument values.

Ah, yes, that was an oversight. Refactored as you suggest.

rossberg · 2021-11-05T10:06:06Z

@nomeata, fair points about constructing the new call.

I would assume that the serialiser would not merge but merely extend the type table it gets from the closure. Since we allow duplication, it could do so blindly.

For the argument tuple, if it extended the type table from the closure as just said, then I agree that it suffices to just copy over the serialised blob for each value. (We could even add an leb128 with the length for each value's encoding to the TM function I now introduced; however, I'm not sure we want that – for security reasons, the deserialiser should validate the data anyway to the extend it can, and could separate the individual values right there.)

So yes, some work is required, but it doesn't seem too bad?

(With hindsight, I actually think it was an oversight that our function types did not allow closures from the beginning. It seems like a glaring omission for a higher-order data format.)

nomeata

I see, if you put the type table from the closure first, you only have to renumber the type indices you are appending. Still more work than now (where the full type table and thus these indices are pre-computed by the Motoko compiler.)

I still see a problem where you need to pass a closure as an argument to a closure, because now you need to merge and renumber after all. And that's not possible: our future types prevent any kind of operations on type tables or indices.

So bound arguments need their own type table, both in the closure, and in the final call.

So that leads to the nicely simple design where the bound arguments in a closure are simply complete argument sequences with their own type table (i.e. B(args), and we allow the concatenation of encoded argument sequences to represent the encoding of the concatenation (yay, distributivity laws :-)).

Overall, it seems that this feature has very narrow use cases on our platform, because of the prevalent authentication via caller. Canister can really only ever call closures received from fully trusted users. Anything more interesting likely need system-level closures. Maybe worth satisfying the proxy-from-ttusted-user use case without candid support (raw calls), and push for system level closures instead (which then may not even need changes to Candid, since these would be refs)?

nomeata · 2021-11-05T10:06:40Z

spec/Candid.md

@@ -987,6 +1004,12 @@ C[service <actortype> <: service <actortype'>](service <text>) = service <text>
 C[principal <: principal](principal <text>) = principal <text>
 ```

+However, functions can be converted into closures with an empty list of bound arguments:
+```
+C[func <functype> <: closure <functype'>](func <text>.<id>) = clos(func <text>.<id>, .)


This equation should hold for all forms of function values

Suggested change

C[func <functype> <: closure <functype'>](func <text>.<id>) = clos(func <text>.<id>, .)

C[func <functype> <: closure <functype'>](f) = clos(f, .)

nomeata · 2021-11-05T10:07:53Z

spec/Candid.md

@@ -1131,7 +1159,10 @@ T : <reftype> -> i8*
 T(func (<datatype1>*) -> (<datatype2>*) <funcann>*) =
  sleb128(-22) T*(<datatype1>*) T*(<datatype2>*) T*(<funcann>*) // 0x6a
 T(service {<methtype>*}) =
-  sleb128(-23) T*(<methtype>*)                                    // 0x69
+  sleb128(-23) T*(<methtype>*)                                  // 0x69
+T(closure (<datatype1>*) -> (<datatype2>*) <funcann>*) =


A helper function for “prepend length for future type” would help here?

nomeata · 2021-11-05T10:08:37Z

spec/Candid.md

+M(clos(f,v*:t*) : closure <functype>) =
+  leb128(|i8(2) M(f : func <functype>) TM*(v* : t*)|)
+  leb128(|R(f : func <functype>) R*(v* : t*)|)
+  i8(2) M(f : func <functype>) TM*(v* : t*)


Dito, a helper function might clarify this somehow?

nomeata · 2021-11-05T10:09:45Z

spec/Candid.md

-M* : <val>* -> <datatype>* -> i8*
-M*(v^N : <datatype>^N) = leb128(N) M(v : <datatype>)^N
+TM : <val> -> <datatype> -> i8*
+TM(v : <datatype>) = T(<datatype>) M(v : <datatype>)


Why not I, as in other places where we refer to types?

Good point, changed.

rossberg

So bound arguments need their own type table, both in the closure, and in the final call.

So that leads to the nicely simple design where the bound arguments in a closure are simply complete argument sequences with their own type table (i.e. B(args), and we allow the concatenation of encoded argument sequences to represent the encoding of the concatenation (yay, distributivity laws :-)).

Interesting point. But that would require a backwards-incompatible change to the encoding of calls, wouldn't it?

I suppose we could circumvent all this by not allowing currying but only complete binding, i.e., thunks instead of partially applied closures. But that perhaps is a bit too specialised...

rossberg · 2021-11-08T10:27:33Z

spec/Candid.md

@@ -987,6 +1004,12 @@ C[service <actortype> <: service <actortype'>](service <text>) = service <text>
 C[principal <: principal](principal <text>) = principal <text>
 ```

+However, functions can be converted into closures with an empty list of bound arguments:
+```
+C[func <functype> <: closure <functype'>](func <text>.<id>) = clos(func <text>.<id>, .)


rossberg · 2021-11-08T10:27:45Z

spec/Candid.md

-M* : <val>* -> <datatype>* -> i8*
-M*(v^N : <datatype>^N) = leb128(N) M(v : <datatype>)^N
+TM : <val> -> <datatype> -> i8*
+TM(v : <datatype>) = T(<datatype>) M(v : <datatype>)


Good point, changed.

rossberg · 2021-11-08T10:39:49Z

spec/Candid.md

@@ -1131,7 +1159,10 @@ T : <reftype> -> i8*
 T(func (<datatype1>*) -> (<datatype2>*) <funcann>*) =
  sleb128(-22) T*(<datatype1>*) T*(<datatype2>*) T*(<funcann>*) // 0x6a
 T(service {<methtype>*}) =
-  sleb128(-23) T*(<methtype>*)                                    // 0x69
+  sleb128(-23) T*(<methtype>*)                                  // 0x69
+T(closure (<datatype1>*) -> (<datatype2>*) <funcann>*) =


rossberg · 2021-11-08T10:39:54Z

spec/Candid.md

+M(clos(f,v*:t*) : closure <functype>) =
+  leb128(|i8(2) M(f : func <functype>) TM*(v* : t*)|)
+  leb128(|R(f : func <functype>) R*(v* : t*)|)
+  i8(2) M(f : func <functype>) TM*(v* : t*)


spec/Candid.md

nomeata · 2021-11-09T16:55:48Z

But that would require a backwards-incompatible change to the encoding of calls, wouldn't it?

Oh, right, of course. But that means we have an impossibly result: we can't merge type tables (because of future types), but we also can't keep them separate (because it wouldn't be backwards compatible). This seems to prevent currying…

Co-authored-by: Joachim Breitner <mail@joachim-breitner.de>

nomeata · 2021-11-09T20:15:25Z

(For some reason I can't mark conversations as resolved here)

spec/Candid.md

Co-authored-by: Claudio Russo <claudio@dfinity.org>

@Bengo

* update: changes to agent and authentication packages * update: locking repo to node 12 * fix: typescript type safety * greening tests * modifying node version * updating linting * Dont use typescript 'as string' override in idp-protocol/request (dfinity#291) * use lockfileVersion=2 (npm) (dfinity#292) * implementing feedback from @Bengo Co-authored-by: Benjamin Goering <benjamin.goering@dfinity.org>

Add support for bound function arguments

d327618

rossberg requested review from crusso, matthewhammer and chenyan-dfinity November 3, 2021 08:48

rossberg mentioned this pull request Nov 3, 2021

Raw Calls - What is necessary? dfinity/motoko#2703

Closed

chenyan-dfinity reviewed Nov 3, 2021

View reviewed changes

Comments

3aa81a1

nomeata reviewed Nov 3, 2021

View reviewed changes

Comments

c5979f8

nomeata reviewed Nov 5, 2021

View reviewed changes

Comments from Joachim

bf7f1a5

rossberg commented Nov 8, 2021

View reviewed changes

nomeata reviewed Nov 9, 2021

View reviewed changes

spec/Candid.md Outdated Show resolved Hide resolved

Update spec/Candid.md

97ea0e7

Co-authored-by: Joachim Breitner <mail@joachim-breitner.de>

crusso reviewed Nov 20, 2021

View reviewed changes

spec/Candid.md Outdated Show resolved Hide resolved

Update spec/Candid.md

ffef920

Co-authored-by: Claudio Russo <claudio@dfinity.org>

This was referenced Apr 7, 2022

feat: Allow deserialization of candid values with unknown types dfinity/agent-js#555

Merged

Making future types renameable #337

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for bound function arguments #291

Add support for bound function arguments #291

rossberg commented Nov 3, 2021 •

edited

Loading

matthewhammer commented Nov 3, 2021 •

edited

Loading

chenyan-dfinity Nov 3, 2021

rossberg Nov 3, 2021

chenyan-dfinity Nov 3, 2021

matthewhammer Nov 3, 2021

chenyan-dfinity Nov 3, 2021

rossberg Nov 3, 2021

nomeata Nov 3, 2021

rossberg Nov 5, 2021

rossberg commented Nov 3, 2021

nomeata left a comment

nomeata Nov 3, 2021

rossberg Nov 5, 2021

nomeata Nov 3, 2021

nomeata Nov 3, 2021

rossberg Nov 5, 2021

rossberg commented Nov 5, 2021

nomeata left a comment

nomeata Nov 5, 2021

rossberg Nov 8, 2021

nomeata Nov 5, 2021

rossberg Nov 8, 2021

nomeata Nov 5, 2021

rossberg Nov 8, 2021

nomeata Nov 5, 2021

rossberg Nov 8, 2021

rossberg left a comment

rossberg Nov 8, 2021

rossberg Nov 8, 2021

rossberg Nov 8, 2021

rossberg Nov 8, 2021

nomeata commented Nov 9, 2021

nomeata commented Nov 9, 2021

	forward : (call : () -> ()) -> ();
	forward : (call : func () -> ()) -> ();

	* `pub(s,n,v:t)`, indicates the public method name `n` of the service referenced by `s`, possibly followed by a list of type-annotated bound argument values.
	* `closure(f,v:t)`, indicates the function reference `f`, followed by a list of type-annotated bound argument values.

	C[func <functype> <: closure <functype'>](func <text>.<id>) = clos(func <text>.<id>, .)
	C[func <functype> <: closure <functype'>](f) = clos(f, .)

Add support for bound function arguments #291

Are you sure you want to change the base?

Add support for bound function arguments #291

Conversation

rossberg commented Nov 3, 2021 • edited Loading

matthewhammer commented Nov 3, 2021 • edited Loading

1. Use Blob

2. Use dynamic (see #292)

3. Use dynamics (see comments to #292)

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rossberg commented Nov 3, 2021

nomeata left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rossberg commented Nov 5, 2021

nomeata left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rossberg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nomeata commented Nov 9, 2021

nomeata commented Nov 9, 2021

rossberg commented Nov 3, 2021 •

edited

Loading

matthewhammer commented Nov 3, 2021 •

edited

Loading

1. Use `Blob`

2. Use `dynamic` (see #292)

3. Use `dynamics` (see comments to #292)