Merge pull request #2699 from aeternity/fate-store-map-update-order

Make sure store maps are copied before updated.

apps/aecontract/test/aecontract_SUITE.erl

@@ -122,6 +122,7 @@
         , sophia_big_map_benchmark/1
         , sophia_pmaps/1
         , sophia_map_of_maps/1
+        , sophia_maps_gc/1
         , sophia_polymorphic_entrypoint/1
         , sophia_arity_check/1
         , sophia_chess/1
@@ -346,6 +347,7 @@ groups() ->
                                  sophia_map_benchmark,
                                  sophia_big_map_benchmark,
                                  sophia_map_of_maps,
+                                 sophia_maps_gc,
                                  sophia_variant_types,
                                  sophia_arity_check,
                                  sophia_chain,
@@ -4091,6 +4093,42 @@ sophia_map_of_maps(_Cfg) ->
 
     ok.
 
+sophia_maps_gc(_Cfg) ->
+    ?skipRest(vm_version() =< ?VM_AEVM_SOPHIA_3, only_lima),
+    state(aect_test_utils:new_state()),
+    Acc = ?call(new_account, 10000000 * aec_test_utils:min_gas_price()),
+    %% Big to make sure it ends up in the store.
+    InitA = maps:from_list([ {integer_to_binary(I), integer_to_binary(I + 100)}
+                             || I <- lists:seq(1, 100) ]),
+    InitB = #{},
+
+    Prune = fun(M) -> maps:without(maps:keys(InitA), M) end,
+
+    Ct  = ?call(create_contract, Acc, maps_gc, {InitA, InitB}, #{fee => 2000000 * aec_test_utils:min_gas_price()}),
+
+    StateT = {tuple, [{map, string, string}, {map, string, string}]},
+
+    KeyA1 = <<"KeyA1">>,
+    KeyA2 = <<"KeyA2">>,
+    ValA  = <<"ValA">>,
+    {} = ?call(call_contract, Acc, Ct, upd_a, {tuple, []}, {KeyA1, KeyA2, ValA}),
+
+    A1 = InitA#{ KeyA1 => ValA },
+    B1 = InitA#{ KeyA2 => ValA },
+    {ResA1, ResB1} = ?call(call_contract, Acc, Ct, get_state, StateT, {}),
+    ?assertEqual({Prune(A1), Prune(B1)}, {Prune(ResA1), Prune(ResB1)}),
+
+    KeyB1 = <<"KeyB1">>,
+    KeyB2 = <<"KeyB2">>,
+    ValB  = <<"ValB">>,
+    {} = ?call(call_contract, Acc, Ct, upd_b, {tuple, []}, {KeyB1, KeyB2, ValB}),
+
+    A2 = B1#{ KeyB1 => ValB },
+    B2 = B1#{ KeyB2 => ValB },
+    {ResA2, ResB2} = ?call(call_contract, Acc, Ct, get_state, StateT, {}),
+    ?assertEqual({Prune(A2), Prune(B2)}, {Prune(ResA2), Prune(ResB2)}),
+    ok.
+
 sophia_polymorphic_entrypoint(_Cfg) ->
     state(aect_test_utils:new_state()),
     Acc = ?call(new_account, 10000000 * aec_test_utils:min_gas_price()),

apps/aefate/src/aefa_stores.erl

@@ -343,7 +343,15 @@ compute_store_updates(Metadata, #cache_entry{terms = TermCache, store = Store})
               [ CopyOrInplace(MapId, Map) || {MapId, Map} <- maps:to_list(Maps) ] ++
               [ {gc_map, RawId}           || RawId <- Garbage ],
 
-    {Metadata2, Updates}.
+    %% It's important (very!) that copy_map runs before update_map, since
+    %% update map does a destructive update of the store. To make sure this is
+    %% the case we sort the updates.
+    Order = fun(push_term)  -> 0;
+               (copy_map)   -> 1;
+               (update_map) -> 2;
+               (gc_map)     -> 3 end,
+    Compare = fun(A, B) -> Order(element(1, A)) =< Order(element(1, B)) end,
+    {Metadata2, lists:sort(Compare, Updates)}.
 
 perform_store_updates(Updates, Meta, Store) ->
     {Meta1, Store1} = lists:foldl(fun perform_store_update/2, {Meta, Store}, Updates),

test/contracts/maps_gc.aes

@@ -0,0 +1,20 @@
+
+// Testing garbage collection and inplace update of maps
+contract MapsGC =
+
+  record state = { a : map(string, string)
+                 , b : map(string, string) }
+
+  entrypoint init(a, b) = {a = a, b = b}
+
+  stateful entrypoint upd_a(k1, k2, v) =
+    let a = state.a{[k1] = v}
+    let b = state.a{[k2] = v}
+    put(state{a = a, b = b})
+
+  stateful entrypoint upd_b(k1, k2, v) =
+    let a = state.b{[k1] = v}
+    let b = state.b{[k2] = v}
+    put(state{a = a, b = b})
+
+  entrypoint get_state() = (state.a, state.b)