Add common_test for batch_size disagreement and fix some bugs

src/hbbft.erl

@@ -147,6 +147,7 @@ start_on_demand(Data = #hbbft_data{buf=Buf, j=J, n=N, secret_key=SK, batch_size=
                                   stamps=Stamps, decrypted=Decrypted}) ->
     %% pick proposed whichever is lesser from batchsize/n or buffer
     Proposed = hbbft_utils:random_n(min((BatchSize div N), length(Buf)), lists:sublist(Buf, BatchSize)),
+    ct:pal("proposed batch of length ~p", [length(Proposed)]),
     %% encrypt x -> tpke.enc(pk, proposed)
     Stamp = case Data#hbbft_data.stampfun of
                 undefined -> <<>>;
@@ -377,9 +378,13 @@ handle_msg(Data = #hbbft_data{round=R}, J, {dec, R, I, Share}) ->
                                                                     failed_decrypt=[I|Data#hbbft_data.failed_decrypt]});
                                 Decrypted ->
                                     case decode_list(Decrypted, []) of
-                                        [_ | Transactions] when length(Transactions) > Data#hbbft_data.batch_size ->
-                                            ignore;
+                                        [_ | Transactions] when length(Transactions) > Data#hbbft_data.batch_size div Data#hbbft_data.n ->
+                                            %% because this violated the agreed on batch size, reject this proposal
+                                            ct:pal("~p dropped batch of length ~p ~p", [Data#hbbft_data.j, length(Transactions), Data#hbbft_data.batch_size]),
+                                            NewDecrypted = maps:put(I, [], Data#hbbft_data.decrypted),
+                                            check_completion(Data#hbbft_data{dec_shares=NewShares, decrypted=NewDecrypted});
                                         [Stamp | Transactions] ->
+                                            ct:pal("~p got batch of length ~p ~p", [Data#hbbft_data.j, length(Transactions), Data#hbbft_data.batch_size]),
                                             NewDecrypted = maps:put(I, Transactions, Data#hbbft_data.decrypted),
                                             Stamps = [{I, Stamp} | Data#hbbft_data.stamps],
                                             check_completion(Data#hbbft_data{dec_shares=NewShares,
@@ -443,6 +448,7 @@ maybe_start_acs(Data = #hbbft_data{n=N, j=J, secret_key=SK, batch_size=BatchSize
             %% get the top b elements from buf
             %% pick a random B/N selection of them
             Proposed = hbbft_utils:random_n(BatchSize div N, lists:sublist(Data#hbbft_data.buf, length(Data#hbbft_data.buf) - BatchSize + 1, BatchSize)),
+            ct:pal("proposed batch of length ~p", [length(Proposed)]),
             %% encrypt x -> tpke.enc(pk, proposed)
             Stamp = case Data#hbbft_data.stampfun of
                 undefined -> <<>>;

test/hbbft_SUITE.erl

@@ -16,6 +16,7 @@
          start_on_demand_test/1,
          one_actor_wrong_key_test/1,
          one_actor_corrupted_key_test/1,
+         one_actor_wrong_batch_size_test/1,
          initial_fakecast_test/1
         ]).
 
@@ -31,6 +32,7 @@ all() ->
      start_on_demand_test,
      one_actor_wrong_key_test,
      one_actor_corrupted_key_test,
+     one_actor_wrong_batch_size_test,
      initial_fakecast_test
     ].
 
@@ -358,6 +360,7 @@ one_actor_corrupted_key_test(Config) ->
     PrivateKeys = [PK2|PrivateKeys0],
 
     Workers = [ element(2, hbbft_worker:start_link(N, F, I, tpke_privkey:serialize(SK), BatchSize, false)) || {I, SK} <- enumerate(PrivateKeys) ],
+
     Msgs = [ crypto:strong_rand_bytes(128) || _ <- lists:seq(1, N*20)],
     %% feed the badgers some msgs
     lists:foreach(fun(Msg) ->
@@ -385,6 +388,49 @@ one_actor_corrupted_key_test(Config) ->
     ct:log("chain contains ~p distinct transactions~n", [length(BlockTxns)]),
     ok.
 
+one_actor_wrong_batch_size_test(Config) ->
+    N = proplists:get_value(n, Config),
+    F = proplists:get_value(f, Config),
+    BatchSize = proplists:get_value(batchsize, Config),
+    PubKey = proplists:get_value(pubkey, Config),
+    PrivateKeys = proplists:get_value(privatekeys, Config),
+    %% boot one worker with the wrong batch size, double the limit the others have
+    Workers = [element(2, hbbft_worker:start_link(N, F, 0, tpke_privkey:serialize(hd(PrivateKeys)), BatchSize*2, false)) ] ++
+    [ element(2, hbbft_worker:start_link(N, F, I+1, tpke_privkey:serialize(SK), BatchSize, false)) || {I, SK} <- enumerate(tl(PrivateKeys)) ],
+
+    Msgs = [ crypto:strong_rand_bytes(128) || _ <- lists:seq(1, N*20)],
+    %% send the bad node some messages we expect should not appear on chain
+    _Worker1Msgs = [ begin Msg = crypto:strong_rand_bytes(128),
+                          hbbft_worker:submit_transaction(Msg, hd(Workers)),
+                          Msg
+                    end|| _ <- lists:seq(1, 60)],
+
+    %% feed the badgers some msgs
+    lists:foreach(fun(Msg) ->
+                          Destinations = random_n(rand:uniform(N), tl(Workers)),
+                          ct:log("destinations ~p~n", [Destinations]),
+                          [ok = hbbft_worker:submit_transaction(Msg, D) || D <- Destinations]
+                  end, Msgs),
+
+    %% wait for all the worker's mailboxes to settle and
+    %% wait for the chains to converge
+    ok = wait_for_chains(Workers, 2),
+
+    Chains = get_common_chain(tl(Workers)),
+
+    1 = sets:size(Chains),
+    [Chain] = sets:to_list(Chains),
+    ct:log("chain is of height ~p~n", [length(Chain)]),
+    %% verify they are cryptographically linked
+    true = hbbft_worker:verify_chain(Chain, PubKey),
+    %% check all the transactions are unique
+    BlockTxns = lists:flatten([ hbbft_worker:block_transactions(B) || B <- Chain ]),
+    true = length(BlockTxns) == sets:size(sets:from_list(BlockTxns)),
+    %% check they're all members of the original message list
+    true = sets:is_disjoint(sets:from_list(BlockTxns), sets:from_list(_Worker1Msgs)),
+    true = sets:is_subset(sets:from_list(BlockTxns), sets:from_list(Msgs)),
+    ct:log("chain contains ~p distinct transactions~n", [length(BlockTxns)]),
+    ok.
 
 -record(state,
         {