Avoid races in aehttp_stake_contract_SUITE (#4300)

* Avoid races in aehttp_stake_contract_SUITE * aehttp_sc_SUITE: Avoid race in reestablish check
aeternity · Mar 13, 2024 · 5e3e499 · 5e3e499
1 parent d4017ef
commit 5e3e499
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 19 deletions.
diff --git a/apps/aecore/src/aec_consensus_common_tests.erl b/apps/aecore/src/aec_consensus_common_tests.erl
@@ -114,14 +114,22 @@ client_request({mine_blocks, NumBlocksToMine, Type}) ->
             {ok, [client_request(emit_mb) || _ <- lists:seq(1, NumBlocksToMine)]}
     end;
 client_request(mine_micro_block_emptying_mempool_or_fail) ->
-    MaybeKB = ensure_leader(),
-    MB = client_request(emit_mb),
-    %% If instant mining is enabled then we can't have microforks :)
-    {ok, []} = aec_tx_pool:peek(infinity),
-    {ok, MaybeKB ++ [MB]};
+    mine_micro_block_emptying_mempool_or_fail(1, []);
 client_request({mine_blocks_until_txs_on_chain, TxHashes, Max}) ->
     mine_blocks_until_txs_on_chain(TxHashes, Max, []).
 
+mine_micro_block_emptying_mempool_or_fail(N, PrevBlocks) ->
+    case aec_tx_pool:peek(infinity) of
+        {ok, []} ->
+            {ok, PrevBlocks};
+        _ when N == 0 ->
+            {error, mempool_not_empty};
+        _ ->
+            MaybeKB = ensure_leader(),
+            MB = client_request(emit_mb),
+            mine_micro_block_emptying_mempool_or_fail(N - 1, PrevBlocks ++ MaybeKB ++ [MB])
+    end.
+
 mine_blocks_until_txs_on_chain(_TxHashes, 0, _Blocks) ->
     {error, max_reached};
 mine_blocks_until_txs_on_chain(TxHashes, Max, Blocks) ->

diff --git a/apps/aecore/test/aecore_suite_utils.erl b/apps/aecore/test/aecore_suite_utils.erl
@@ -52,7 +52,7 @@
          wait_for_height/2,
          wait_for_height/3,
          flush_new_blocks/0,
-         flush_mempool/1,
+         flush_mempool/2,
          spend/5,         %% (Node, FromPub, ToPub, Amount, Fee) -> ok
          sign_on_node/2,
          sign_on_node/3,
@@ -908,11 +908,20 @@ flush_new_blocks_produced() ->
             flush_new_blocks_produced()
     end.
 
-flush_mempool(Node) ->
-    {ok, SignedTxs} = rpc:call(Node, aec_tx_pool, peek, [infinity]),
-    [ rpc:call(Node, aec_tx_pool, delete, [aetx_sign:hash(STx)]) || STx <- SignedTxs ],
-    ct:log("Flushed ~p txs from ~p's mempool", [length(SignedTxs), Node]),
-    ok.
+%% Try to flush at least N messages from mempool
+%% (the N is to avoid races - so it just waits "a little bit").
+flush_mempool(N, Node) ->
+    flush_mempool(2, N, Node).
+
+flush_mempool(Retries, N, Node) ->
+    case rpc:call(Node, aec_tx_pool, peek, [infinity]) of
+        {ok, SignedTxs} when length(SignedTxs) >= N orelse Retries == 0 ->
+            [ rpc:call(Node, aec_tx_pool, delete, [aetx_sign:hash(STx)]) || STx <- SignedTxs ],
+            ct:log("Flushed ~p txs from ~p's mempool", [length(SignedTxs), Node]);
+        _ ->
+            timer:sleep(500),
+            flush_mempool(Retries - 1, N, Node)
+    end.
 
 %% block the process until a certain height is reached
 %% this has the expectation that the Node is mining

diff --git a/apps/aehttp/test/aehttp_sc_SUITE.erl b/apps/aehttp/test/aehttp_sc_SUITE.erl
@@ -1681,7 +1681,10 @@ sc_ws_reestablish_(ReestablishOptions, Config) ->
                                 {RCP, FsmId}
                           end,
     ReestablishOptions1 = maps:put(host, <<"localhost">>, ReestablishOptions),
-    {ok, IrConnPid, IFsmId} = channel_ws_start(initiator, ReestablishOptions1, Config),
+    %% We need to register for events - before starting the channel (asynch!)
+    ok = ?WS:register_test_for_channel_events(RrConnPid, [info, update]),
+    {ok, IrConnPid, IFsmId} = channel_ws_start(initiator, ReestablishOptions1, Config, [info, update]),
+
     Config1 = lists:keystore(channel_clients, 1, Config,
                             {channel_clients, #{ initiator => IrConnPid
                                                , initiator_fsm_id => IFsmId
@@ -1694,8 +1697,6 @@ await_reestablish_reports(Config) ->
     ResponderLeaves = proplists:get_value(responder_leaves, Config, true),
     #{initiator := IConnPid, responder := RConnPid}
         = proplists:get_value(channel_clients, Config),
-    ok = ?WS:register_test_for_channel_events(RConnPid, [info, update]),
-    ok = ?WS:register_test_for_channel_events(IConnPid, [info, update]),
     {ok, ChId, IMsg} = wait_for_channel_event(IConnPid, info, Config),
     {ok, ChId, RMsg} = wait_for_channel_event(RConnPid, info, Config),
     {ChId, true} = {ChId, ChId =/= null},
@@ -3440,10 +3441,17 @@ reconnect_client_(ReestablishOpts, Role, Config) ->
                            responder ->
                                ReestablishOpts
                        end,
-    {ok, ConnPid, FsmId} = channel_ws_start(Role, ReestablishOpts1, Config),
+    OldClients = ?config(channel_clients, Config),
+    OtherConnPid = case Role of
+                       initiator -> maps:get(responder, OldClients);
+                       responder -> maps:get(initiator, OldClients)
+                   end,
+    %% We need to register for events - before starting the channel (asynch!)
+    ok = ?WS:register_test_for_channel_events(OtherConnPid, [info, update]),
+    {ok, ConnPid, FsmId} = channel_ws_start(Role, ReestablishOpts1, Config, [info, update]),
+
     ct:log("New ConnPid = ~p", [ConnPid]),
     ct:log("Check if reestablish resulted in a reconnect", []),
-    OldClients = ?config(channel_clients, Config),
     ct:log("OldClients = ~p", [OldClients]),
     {OldFsmId, NewClients} = case Role of
                                  initiator ->

diff --git a/apps/aehttp/test/aehttp_stake_contract_SUITE.erl b/apps/aehttp/test/aehttp_stake_contract_SUITE.erl
@@ -484,7 +484,7 @@ set_up_lazy_leader_node(NetworkId, Config) ->
     ReceiveAddress = encoded_pubkey(?FORD),
     NodeConfig = node_config(NetworkId,?LAZY_NODE, Config, [{?LISA, ?LISA}], ReceiveAddress, ?CONSENSUS_HC,
                                 false, GenesisStartTime),
-    build_json_files(ElectionContract, [NodeConfig]),    
+    build_json_files(ElectionContract, [NodeConfig]),
     aecore_suite_utils:create_config(?LAZY_NODE, Config,
                                      NodeConfig,
                                     [{add_peers, true} ]),
@@ -764,7 +764,7 @@ empty_parent_block(Config) ->
 empty_parent_block_(_Config) ->
     TopHeight = rpc(?LAZY_NODE, aec_chain, top_height, []),
     %% Remove the posted commitments to create a parent generation without commitments
-    aecore_suite_utils:flush_mempool(?PARENT_CHAIN_NODE1_NAME),
+    aecore_suite_utils:flush_mempool(2, ?PARENT_CHAIN_NODE1_NAME),
     ok = produce_blocks_hc(?LAZY_NODE, ?LAZY_NODE_NAME, 1, lazy_leader),
 
     %% The lazy leader block is either from ?LISA (dev8) or ?ALICE/?BOB (dev1)
@@ -1497,7 +1497,7 @@ build_json_files(ElectionContract, NodeConfigs) ->
 		 Call7, Call8, Call9, Call10, Call11, Call12, Call13],
     ProtocolBin = integer_to_binary(aect_test_utils:latest_protocol_version()),
     ContractsFileNames = [ContractsFileName  || #{<<"chain">> := #{<<"hard_forks">> := #{ProtocolBin := #{<<"contracts_file">> := ContractsFileName}}}} <- NodeConfigs],
-    AccountsFileNames = [AccountsFileName  || #{<<"chain">> := #{<<"hard_forks">> := #{ProtocolBin := #{<<"accounts_file">> := AccountsFileName}}}} <- NodeConfigs],    
+    AccountsFileNames = [AccountsFileName  || #{<<"chain">> := #{<<"hard_forks">> := #{ProtocolBin := #{<<"accounts_file">> := AccountsFileName}}}} <- NodeConfigs],
     aecore_suite_utils:create_seed_file(ContractsFileNames,
         #{<<"contracts">> => [C0, SC, EC], <<"calls">> => AllCalls}),
     aecore_suite_utils:create_seed_file(AccountsFileNames,