Multi_asset safety check in spec, LIGO

[tomjack]

Hello!

The multi asset spec mentions "safety check" for token receivers in a couple places, and also uses the language "MUST implement" -- in particular regarding the multi_token_receiver interface.

What is the meaning of this, exactly? It is not very clear.

The multi_token_receiver interface is defined like this as a LIGO variant:

type multi_token_receiver =
  | On_multi_tokens_received of on_multi_tokens_received_param

The variant has only one case, so in LIGO, the type multi_token_receiver is compiled the same way as on_multi_tokens_received_param -- as a tree of pair types.

(I think, at the moment, you should not write Michelson interface specifications using LIGO types!)

In the draft LIGO implementation, we have code like this:

  let receiver : multi_token_receiver contract = 
    Operation.get_contract param.to_ in

(Recall to_ : address in the transfer parameter.)

In the compiled Michelson, this shows up as:

CONTRACT (pair (pair (list (pair nat nat)) bytes) (pair (option address) address))
IF_NONE { PUSH string "bad address for get_contract" ; FAILWITH } {} ;

This seems to have some interesting consequences.

Most importantly, I believe this means that passing a to_ address with an entry point (intending to transfer to a receiver who implements %on_multi_tokens_received as well as other entrypoints) will result in locked tokens. Token receivers will require proxy contracts,

It is possible I am confused? It is late.

┆Created By: tqbot

[tomjack]

I believe that one possible solution will be to implement support for CONTRACT %entry p in LIGO. Then this could be used for the safety check.

In addition to ensuring that the destination really has an entrypoint labelled "on_multi_tokensreceived" (which provides some comfort, even if it is not very specific,) this will mean that the contract only accepts bare addresses for `to`, not addresses with entrypoints.

Such a bare address will compare equal to SENDER later.

[emishur]

The multi asset spec mentions "safety check" for token receivers in a couple places, and also uses the language "MUST implement" -- in particular regarding the multi_token_receiver interface.

What is the meaning of this, exactly? It is not very clear.

It means that if destination contract does not implement this interface, the transaction will fail.

[emishur]

The multi_token_receiver interface is defined like this as a LIGO variant:

type multi_token_receiver = | On_multi_tokens_received of on_multi_tokens_received_param The variant has only one case

It is an artifact from previous iteration when this interface had more than one entry point. It can be changed to a simple "flat" type On_multi_tokens_received

[emishur]

In the draft LIGO implementation, we have code like this:

let receiver : multi_token_receiver contract = Operation.getcontract param.to in (Recall to_ : address in the transfer parameter.)

In the compiled Michelson, this shows up as:

CONTRACT (pair (pair (list (pair nat nat)) bytes) (pair (option address) address)) IF_NONE { PUSH string "bad address for get_contract" ; FAILWITH } {} ;

It would work as intended (given that LIGO get_contract does not have support for multiple entry points and the receiver contract needs to implement only multi_token_receiver interface and nothing else). If the receiver contract does not implement multi_token_receiver, the transaction will fail.

Most importantly, I believe this means that passing a to_ address with an entry point (intending to transfer to a receiver who implements %on_multi_tokens_received as well as other entrypoints) will result in locked tokens.

If the receiver does not implement %on_multi_tokens_received, the transaction will fail and no tokens will be transferred. Yes, this is the problem (lack of multi-entry points support in get_contract in LIGO) at the moment: receiver can only implement only %on_multi_tokens_received and nothing else.

[emishur]

(I think, at the moment, you should not write Michelson interface specifications using LIGO types!)

And what other options do I have? Even if I specify entry points in Michelson, how would I convert it to LIGO to write actual contract (see https://gitlab.com/ligolang/ligo/issues/41)?

[tomjack]

I couldn't tell from your response whether you understood the issue or not.

I claim the safety check is broken in the current draft -- in the case that an address with an entry point is passed as to_, tokens can be locked forever.

Even if I specify entry points in Michelson, how would I convert it to LIGO to write actual contract

I think that is a very important question, and we have no answer so far... IMO LIGO is currently suitable only for experimentation (not just for this reason.) I recognize that this position is not tenable for you, if you have been tasked to do something in LIGO which is not just an experiment.

[tomjack]

It means that if destination contract does not implement this interface, the transaction will fail.

What does this mean? It is not clear... What does "implement an interface" mean? There is no such concept in Michelson...

[emishur]

Implement the interface means implement entry points specified by the interface.

I claim the safety check is broken in the current draft -- in the case that an address with an entry point is passed as to_, tokens can be locked forever.

I do not understand what do you mean when saying that tokens get locked. Could you explain this scenario in more details?

Let's say we intend to transfer some tokens from address A to address B. If contract execution fails for any reason, the state of the multi_asset contract is not changed and tokens remain assigned to address A. Even if safety check on address B fails or succeeds, but the whole transaction fails, it will discard all the changes made to contracts A and B.

[tomjack]

I think the problem should happen when receiver (say KT1receiver) implements multiple entrypoints (including %on_multi_tokensreceived and some other entrypoints), and when someone transfers tokens to = KT1receiver%on_multi_tokens_received.

Then the tokens will be locked. They will be owned by KT1receiver%on_multi_tokens_received forever, it will be impossible for KT1receiver to transfer them to another address.

[tomjack]

You could think of this as a designed limitation of the "safety check". You could say that users should not do what I described, that they are shooting themselves in the foot.

Alternatively, you could think that it is a bug in the specification. The safety check should prevent such shooting in the foot, but does not.

Alternatively, you could think of it as a bug in the implementation. The spec is so vague that it is not clear to me whether the draft implementation meets the spec.

[tomjack]

After thinking about this a little bit more, I am not sure how the current draft behavior could possibly be useful.

Suppose our token receiver implements only an %on_multi_tokens_received entrypoint. It receives tokens. Now we want to send those tokens somewhere else. How will we do that? We will somehow cause the receiver to send a transfer to the token contract, even though it only has a %on_multi_tokens_received entrypoint?

[tomjack]

Of course, I immediately realized one answer: the receiver must immediately send an Add_operator message to the token contract (at least once) upon receiving %on_multi_tokens_received. This way a control contract or implicit account address can be added as operator, and they can send future transfers to the token contract on behalf of the receiver contract.

(Was this anticipated but omitted from the design doc?)

[emishur]

I can see now what's the functionality gap. The receiver contract needs either add its manager as an operator or have transfer_from entry point, which will call multi_token transfer. I am going to modify receiver interface and multi_token implementation to guarantee that at least one operator (receiver manager) is added.

[emishur]

After deliberation, I decided not to change receiver interface, but update the spec only. There are multiple scenarios and/or use cases how token management behind a receiver contract can be implemented (from simple automatic forwarding of received tokens to an elaborate operator management). We do not want to lock down on only on one of the possible scenarios, since this contract spec focuses on transfer logic and leaves other essential aspects out of scope to be customized for particular business cases.

[tomjack]

OK.

I don't understand how any other scenario is possible -- even if you forward the tokens, you must forward them to a contract which will immediately (or has already) done Add_operator.

I believe there is still the original issue that the safety check will allow tokens to be locked forever if an address with an entry point is given as the destination/to_.

Hopefully people who would do that will test it before trying it with tokens which matter, though. :)

Both of these issues (and at least one other I didn't mention, which is less important) would be solved, I think, by using CONTRACT %on_multi_tokens_received ... -- on one hand, a receiver contract could have other entrypoints, on the other hand, the 'safety check' would better fulfill its intended purpose.

[emishur]

@tomjack There is a PR (https://github.com/tqtezos/smart-contracts/pull/18) to explain what are possible scenarios are. But it looks you found it already 🙂

[tomjack]

This is largely fixed now (in the implementation at least -- spec is still not very clear) with the use of CONTRACT %on_multi_tokens_received ....