Merge 3391a5d into 44109bc

helium · Jan 16, 2019 · cf154bf · cf154bf
2 parents 44109bc + 3391a5d
commit cf154bf
Show file tree

Hide file tree

Showing 2 changed files with 103 additions and 22 deletions.
diff --git a/src/hbbft.erl b/src/hbbft.erl
@@ -218,34 +218,31 @@ handle_msg(Data = #hbbft_data{round=R}, J, {dec, R, I, Share}) ->
         true ->
             {I, Enc} = lists:keyfind(I, 1, Data#hbbft_data.acs_results),
             EncKey = get_encrypted_key(Data#hbbft_data.secret_key, Enc),
-            %% TODO verify the shares with verify_share/3
-            case tpke_pubkey:combine_shares(tpke_privkey:public_key(Data#hbbft_data.secret_key), EncKey, SharesForThisBundle) of
+            case combine_shares(Data#hbbft_data.f, Data#hbbft_data.secret_key, SharesForThisBundle, EncKey) of
                 undefined ->
-                    %% can't recover the key
-                    {Data#hbbft_data{dec_shares=NewShares}, ok};
+                    %% can't recover the key, consider this ACS failed if we have 2f+1 shares and still can't recover the key
+                    case length(SharesForThisBundle) > 2 * Data#hbbft_data.f of
+                        true ->
+                            %% ok, just declare this ACS returned an empty list
+                            NewDecrypted = maps:put(I, [], Data#hbbft_data.decrypted),
+                            check_completion(Data#hbbft_data{dec_shares=NewShares, decrypted=NewDecrypted});
+                        false ->
+                            {Data#hbbft_data{dec_shares=NewShares}, ok}
+                    end;
                 DecKey ->
                     case decrypt(DecKey, Enc) of
                         error ->
-                            {Data#hbbft_data{dec_shares=NewShares}, ok};
+                            %% can't decrypt, consider this ACS a failure
+                            %% just declare this ACS returned an empty list because we had
+                            %% f+1 valid shares but the resulting decryption key was unusuable to decrypt
+                            %% the transaction bundle
+                            NewDecrypted = maps:put(I, [], Data#hbbft_data.decrypted),
+                            check_completion(Data#hbbft_data{dec_shares=NewShares, decrypted=NewDecrypted});
                         Decrypted ->
                             {Stamp, Transactions} = binary_to_term(Decrypted),
                             NewDecrypted = maps:put(I, Transactions, Data#hbbft_data.decrypted),
                             Stamps = [{I, Stamp} | Data#hbbft_data.stamps],
-                            case maps:size(NewDecrypted) == length(Data#hbbft_data.acs_results) andalso not Data#hbbft_data.sent_txns of
-                                true ->
-                                    %% we did it!
-                                    %% Combine all unique messages into a single list
-                                    TransactionsThisRound = lists:usort(lists:flatten(maps:values(NewDecrypted))),
-                                    StampsThisRound = lists:usort(Stamps),
-                                    %% return the transactions we agreed on to the user
-                                    %% we have no idea which transactions are valid, invalid, out of order or missing
-                                    %% causal context (eg. a nonce is not monotonic) so we return them to the user to let them
-                                    %% figure it out. We expect the user to call finalize_round/3 once they've decided what they want to accept
-                                    %% from this set of transactions.
-                                    {Data#hbbft_data{dec_shares=NewShares, decrypted=NewDecrypted, stamps=Stamps, sent_txns=true}, {result, {transactions, StampsThisRound, TransactionsThisRound}}};
-                                false ->
-                                    {Data#hbbft_data{dec_shares=NewShares, decrypted=NewDecrypted, stamps=Stamps}, ok}
-                            end
+                            check_completion(Data#hbbft_data{dec_shares=NewShares, decrypted=NewDecrypted, stamps=Stamps})
                     end
             end;
         false ->
@@ -461,3 +458,33 @@ group_by([], D) ->
     lists:keysort(1, [{K, lists:sort(V)} || {K, V} <- dict:to_list(D)]);
 group_by([{K, V}|T], D) ->
     group_by(T, dict:append(K, V, D)).
+
+check_completion(Data) ->
+    case maps:size(Data#hbbft_data.decrypted) == length(Data#hbbft_data.acs_results) andalso not Data#hbbft_data.sent_txns of
+        true ->
+            %% we did it!
+            %% Combine all unique messages into a single list
+            TransactionsThisRound = lists:usort(lists:flatten(maps:values(Data#hbbft_data.decrypted))),
+            StampsThisRound = lists:usort(Data#hbbft_data.stamps),
+            %% return the transactions we agreed on to the user
+            %% we have no idea which transactions are valid, invalid, out of order or missing
+            %% causal context (eg. a nonce is not monotonic) so we return them to the user to let them
+            %% figure it out. We expect the user to call finalize_round/3 once they've decided what they want to accept
+            %% from this set of transactions.
+            {Data#hbbft_data{sent_txns=true}, {result, {transactions, StampsThisRound, TransactionsThisRound}}};
+        false ->
+            {Data, ok}
+    end.
+
+-spec combine_shares(pos_integer(), tpke_privkey:privkey(), [tpke_privkey:share()], tpke_pubkey:ciphertext()) -> undefined | binary().
+combine_shares(F, SK, SharesForThisBundle, EncKey) ->
+    %% filter the shares with verify_share/3
+    %% only use valid shares so an invalid share doesn't corrupt our result
+    ValidSharesForThisBundle = [ S || S <- SharesForThisBundle, tpke_pubkey:verify_share(tpke_privkey:public_key(SK), S, EncKey) ],
+    case length(ValidSharesForThisBundle) > F of
+        true ->
+            tpke_pubkey:combine_shares(tpke_privkey:public_key(SK), EncKey, ValidSharesForThisBundle);
+        false ->
+            %% not enough valid shares to bother trying to combine them
+            undefined
+    end.
diff --git a/test/hbbft_SUITE.erl b/test/hbbft_SUITE.erl
@@ -11,7 +11,8 @@
          one_actor_missing_test/1,
          two_actors_missing_test/1,
          encrypt_decrypt_test/1,
-         start_on_demand_test/1
+         start_on_demand_test/1,
+         one_actor_wrong_key_test/1
         ]).
 
 all() ->
@@ -22,7 +23,8 @@ all() ->
      one_actor_missing_test,
      two_actors_missing_test,
      encrypt_decrypt_test,
-     start_on_demand_test
+     start_on_demand_test,
+     one_actor_wrong_key_test
     ].
 
 init_per_testcase(_, Config) ->
@@ -274,6 +276,58 @@ start_on_demand_test(Config) ->
     io:format("chain contains ~p distinct transactions~n", [length(BlockTxns)]),
     ok.
 
+one_actor_wrong_key_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),
+    PrivateKeys0 = proplists:get_value(privatekeys, Config),
+    {ok, Dealer} = dealer:new(N, F+1, 'SS512'),
+    {ok, {_PubKey, PrivateKeys1}} = dealer:deal(Dealer),
+    PrivateKeys = [hd(PrivateKeys1)|tl(PrivateKeys0)],
+
+    ct:pal("Private keys ~p", [PrivateKeys]),
+    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) ->
+                          Destinations = random_n(rand:uniform(N), Workers),
+                          io:format("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 = hbbft_ct_utils:wait_until(fun() ->
+                                           Chains = sets:from_list(lists:map(fun(W) ->
+                                                                                     {ok, Blocks} = hbbft_worker:get_blocks(W),
+                                                                                     Blocks
+                                                                             end, tl(Workers))),
+
+                                           0 == lists:sum([element(2, erlang:process_info(W, message_queue_len)) || W <- Workers ]) andalso
+                                           1 == sets:size(Chains) andalso
+                                           0 /= length(hd(sets:to_list(Chains)))
+                                   end, 60*2, 500),
+
+
+    Chains = sets:from_list(lists:map(fun(W) ->
+                                              {ok, Blocks} = hbbft_worker:get_blocks(W),
+                                              Blocks
+                                      end, tl(Workers))),
+    1 = sets:size(Chains),
+    [Chain] = sets:to_list(Chains),
+    io:format("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_subset(sets:from_list(BlockTxns), sets:from_list(Msgs)),
+    io:format("chain contains ~p distinct transactions~n", [length(BlockTxns)]),
+    ok.
+
+
 %% helper functions
 
 enumerate(List) ->