From 04cce3ea038bd6cbda67f75bde69faf3dc5b2173 Mon Sep 17 00:00:00 2001 From: Lev Berman Date: Tue, 12 Nov 2019 16:34:06 +0100 Subject: [PATCH] Return specific error codes for failed txs --- apps/arweave/src/ar_http_iface_middleware.erl | 47 ++++---------- apps/arweave/src/ar_tx.erl | 61 +++++++++---------- apps/arweave/src/ar_tx_db.erl | 8 +-- apps/arweave/src/ar_tx_replay_pool.erl | 6 +- .../test/ar_multiple_txs_per_wallet_tests.erl | 24 ++++---- 5 files changed, 59 insertions(+), 87 deletions(-) diff --git a/apps/arweave/src/ar_http_iface_middleware.erl b/apps/arweave/src/ar_http_iface_middleware.erl index 22b14a4eb..f9d735126 100644 --- a/apps/arweave/src/ar_http_iface_middleware.erl +++ b/apps/arweave/src/ar_http_iface_middleware.erl @@ -697,8 +697,7 @@ get_tx_filename(Hash) -> false -> case ar_tx_db:get_error_codes(ID) of {ok, ErrorCodes} -> - ErrorBody = list_to_binary(lists:join(" ", ErrorCodes)), - {response, {410, #{}, ErrorBody}}; + {response, {410, #{}, jiffy:encode(ErrorCodes)}}; not_found -> {response, {404, #{}, <<"Not Found.">>}} end @@ -781,7 +780,7 @@ handle_post_tx(PeerIP, TX) -> Height = ar_node:get_height(Node), case verify_mempool_txs_size(MempoolTXs, TX, Height) of invalid -> - handle_post_tx_no_mempool_space_response(); + handle_post_tx_invalid(mempool_is_full); valid -> handle_post_tx(PeerIP, Node, TX, Height, [MTX#tx.id || MTX <- MempoolTXs]) end. @@ -808,7 +807,7 @@ handle_post_tx2(PeerIP, Node, TX, Height, MempoolTXIDs) -> {balance, Balance}, {tx_cost, TX#tx.reward + TX#tx.quantity} ]), - handle_post_tx_exceed_balance_response(); + handle_post_tx_invalid(tx_insufficient_funds); _ -> handle_post_tx(PeerIP, Node, TX, Height, MempoolTXIDs, WalletList) end. @@ -824,18 +823,8 @@ handle_post_tx(PeerIP, Node, TX, Height, MempoolTXIDs, WalletList) -> MempoolTXIDs, WalletList ) of - {invalid, tx_verification_failed} -> - handle_post_tx_verification_response(); - {invalid, last_tx_in_mempool} -> - handle_post_tx_last_tx_in_mempool_response(); - {invalid, invalid_last_tx} -> - handle_post_tx_verification_response(); - {invalid, tx_bad_anchor} -> - handle_post_tx_bad_anchor_response(); - {invalid, tx_already_in_weave} -> - handle_post_tx_already_in_weave_response(); - {invalid, tx_already_in_mempool} -> - handle_post_tx_already_in_mempool_response(); + {invalid, Reason} -> + handle_post_tx_invalid(Reason); {valid, _, _} -> handle_post_tx_accepted(PeerIP, TX) end. @@ -876,27 +865,13 @@ handle_post_tx_accepted(PeerIP, TX) -> ar_bridge:add_tx(whereis(http_bridge_node), TX), ok. -handle_post_tx_exceed_balance_response() -> - {error_response, {400, #{}, <<"Waiting TXs exceed balance for wallet.">>}}. +handle_post_tx_invalid({tx_verification_failed, ErrorCodes}) -> + post_invalid_tx_response(ErrorCodes); +handle_post_tx_invalid(ErrorCode) -> + post_invalid_tx_response([ErrorCode]). -handle_post_tx_verification_response() -> - {error_response, {400, #{}, <<"Transaction verification failed.">>}}. - -handle_post_tx_last_tx_in_mempool_response() -> - {error_response, {400, #{}, <<"Invalid anchor (last_tx from mempool).">>}}. - -handle_post_tx_no_mempool_space_response() -> - ar:err([ar_http_iface_middleware, rejected_transaction, {reason, mempool_is_full}]), - {error_response, {400, #{}, <<"Mempool is full.">>}}. - -handle_post_tx_bad_anchor_response() -> - {error_response, {400, #{}, <<"Invalid anchor (last_tx).">>}}. - -handle_post_tx_already_in_weave_response() -> - {error_response, {400, #{}, <<"Transaction is already on the weave.">>}}. - -handle_post_tx_already_in_mempool_response() -> - {error_response, {400, #{}, <<"Transaction is already in the mempool.">>}}. +post_invalid_tx_response(ErrorCodes) -> + {error_response, {400, #{}, jiffy:encode(ErrorCodes)}}. check_internal_api_secret(Req) -> Reject = fun(Msg) -> diff --git a/apps/arweave/src/ar_tx.erl b/apps/arweave/src/ar_tx.erl index 8ed8a5e6e..f4a551ebe 100644 --- a/apps/arweave/src/ar_tx.erl +++ b/apps/arweave/src/ar_tx.erl @@ -77,7 +77,7 @@ sign(TX, PrivKey, PubKey) -> %% NB: The signature verification is the last step due to the potential of an attacker %% submitting an arbitrary length signature field and having it processed. -ifdef(DEBUG). -verify(#tx { signature = <<>> }, _, _, _, _) -> true; +verify(#tx { signature = <<>> }, _, _, _, _) -> valid; verify(TX, Diff, Height, Wallets, Timestamp) -> do_verify(TX, Diff, Height, Wallets, Timestamp). -else. @@ -94,25 +94,23 @@ do_verify(TX, Diff, Height, Wallets, Timestamp) -> check_last_tx(Wallets, TX) end, Checks = [ - {"quantity_negative", + {quantity_negative, TX#tx.quantity >= 0}, - {"same_owner_as_target", + {same_owner_as_target, (ar_wallet:to_address(TX#tx.owner) =/= TX#tx.target)}, - {"tx_too_cheap", + {tx_too_cheap, tx_cost_above_min(TX, Diff, Height, Wallets, TX#tx.target, Timestamp)}, - {"tx_fields_too_large", - tx_field_size_limit(TX, Height)}, - {"tag_field_illegally_specified", + {tag_field_illegally_specified, tag_field_legal(TX)}, - {"last_tx_not_valid", + {last_tx_not_valid, LastTXCheck}, - {"tx_id_not_valid", + {tx_id_not_valid, tx_verify_hash(TX)}, - {"overspend", + {tx_insufficient_funds, validate_overspend(TX, ar_node_utils:apply_tx(Wallets, TX, Height))}, - {"tx_signature_not_valid", + {tx_signature_not_valid, ar_wallet:verify(TX#tx.owner, signature_data_segment(TX), TX#tx.signature)} - ], + ] ++ tx_field_size_limits(TX, Height), KeepFailed = fun ({_, true}) -> false; @@ -121,10 +119,10 @@ do_verify(TX, Diff, Height, Wallets, Timestamp) -> end, case lists:filtermap(KeepFailed, Checks) of [] -> - true; + valid; ErrorCodes -> ar_tx_db:put_error_codes(TX#tx.id, ErrorCodes), - false + {invalid, ErrorCodes} end. validate_overspend(TX, Wallets) -> @@ -179,7 +177,7 @@ verify_txs(valid_size_txs, [], _, _, _, _) -> true; verify_txs(valid_size_txs, [TX | TXs], Diff, Height, WalletMap, Timestamp) -> case verify(TX, Diff, Height, WalletMap, Timestamp) of - true -> + valid -> verify_txs( valid_size_txs, TXs, @@ -188,7 +186,7 @@ verify_txs(valid_size_txs, [TX | TXs], Diff, Height, WalletMap, Timestamp) -> ar_node_utils:apply_tx(WalletMap, TX, Height), Timestamp ); - false -> + {invalid, _} -> false end. @@ -235,7 +233,7 @@ min_tx_cost(DataSize, _Diff, DiffCenter) -> min_tx_cost(DataSize, DiffCenter, DiffCenter). %% @doc Check whether each field in a transaction is within the given byte size limits. -tx_field_size_limit(TX, Height) -> +tx_field_size_limits(TX, Height) -> Fork_1_8 = ar_fork:height_1_8(), LastTXLimit = case Height of H when H >= Fork_1_8 -> @@ -243,19 +241,18 @@ tx_field_size_limit(TX, Height) -> _ -> 32 end, - case tag_field_legal(TX) of - true -> - (byte_size(TX#tx.id) =< 32) and - (byte_size(TX#tx.last_tx) =< LastTXLimit) and - (byte_size(TX#tx.owner) =< 512) and - (byte_size(tags_to_binary(TX#tx.tags)) =< 2048) and - (byte_size(TX#tx.target) =< 32) and - (byte_size(integer_to_binary(TX#tx.quantity)) =< 21) and - (byte_size(TX#tx.data) =< (?TX_DATA_SIZE_LIMIT)) and - (byte_size(TX#tx.signature) =< 512) and - (byte_size(integer_to_binary(TX#tx.reward)) =< 21); - false -> false - end. + [ + {tags_is_not_a_list_of_pairs, tag_field_legal(TX)}, + {tx_id_too_large, byte_size(TX#tx.id) =< 32}, + {last_tx_too_large, byte_size(TX#tx.last_tx) =< LastTXLimit}, + {tx_owner_too_large, byte_size(TX#tx.owner) =< 512}, + {tx_tags_too_large, byte_size(tags_to_binary(TX#tx.tags)) =< 2048}, + {tx_quantity_too_large, byte_size(integer_to_binary(TX#tx.quantity)) =< 21}, + {tx_target_too_large, byte_size(TX#tx.target) =< 32}, + {tx_data_too_large, byte_size(TX#tx.data) =< (?TX_DATA_SIZE_LIMIT)}, + {tx_signature_too_large, byte_size(TX#tx.signature) =< 512}, + {tx_reward_too_large, byte_size(integer_to_binary(TX#tx.reward)) =< 21} + ]. %% @doc Verify that the transactions ID is a hash of its signature. tx_verify_hash(#tx {signature = Sig, id = ID}) -> @@ -341,7 +338,7 @@ sign_tx_test() -> Diff = 1, Height = 0, Timestamp = os:system_time(seconds), - ?assert(verify(sign(NewTX, Priv, Pub), Diff, Height, [], Timestamp)). + ?assertEqual(valid, verify(sign(NewTX, Priv, Pub), Diff, Height, [], Timestamp)). %% @doc Ensure that a forged transaction does not pass verification. forge_test() -> @@ -351,7 +348,7 @@ forge_test() -> Height = 0, InvalidSignTX = (sign(NewTX, Priv, Pub))#tx { data = <<"FAKE DATA">> }, Timestamp = os:system_time(seconds), - ?assert(not verify(InvalidSignTX, Diff, Height, [], Timestamp)). + ?assertEqual({invalid, [tx_signature_not_valid]}, verify(InvalidSignTX, Diff, Height, [], Timestamp)). %% @doc Ensure that transactions above the minimum tx cost are accepted. tx_cost_above_min_test() -> diff --git a/apps/arweave/src/ar_tx_db.erl b/apps/arweave/src/ar_tx_db.erl index 88cd10151..8b9201619 100644 --- a/apps/arweave/src/ar_tx_db.erl +++ b/apps/arweave/src/ar_tx_db.erl @@ -74,13 +74,13 @@ tx_db_test() -> OrphanedTX1 = ar_tx:new(Pub1, ?AR(1), ?AR(5000), <<>>), BadTX = OrphanedTX1#tx { owner = Pub1, signature = <<"BAD">> }, Timestamp = os:system_time(seconds), - ?assert(not ar_tx:verify(BadTX, 8, 1, B0#block.wallet_list, Timestamp)), - Expected = {ok, ["same_owner_as_target", "tx_id_not_valid", "tx_signature_not_valid"]}, - ?assertEqual(Expected, get_error_codes(BadTX#tx.id)), + ExpectedErrorCodes = [same_owner_as_target, tx_id_not_valid, tx_signature_not_valid], + ?assertEqual({invalid, ExpectedErrorCodes}, ar_tx:verify(BadTX, 8, 1, B0#block.wallet_list, Timestamp)), + ?assertEqual({ok, ExpectedErrorCodes}, get_error_codes(BadTX#tx.id)), %% Test good transaction OrphanedTX2 = ar_tx:new(Pub1, ?AR(1), ?AR(5000), <<>>), SignedTX = ar_tx:sign(OrphanedTX2, Priv2, Pub2), - ?assert(ar_tx:verify(SignedTX, 8, 1, B0#block.wallet_list, Timestamp)), + ?assertEqual(valid, ar_tx:verify(SignedTX, 8, 1, B0#block.wallet_list, Timestamp)), clear_error_codes(BadTX#tx.id), clear_error_codes(SignedTX#tx.id), ok. diff --git a/apps/arweave/src/ar_tx_replay_pool.erl b/apps/arweave/src/ar_tx_replay_pool.erl index ae233e69e..a8231a6b1 100644 --- a/apps/arweave/src/ar_tx_replay_pool.erl +++ b/apps/arweave/src/ar_tx_replay_pool.erl @@ -133,10 +133,10 @@ create_state(BlockTXPairs) -> verify_tx(general_verification, TX, Diff, Height, Timestamp, FloatingWallets, WeaveState, Mempool) -> case ar_tx:verify(TX, Diff, Height, FloatingWallets, Timestamp) of - true -> + valid -> verify_tx(last_tx_in_mempool, TX, Diff, Height, FloatingWallets, WeaveState, Mempool); - false -> - {invalid, tx_verification_failed} + {invalid, ErrorCodes} -> + {invalid, {tx_verification_failed, ErrorCodes}} end. verify_tx(last_tx_in_mempool, TX, Diff, Height, FloatingWallets, WeaveState, Mempool) -> diff --git a/apps/arweave/test/ar_multiple_txs_per_wallet_tests.erl b/apps/arweave/test/ar_multiple_txs_per_wallet_tests.erl index ca2904e6a..8eb7a75b2 100644 --- a/apps/arweave/test/ar_multiple_txs_per_wallet_tests.erl +++ b/apps/arweave/test/ar_multiple_txs_per_wallet_tests.erl @@ -299,7 +299,7 @@ returns_error_when_txs_exceed_balance(B0, TXs, ExceedBalanceTX) -> %% Post the balance exceeding transaction again %% and expect the balance exceeded error. slave_call(ets, delete, [ignored_ids, ExceedBalanceTX#tx.id]), - {ok, {{<<"400">>, _}, _, <<"Waiting TXs exceed balance for wallet.">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_insufficient_funds\"]">>, _, _}} = ar_httpc:request( <<"POST">>, {127, 0, 0, 1, slave_call(ar_meta_db, get, [port])}, @@ -324,8 +324,8 @@ rejects_transactions_above_the_size_limit() -> GoodTX = sign_tx(Key1, #{ data => SmallData }), assert_post_tx_to_slave(Slave, GoodTX), BadTX = sign_tx(Key2, #{ data => BigData }), - {ok, {{<<"400">>, _}, _, <<"Transaction verification failed.">>, _, _}} = post_tx_to_slave(Slave, BadTX), - {ok, ["tx_fields_too_large"]} = slave_call(ar_tx_db, get_error_codes, [BadTX#tx.id]). + {ok, {{<<"400">>, _}, _, <<"[\"tx_data_too_large\"]">>, _, _}} = post_tx_to_slave(Slave, BadTX), + {ok, [tx_data_too_large]} = slave_call(ar_tx_db, get_error_codes, [BadTX#tx.id]). accepts_at_most_one_wallet_list_anchored_tx_per_block() -> %% Post a TX, mine a block. @@ -350,7 +350,7 @@ accepts_at_most_one_wallet_list_anchored_tx_per_block() -> TX2 = sign_tx(Key, #{ last_tx => TX1#tx.id }), assert_post_tx_to_slave(Slave, TX2), TX3 = sign_tx(Key, #{ last_tx => TX2#tx.id }), - {ok, {{<<"400">>, _}, _, <<"Invalid anchor (last_tx from mempool).">>, _, _}} = post_tx_to_slave(Slave, TX3), + {ok, {{<<"400">>, _}, _, <<"[\"last_tx_in_mempool\"]">>, _, _}} = post_tx_to_slave(Slave, TX3), TX4 = sign_tx(Key, #{ last_tx => B0#block.indep_hash }), assert_post_tx_to_slave(Slave, TX4), slave_mine(Slave), @@ -388,7 +388,7 @@ does_not_allow_to_spend_mempool_tokens() -> tags => [{<<"nonce">>, <<"1">>}] } ), - {ok, {{<<"400">>, _}, _, <<"Waiting TXs exceed balance for wallet.">>, _, _}} = post_tx_to_slave(Slave, TX2), + {ok, {{<<"400">>, _}, _, <<"[\"tx_insufficient_funds\"]">>, _, _}} = post_tx_to_slave(Slave, TX2), slave_mine(Slave), SlaveBHL = assert_slave_wait_until_height(Slave, 1), B1 = slave_call(ar_storage, read_block, [hd(SlaveBHL), SlaveBHL]), @@ -476,7 +476,7 @@ does_not_allow_to_replay_empty_wallet_txs() -> assert_slave_wait_until_height(Slave, 3), %% Remove the replay TX from the ingnore list (to simulate e.g. a node restart). slave_call(ets, delete, [ignored_ids, TX2#tx.id]), - {ok, {{<<"400">>, _}, _, <<"Invalid anchor (last_tx).">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_bad_anchor\"]">>, _, _}} = post_tx_to_slave(Slave, TX2). mines_blocks_under_the_size_limit(B0, TXGroups) -> @@ -522,7 +522,7 @@ rejects_txs_with_outdated_anchors() -> slave_mine_blocks(Slave, ?MAX_TX_ANCHOR_DEPTH), assert_slave_wait_until_height(Slave, ?MAX_TX_ANCHOR_DEPTH), TX1 = sign_tx(Key, #{ last_tx => B0#block.indep_hash }), - {ok, {{<<"400">>, _}, _, <<"Invalid anchor (last_tx).">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_bad_anchor\"]">>, _, _}} = post_tx_to_slave(Slave, TX1). rejects_txs_exceeding_mempool_limit() -> @@ -554,7 +554,7 @@ rejects_txs_exceeding_mempool_limit() -> end, lists:sublist(TXs, 5) ), - {ok, {{<<"400">>, _}, _, <<"Mempool is full.">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"mempool_is_full\"]">>, _, _}} = post_tx_to_slave(Slave, lists:last(TXs)). joins_network_successfully(ForkHeight) -> @@ -618,7 +618,7 @@ joins_network_successfully(ForkHeight) -> BHL = slave_call(ar_node, get_hash_list, [Slave]), assert_wait_until_block_hash_list(Master, BHL), TX1 = sign_tx(Key, #{ last_tx => lists:nth(?MAX_TX_ANCHOR_DEPTH + 1, BHL) }), - {ok, {{<<"400">>, _}, _, <<"Invalid anchor (last_tx).">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_bad_anchor\"]">>, _, _}} = post_tx_to_master(Master, TX1), TX2 = sign_tx(Key, #{ last_tx => lists:nth(?MAX_TX_ANCHOR_DEPTH, BHL) }), assert_post_tx_to_master(Master, TX2), @@ -626,14 +626,14 @@ joins_network_successfully(ForkHeight) -> forget_txs(PreForkTXs ++ PostForkTXs), lists:foreach( fun(TX) -> - {ok, {{<<"400">>, _}, _, <<"Invalid anchor (last_tx).">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_bad_anchor\"]">>, _, _}} = post_tx_to_master(Master, TX) end, PreForkTXs ), lists:foreach( fun(TX) -> - {ok, {{<<"400">>, _}, _, <<"Transaction is already on the weave.">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_already_in_weave\"]">>, _, _}} = post_tx_to_master(Master, TX) end, PostForkTXs @@ -798,7 +798,7 @@ recovers_from_forks(ForkHeight, ForkHeight_1_8) -> fun(TX) -> Confirmations = get_tx_confirmations(master, TX#tx.id), ?assertEqual(1, Confirmations), - {ok, {{<<"400">>, _}, _, <<"Transaction is already on the weave.">>, _, _}} = + {ok, {{<<"400">>, _}, _, <<"[\"tx_already_in_weave\"]">>, _, _}} = post_tx_to_master(Master, TX) end, MasterPostForkBlockAnchoredTXs -- [IncludeOnMasterTX]