From 2369c568821e0b9a34a6e7913a122abda47f4357 Mon Sep 17 00:00:00 2001 From: macpie Date: Fri, 15 Apr 2022 15:59:37 -0700 Subject: [PATCH 01/10] Introduce var to change how xor filter update txn fees are calculated --- include/blockchain_vars.hrl | 3 ++ .../v1/blockchain_ledger_routing_v1.erl | 1 + .../v1/blockchain_txn_routing_v1.erl | 46 ++++++++++++++++++- .../v1/blockchain_txn_vars_v1.erl | 9 ++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/include/blockchain_vars.hrl b/include/blockchain_vars.hrl index d6bef03641..83c747e9fa 100644 --- a/include/blockchain_vars.hrl +++ b/include/blockchain_vars.hrl @@ -392,6 +392,9 @@ %% - 1 :: accept first dispute, drop all DC from opener, no rewards -define(sc_dispute_strategy_version, sc_dispute_strategy_version). +%% Txn Routing Xor Filter Fee calculation var HIP-XXX +-define(txn_routing_update_xor_fees_version, txn_routing_update_xor_fees_version). + %% ------------------------------------------------------------------ %% snapshot vars diff --git a/src/ledger/v1/blockchain_ledger_routing_v1.erl b/src/ledger/v1/blockchain_ledger_routing_v1.erl index 6fe2ffb562..8bac1e1897 100644 --- a/src/ledger/v1/blockchain_ledger_routing_v1.erl +++ b/src/ledger/v1/blockchain_ledger_routing_v1.erl @@ -173,6 +173,7 @@ replace(Index, Element, List) -> {Head, [_ToRemove|Tail]} = lists:split(Index, List), Head ++ [Element] ++ Tail. + %% ------------------------------------------------------------------ %% EUNIT Tests %% ------------------------------------------------------------------ diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index 76d7f3eee9..5f54f4dd33 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -168,8 +168,50 @@ calculate_fee(Txn, Chain) -> -spec calculate_fee(txn_routing(), blockchain_ledger_v1:ledger(), pos_integer(), pos_integer(), boolean()) -> non_neg_integer(). calculate_fee(_Txn, _Ledger, _DCPayloadSize, _TxnFeeMultiplier, false) -> ?LEGACY_TXN_FEE; -calculate_fee(Txn, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> - ?calculate_fee(Txn#blockchain_txn_routing_v1_pb{fee=0, staking_fee = 0, signature = <<0:512>>}, Ledger, DCPayloadSize, TxnFeeMultiplier). +calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> + FeeVersion = + case blockchain:config(?txn_routing_update_xor_fees_version, Ledger) of + {ok, V} -> V; + _ -> 0 + end, + Txn1 = Txn0#blockchain_txn_routing_v1_pb{ + fee=0, + staking_fee = 0, + signature = <<0:512>> + }, + case FeeVersion of + 1 -> + case Txn0#blockchain_txn_routing_v1_pb.update of + {update_xor, Index, Filter} -> + %% Find out current size at index + %% Get new size + %% calulate diff + OUI = Txn0#blockchain_txn_routing_v1_pb.oui, + OldFilter = + case blockchain_ledger_v1:find_routing(OUI, Ledger) of + {ok, Routing} -> + Filters = blockchain_ledger_routing_v1:filters(Routing), + %% We do +1 here because Index is "0 indexed" + %% and lists:nth/2 does not support 0 + lists:nth(Index+1, Filters); + _Error -> + <<>> + end, + SizeDiff = erlang:byte_size(OldFilter) - erlang:byte_size(Filter), + case SizeDiff < 0 of + true -> + Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<>>}}, + ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier); + false -> + Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, crypto:strong_rand_bytes(SizeDiff)}}, + ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier) + end; + _ -> + ?calculate_fee(Txn1, Ledger, DCPayloadSize, TxnFeeMultiplier) + end; + _ -> + ?calculate_fee(Txn1, Ledger, DCPayloadSize, TxnFeeMultiplier ) + end. %%-------------------------------------------------------------------- %% @doc diff --git a/src/transactions/v1/blockchain_txn_vars_v1.erl b/src/transactions/v1/blockchain_txn_vars_v1.erl index e2780067e9..dbf14b7e38 100644 --- a/src/transactions/v1/blockchain_txn_vars_v1.erl +++ b/src/transactions/v1/blockchain_txn_vars_v1.erl @@ -1239,6 +1239,15 @@ validate_var(?sc_only_count_open_active, Value) -> validate_var(?sc_dispute_strategy_version, Value) -> validate_int(Value, "sc_dispute_strategy_version", 0, 1, false); +%% Txn Routing Xor Filter Fee calculation var HIP-XXX +validate_var(?txn_routing_update_xor_fees_version, Value) -> + case Value of + N when is_integer(N), N == 1 -> + ok; + _ -> + throw({error, {invalid_txn_routing_update_xor_fees_version, Value}}) + end; + %% txn snapshot vars validate_var(?snapshot_version, Value) -> case Value of From e3726cef57c7d7b0fd8f21bb1ab8573f08f3a983 Mon Sep 17 00:00:00 2001 From: macpie Date: Mon, 18 Apr 2022 10:06:01 -0700 Subject: [PATCH 02/10] Update comments --- src/transactions/v1/blockchain_txn_routing_v1.erl | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index 5f54f4dd33..5bc5ed4409 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -183,9 +183,7 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> 1 -> case Txn0#blockchain_txn_routing_v1_pb.update of {update_xor, Index, Filter} -> - %% Find out current size at index - %% Get new size - %% calulate diff + %% Find out current size at index, get new size, calculate diff OUI = Txn0#blockchain_txn_routing_v1_pb.oui, OldFilter = case blockchain_ledger_v1:find_routing(OUI, Ledger) of @@ -198,6 +196,8 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> <<>> end, SizeDiff = erlang:byte_size(OldFilter) - erlang:byte_size(Filter), + %% If diff < 0, meaning that old filter is bigger than new one we set new filter to be empty + %% If diff > 0, we calculate fees base on a random binarey that size case SizeDiff < 0 of true -> Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<>>}}, From f9a91d103493aa79bd1820267e1ab5a0f6ed008a Mon Sep 17 00:00:00 2001 From: macpie Date: Mon, 18 Apr 2022 11:16:54 -0700 Subject: [PATCH 03/10] Remove extra empty line --- src/ledger/v1/blockchain_ledger_routing_v1.erl | 1 - 1 file changed, 1 deletion(-) diff --git a/src/ledger/v1/blockchain_ledger_routing_v1.erl b/src/ledger/v1/blockchain_ledger_routing_v1.erl index 8bac1e1897..6fe2ffb562 100644 --- a/src/ledger/v1/blockchain_ledger_routing_v1.erl +++ b/src/ledger/v1/blockchain_ledger_routing_v1.erl @@ -173,7 +173,6 @@ replace(Index, Element, List) -> {Head, [_ToRemove|Tail]} = lists:split(Index, List), Head ++ [Element] ++ Tail. - %% ------------------------------------------------------------------ %% EUNIT Tests %% ------------------------------------------------------------------ From f4af82d231fa73a026125039f45ba9862bb46514 Mon Sep 17 00:00:00 2001 From: macpie Date: Mon, 18 Apr 2022 11:18:45 -0700 Subject: [PATCH 04/10] Typo --- src/transactions/v1/blockchain_txn_routing_v1.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index 5bc5ed4409..d1e5cdce1b 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -197,7 +197,7 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> end, SizeDiff = erlang:byte_size(OldFilter) - erlang:byte_size(Filter), %% If diff < 0, meaning that old filter is bigger than new one we set new filter to be empty - %% If diff > 0, we calculate fees base on a random binarey that size + %% If diff > 0, we calculate fees based on a random binary of same size case SizeDiff < 0 of true -> Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<>>}}, From 6b8166c86e1826a519c5814df05fb925101a709a Mon Sep 17 00:00:00 2001 From: macpie Date: Mon, 18 Apr 2022 14:28:53 -0700 Subject: [PATCH 05/10] Fix pr comments --- src/transactions/v1/blockchain_txn_routing_v1.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index d1e5cdce1b..0f0cf30a3e 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -193,6 +193,7 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> %% and lists:nth/2 does not support 0 lists:nth(Index+1, Filters); _Error -> + lager:error("we failed to get routing info for OUI=~p : ~p", [OUI, _Error]), <<>> end, SizeDiff = erlang:byte_size(OldFilter) - erlang:byte_size(Filter), @@ -203,7 +204,7 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<>>}}, ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier); false -> - Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, crypto:strong_rand_bytes(SizeDiff)}}, + Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<0:(8*SizeDiff)>>}}, ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier) end; _ -> From cef45ca88680d26c0a3f0bcb7df99336571d381b Mon Sep 17 00:00:00 2001 From: Macpie Date: Wed, 27 Apr 2022 10:26:27 -0700 Subject: [PATCH 06/10] Fix SizeDiff comparison --- src/transactions/v1/blockchain_txn_routing_v1.erl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index 0f0cf30a3e..af9b0865c4 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -197,9 +197,9 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> <<>> end, SizeDiff = erlang:byte_size(OldFilter) - erlang:byte_size(Filter), - %% If diff < 0, meaning that old filter is bigger than new one we set new filter to be empty + %% If diff =< 0, meaning that old filter is bigger than new one we set new filter to be empty %% If diff > 0, we calculate fees based on a random binary of same size - case SizeDiff < 0 of + case SizeDiff =< 0 of true -> Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<>>}}, ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier); From 95ab0e60bb1702a4b4592f2d7e5bdb66356f8631 Mon Sep 17 00:00:00 2001 From: Macpie Date: Wed, 27 Apr 2022 11:04:17 -0700 Subject: [PATCH 07/10] Fix calculate_fee and add eunit --- .../v1/blockchain_txn_routing_v1.erl | 59 ++++++++++++++++++- 1 file changed, 56 insertions(+), 3 deletions(-) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index af9b0865c4..911c55e852 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -181,7 +181,8 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> }, case FeeVersion of 1 -> - case Txn0#blockchain_txn_routing_v1_pb.update of + Action = ?MODULE:action(Txn1), + case Action of {update_xor, Index, Filter} -> %% Find out current size at index, get new size, calculate diff OUI = Txn0#blockchain_txn_routing_v1_pb.oui, @@ -201,10 +202,14 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> %% If diff > 0, we calculate fees based on a random binary of same size case SizeDiff =< 0 of true -> - Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<>>}}, + Txn2 = Txn1#blockchain_txn_routing_v1_pb{ + update={update_xor, #update_xor_pb{index=Index, filter= <<>>}} + }, ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier); false -> - Txn2 = Txn1#blockchain_txn_routing_v1_pb{update= {update_xor, Index, <<0:(8*SizeDiff)>>}}, + Txn2 = Txn1#blockchain_txn_routing_v1_pb{ + update={update_xor, #update_xor_pb{index=Index, filter= <<0:(8*SizeDiff)>>}} + }, ?calculate_fee(Txn2, Ledger, DCPayloadSize, TxnFeeMultiplier) end; _ -> @@ -627,4 +632,52 @@ to_json_test() -> ?assert(lists:all(fun(K) -> maps:is_key(K, Json) end, [type, hash, oui, owner, fee, action, nonce])). + +calculate_fee_test_() -> + {timeout, 30000, + fun() -> + OUI = 1, + Owner = crypto:strong_rand_bytes(32), + Routing = + blockchain_ledger_routing_v1:new( + OUI, + Owner, + [], + crypto:strong_rand_bytes(100), + <<>>, + 1 + ), + meck:new(blockchain_ledger_v1, [passthrough]), + meck:expect(blockchain_ledger_v1, find_routing, fun(_, _) -> + {ok, Routing} + end), + + meck:new(blockchain, [passthrough]), + %% Set txn_routing_update_xor_fees_version to 0 + meck:expect(blockchain, config, fun(_, _) -> + {ok, 0} + end), + + Index = 0, + Xor = crypto:strong_rand_bytes(200), + Nonce = 1, + Txn = blockchain_txn_routing_v1:update_xor(OUI, Owner, Index, Xor, Nonce), + + ?assertEqual(?LEGACY_TXN_FEE, calculate_fee(Txn, ledger, 1, 1, false)), + + ?assertEqual(313, calculate_fee(Txn, ledger, 1, 1, true)), + + %% Set txn_routing_update_xor_fees_version to 1 + meck:expect(blockchain, config, fun(_, _) -> + {ok, 1} + end), + + ?assertEqual(108, calculate_fee(Txn, ledger, 1, 1, true)), + + meck:unload(blockchain_ledger_v1), + meck:unload(blockchain), + ok + end + }. + -endif. From 127f90c7b59fe90ef4deee61ec9922e246c7cc80 Mon Sep 17 00:00:00 2001 From: Macpie Date: Wed, 27 Apr 2022 11:15:22 -0700 Subject: [PATCH 08/10] More eunits --- src/transactions/v1/blockchain_txn_routing_v1.erl | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index 911c55e852..ded009cb3e 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -197,7 +197,7 @@ calculate_fee(Txn0, Ledger, DCPayloadSize, TxnFeeMultiplier, true) -> lager:error("we failed to get routing info for OUI=~p : ~p", [OUI, _Error]), <<>> end, - SizeDiff = erlang:byte_size(OldFilter) - erlang:byte_size(Filter), + SizeDiff = erlang:byte_size(Filter) - erlang:byte_size(OldFilter), %% If diff =< 0, meaning that old filter is bigger than new one we set new filter to be empty %% If diff > 0, we calculate fees based on a random binary of same size case SizeDiff =< 0 of @@ -663,8 +663,10 @@ calculate_fee_test_() -> Nonce = 1, Txn = blockchain_txn_routing_v1:update_xor(OUI, Owner, Index, Xor, Nonce), + %% Testing legacy path ?assertEqual(?LEGACY_TXN_FEE, calculate_fee(Txn, ledger, 1, 1, false)), + %% Testing old version (pay for full xor size) ?assertEqual(313, calculate_fee(Txn, ledger, 1, 1, true)), %% Set txn_routing_update_xor_fees_version to 1 @@ -672,7 +674,14 @@ calculate_fee_test_() -> {ok, 1} end), - ?assertEqual(108, calculate_fee(Txn, ledger, 1, 1, true)), + %% Testing with version 1 (pay for diff of xor size) + ?assertEqual(211, calculate_fee(Txn, ledger, 1, 1, true)), + + Xor1 = crypto:strong_rand_bytes(50), + Txn1 = blockchain_txn_routing_v1:update_xor(OUI, Owner, Index, Xor1, Nonce), + + %% Testing with smaller xor + ?assertEqual(108, calculate_fee(Txn1, ledger, 1, 1, true)), meck:unload(blockchain_ledger_v1), meck:unload(blockchain), From 14b9b5bd9d2f41668c4c5e7d8d6de78b6467d0e2 Mon Sep 17 00:00:00 2001 From: Macpie Date: Wed, 27 Apr 2022 11:22:06 -0700 Subject: [PATCH 09/10] More test --- src/transactions/v1/blockchain_txn_routing_v1.erl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index ded009cb3e..25f282b48f 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -683,6 +683,13 @@ calculate_fee_test_() -> %% Testing with smaller xor ?assertEqual(108, calculate_fee(Txn1, ledger, 1, 1, true)), + Xor2 = crypto:strong_rand_bytes(102), + Txn2 = blockchain_txn_routing_v1:update_xor(OUI, Owner, Index, Xor2, Nonce), + + %% Testing with small update to show price diff with previous version + %% 112 compare to > 300 + ?assertEqual(112, calculate_fee(Txn2, ledger, 1, 1, true)), + meck:unload(blockchain_ledger_v1), meck:unload(blockchain), ok From eb1d6dcdd6af45a2177b3b730fd4e0f7394de32c Mon Sep 17 00:00:00 2001 From: macpie Date: Wed, 27 Apr 2022 15:16:43 -0700 Subject: [PATCH 10/10] Update src/transactions/v1/blockchain_txn_routing_v1.erl Co-authored-by: Rahul Garg <3199183+vihu@users.noreply.github.com> --- src/transactions/v1/blockchain_txn_routing_v1.erl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/transactions/v1/blockchain_txn_routing_v1.erl b/src/transactions/v1/blockchain_txn_routing_v1.erl index 25f282b48f..c525c4e414 100644 --- a/src/transactions/v1/blockchain_txn_routing_v1.erl +++ b/src/transactions/v1/blockchain_txn_routing_v1.erl @@ -689,6 +689,12 @@ calculate_fee_test_() -> %% Testing with small update to show price diff with previous version %% 112 compare to > 300 ?assertEqual(112, calculate_fee(Txn2, ledger, 1, 1, true)), + + Xor3 = crypto:strong_rand_bytes(102), + Txn3 = blockchain_txn_routing_v1:update_xor(OUI, Owner, Index, Xor3, Nonce), + + %% Xor3 is same size as Xor2, no change in fee. + ?assertEqual(112, calculate_fee(Txn3, ledger, 1, 1, true)), meck:unload(blockchain_ledger_v1), meck:unload(blockchain),