Skip to content

Commit

Permalink
HC/FATE: fix issue when calling Chain.blockhash on a competing key-bl…
Browse files Browse the repository at this point in the history
…ock (#4263)

* Fix rare bug in FATE blockhash function

* Harden the HC consensus contract execution

* Disable HC tests on Iris - FATE issue
  • Loading branch information
hanssv committed Feb 5, 2024
1 parent fa02ab9 commit dc788c5
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 11 deletions.
2 changes: 1 addition & 1 deletion apps/aecontract/test/aecontract_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -691,7 +691,7 @@ init_per_testcase(TC, Config) when TC == aevm_version_interaction;
init_per_testcase(fate_environment, Config) ->
meck:new(aefa_chain_api, [passthrough]),
meck:expect(aefa_chain_api, blockhash,
fun(N, S) when is_integer(N) ->
fun(N, _, S) when is_integer(N) ->
%% Just to ensure the arg format
_ = aefa_chain_api:generation(S),
aeb_fate_data:make_hash(<<N:256>>)
Expand Down
10 changes: 8 additions & 2 deletions apps/aecore/src/aec_consensus_hc.erl
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,9 @@ state_pre_transform_key_node(Node, PrevNode, Trees) ->
elect_lazy_leader(Beneficiary, TxEnv, Trees) %% initial trees!!!
catch error:{consensus_call_failed, {error, Why}} ->
lager:info("Consensus contract failed with ~p", [Why]),
elect_lazy_leader(Beneficiary, TxEnv, Trees) %% initial trees!!!
elect_lazy_leader(Beneficiary, TxEnv, Trees); %% initial trees!!!
error:{consensus_call_crashed, {error, _Why}} ->
aec_conductor:throw_error(consensus_call_crashed)
end
end;
false -> Trees %% do not elect leader for genesis
Expand Down Expand Up @@ -535,7 +537,7 @@ call_consensus_contract_(ContractType, TxEnv, Trees, EncodedCallData, Keyword, A
gas_price => GasPrice,
call_data => CallData},
{ok, Tx} = aect_call_tx:new(CallSpec),
case aetx:process(Tx, Trees1, TxEnv) of
try aetx:process(Tx, Trees1, TxEnv) of
{ok, Trees2, _} ->
Calls = aec_trees:calls(Trees2),
{contract_call_tx, CallTx} = aetx:specialize_type(Tx),
Expand All @@ -556,6 +558,10 @@ call_consensus_contract_(ContractType, TxEnv, Trees, EncodedCallData, Keyword, A
{ok, aect_call_state_tree:prune(Height, Trees2), Call};
{error, _What} = Err ->
Err
catch Err:Reason:St ->
lager:error("Consensus contract call to ~p crashed(~p): ~p", [Keyword, Err, Reason]),
lager:info("Crash stacktrace: ~p", [St]),
error({consensus_call_crashed, {error, Reason}})
end.

beneficiary() ->
Expand Down
19 changes: 13 additions & 6 deletions apps/aefate/src/aefa_chain_api.erl
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
-export([ account_balance/2
, next_nonce/2
, beneficiary/1
, blockhash/2
, blockhash/3
, put_contract/2
, contract_vm_version/2
, contract_fate_bytecode/2
Expand Down Expand Up @@ -186,8 +186,8 @@ creator(Pubkey, #state{primop_state = PState}) ->
%%%-------------------------------------------------------------------
%%% Slightly more involved getters with caching

-spec blockhash(non_neg_integer(), #state{}) -> aeb_fate_data:fate_hash().
blockhash(Height, #state{} = S) ->
-spec blockhash(non_neg_integer(), aect_contracts:vm_version(), #state{}) -> aeb_fate_data:fate_hash().
blockhash(Height, VMVersion, #state{} = S) ->
TxEnv = tx_env(S),
case aetx_env:key_hash(TxEnv) of
<<0:?BLOCK_HEADER_HASH_BYTES/unit:8>> = Hash ->
Expand All @@ -201,9 +201,16 @@ blockhash(Height, #state{} = S) ->
{ok, <<_:?BLOCK_HEADER_HASH_BYTES/unit:8 >> = Hash} ->
aeb_fate_data:make_hash(Hash);
{ok, _Other} ->
<<_:?BLOCK_HEADER_HASH_BYTES/unit:8 >> = Hash =
traverse_to_key_hash(Height, KeyHash),
aeb_fate_data:make_hash(Hash)
case VMVersion > ?VM_FATE_SOPHIA_2 of
false -> %% Bug unintended match
<<_:?BLOCK_HEADER_HASH_BYTES/unit:8 >> = Hash =
traverse_to_key_hash(Height, KeyHash),
aeb_fate_data:make_hash(Hash);
true ->
<<_:?BLOCK_HEADER_HASH_BYTES/unit:8 >> = Hash1 =
traverse_to_key_hash(Height, KeyHash),
aeb_fate_data:make_hash(Hash1)
end
end
end.

Expand Down
2 changes: 1 addition & 1 deletion apps/aefate/src/aefa_fate_op.erl
Original file line number Diff line number Diff line change
Expand Up @@ -1405,7 +1405,7 @@ blockhash(Arg0, Arg1, ES) ->
true ->
write(Arg0, make_none(), ES1);
false ->
FateHash = aefa_chain_api:blockhash(N, API),
FateHash = aefa_chain_api:blockhash(N, VMVsn, API),
write(Arg0, make_some(FateHash), ES1)
end;
{Value, ES1} ->
Expand Down
18 changes: 17 additions & 1 deletion apps/aehttp/test/aehttp_stake_contract_SUITE.erl
Original file line number Diff line number Diff line change
Expand Up @@ -753,7 +753,15 @@ change_leaders(Config) ->
{ok, _B} = wait_same_top(),
ok.

empty_parent_block(_Config) ->
empty_parent_block(Config) ->
case aect_test_utils:latest_protocol_version() < ?CERES_PROTOCOL_VSN of
true ->
{skip, lazy_leader_sync_broken_on_iris};
false ->
empty_parent_block_(Config)
end.

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),
Expand Down Expand Up @@ -1099,6 +1107,14 @@ block_difficulty(Config) ->
ok.

elected_leader_did_not_show_up(Config) ->
case aect_test_utils:latest_protocol_version() < ?CERES_PROTOCOL_VSN of
true ->
{skip, lazy_leader_sync_broken_on_iris};
false ->
elected_leader_did_not_show_up_(Config)
end.

elected_leader_did_not_show_up_(Config) ->
aecore_suite_utils:stop_node(?NODE1, Config), %% stop the block producer
TopHeader0 = rpc(?NODE2, aec_chain, top_header, []),
{TopHeader0, TopHeader0} = {rpc(?LAZY_NODE, aec_chain, top_header, []), TopHeader0},
Expand Down

0 comments on commit dc788c5

Please sign in to comment.