Zilliqa / scilla

Scilla - A Smart Contract Intermediate Level Language
https://scilla-lang.org
GNU General Public License v3.0
240 stars 78 forks source link

Support in-place Map updates #285

Closed jjcnn closed 6 years ago

jjcnn commented 6 years ago

Since OCaml hashtables are mutable, Scilla maps have become mutable since we changed their implementation to use hashtables.

This change comes with a number of problems, chief among them being that this was never the intended behaviour. Also, should one choose to share a map between two fields, that sharing is lost when the transition is over, and the state of the contract is output to a json file.

From a slack conversation between Ilya and myself:

lya Sergey 41 minutes ago This all sounds pretty bad overall, as the semantics of Scilla maps (which are considered immutable) is now compromised in many ways by their “efficient” implementation. And it’s not only about sharing between fields: imagine a map coming from a message, and then mutated in several ways, under assumptions that each time we have a new copy. I see no good way of resolving it but by re-implementing Scilla maps as immutable OCaml maps (I hope they are already there). Ilya Sergey 36 minutes ago [...] When you’re back here could you please create an issue for Vaivas (referencing this conversation) and ask him to rework Scilla map literals from OCaml Hashtables to OCaml maps: https://v1.realworldocaml.org/v1/en/html/maps-and-hash-tables.html

vaivaswatha commented 6 years ago

@jjcnn This is not true. Scilla maps are immutable because the implementation of builtins for Scilla Maps ensure that when they are modified, a copy is made. See the comment(s) (* Scilla semantics is not in-place modification. *) in Builtins.ml. So if you are seeing a behaviour where it is modified in place, that's a bug and can be fixed by creating a copy as in these places.

share a map between two fields

What is sharing a map between two fields? Can you post an example where you're seeing unintended behaviour?

jjcnn commented 6 years ago

@vaivaswatha is right. Maps are in fact immutable at the moment because a put operation makes a copy of the hashtable.

However, this means that the put operation is linear in the size of the map, and the only way to get around that is to make bindings with maps (let and bind) be linear instead.

vaivaswatha commented 6 years ago

Based on offline discussion, replace Caml.Hashtbl implementation with Caml.Map for Scilla `Map.

vaivaswatha commented 6 years ago

Based on offline discussion, here is what should be done:

  1. Continue using OCaml's Hashtbl to represent Scilla Maps.
  2. Existing semantics of Maps in expressions will continue, i.e., as immutable values.
  3. Provide new syntaxes for accessing Map values directly and updating them: v <- m[k] for reading a field directly and m[k] := v for updating a field.
  4. The syntax update will support nested accesses too, so something like m[k1][k2] := v will be valid.
vaivaswatha commented 6 years ago

Implemented with #290. Closing.