Skip to content

Commit

Permalink
PR updates
Browse files Browse the repository at this point in the history
  • Loading branch information
@JamesPiechota
JamesPiechota committed Jul 12, 2024
1 parent aad215e commit 9e18942
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 23 deletions.
9 changes: 7 additions & 2 deletions apps/arweave/src/ar_http_iface_middleware.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,13 @@ handle(<<"GET">>, [<<"info">>], Req, _Pid) ->
{200, #{}, ar_serialize:jsonify(ar_info:get_info()), Req};

handle(<<"GET">>, [<<"recent">>], Req, _Pid) ->
{200, #{}, ar_serialize:jsonify(ar_info:get_recent()), Req};

case ar_node:is_joined() of
false ->
not_joined(Req);
true ->
{200, #{}, ar_serialize:jsonify(ar_info:get_recent()), Req}
end;

handle(<<"GET">>, [<<"is_tx_blacklisted">>, EncodedTXID], Req, _Pid) ->
case ar_util:safe_decode(EncodedTXID) of
{error, invalid} ->
Expand Down
20 changes: 12 additions & 8 deletions apps/arweave/src/ar_info.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,22 @@ get_recent() ->
<<"forks">> => get_recent_forks()
}.

%% @doc Return the the most recent blocks in reverse chronological order.
get_recent_blocks() ->
Anchors = lists:sublist(ar_node:get_block_anchors(), ?CHECKPOINT_DEPTH),
CutOff = length(Anchors) - ?RECENT_BLOCKS_WITHOUT_TIMESTAMP,

This comment has been minimized.

Sign in to view

Copy link
@ldmberman

ldmberman Jul 12, 2024

Member

Should not CutOff be ?RECENT_BLOCKS_WITHOUT_TIMESTAMP and the guard below when Depth < CutOff ->? The block anchors are latest first and we traverse them from the left in foldl so the first iteration would call get_block_timestamp(H, 0, CutOff)

This comment has been minimized.

Sign in to view

Copy link
@JamesPiechota

JamesPiechota Jul 12, 2024

Author Collaborator

Hm, yeah. That's what I had before... but when I reviewed the tests the block ordering was actually reversed. Based on tests the current implementation correctly returns the blocks in reverse chronological order and blanks out the most recent X times.

But let me revisit, because I agree it seems like what you're suggesting (and what was there before) should be correct

This comment has been minimized.

Sign in to view

Copy link
@JamesPiechota

JamesPiechota Jul 12, 2024

Author Collaborator

Ah found the issue. The test was gossipping the blocks in the reverse order. Working on a fix now.
lists:foldl(
fun(H, Acc) ->
Acc ++ [#{
[#{
<<"id">> => ar_util:encode(H),
<<"received">> => get_block_timestamp(H, length(Acc))
}]
<<"received">> => get_block_timestamp(H, length(Acc), CutOff)
} | Acc]
end,
[],
lists:sublist(ar_node:get_block_anchors(), ?CHECKPOINT_DEPTH)
Anchors
).

%% @doc Return the the most recent forks in reverse chronological order.
get_recent_forks() ->
CutOffTime = os:system_time(seconds) - ?RECENT_FORKS_AGE,
case ar_chain_stats:get_forks(CutOffTime) of
Expand All @@ -77,21 +81,21 @@ get_recent_forks() ->
#fork{
id = ID, height = Height, timestamp = Timestamp,
block_ids = BlockIDs} = Fork,
Acc ++ [#{
[#{
<<"id">> => ar_util:encode(ID),
<<"height">> => Height,
<<"timestamp">> => Timestamp div 1000,
<<"blocks">> => [ ar_util:encode(BlockID) || BlockID <- BlockIDs ]
}]
} | Acc]
end,
[],
lists:sublist(Forks, ?RECENT_FORKS_LENGTH)
)
end.

get_block_timestamp(H, Depth) when Depth < ?RECENT_BLOCKS_WITHOUT_TIMESTAMP ->
get_block_timestamp(H, Depth, CutOff) when Depth >= CutOff ->
<<"pending">>;
get_block_timestamp(H, _Depth) ->
get_block_timestamp(H, _Depth, _Cutoff) ->
B = ar_block_cache:get(block_cache, H),
case B#block.receive_timestamp of
undefined -> <<"pending">>;
Expand Down
2 changes: 1 addition & 1 deletion apps/arweave/src/ar_node.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ get_block_anchors() ->
[{block_anchors, BlockAnchors}] ->
BlockAnchors;
[] ->
[]
not_joined
end.

%% @doc Return a map TXID -> ok containing all the recent transaction identifiers.
Expand Down
20 changes: 8 additions & 12 deletions apps/arweave/test/ar_info_tests.erl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ test_recent_blocks(Type) ->
%% Share blocks to peer1
lists:foreach(
fun({H, _WeaveSize, _TXRoot}) ->
timer:sleep(1000),
B = ar_test_node:remote_call(peer1, ar_block_cache, get, [block_cache, H]),
case Type of
post ->
Expand All @@ -73,11 +74,6 @@ test_recent_blocks(Type) ->
%% ones.
?assertEqual(expected_blocks(peer1, PeerBI),
get_recent(ar_test_node:peer_ip(peer1), blocks)).

assert_forks_equal(ExpectedForks, ActualForks) ->
ExpectedForksStripped = [ Fork#fork{timestamp = undefined} || Fork <- ExpectedForks],
ActualForksStripped = [ Fork#fork{timestamp = undefined} || Fork <- ActualForks],
?assertEqual(ExpectedForksStripped, ActualForksStripped).

expected_blocks(Node, BI) ->
expected_blocks(Node, BI, false).
Expand All @@ -99,7 +95,7 @@ expected_blocks(Node, BI, ForcePending) ->
} | Acc]
end,
[],
lists:reverse(lists:sublist(BI, ?CHECKPOINT_DEPTH))
lists:sublist(BI, ?CHECKPOINT_DEPTH)
).

%% -------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -127,7 +123,7 @@ test_get_recent_forks() ->
height = 2,
block_ids = Orphans2
},
assert_forks_json_equal([ExpectedFork1, ExpectedFork2]),
assert_forks_json_equal([ExpectedFork2, ExpectedFork1]),

Orphans3 = [<<"b">>, <<"c">>, <<"d">>],
timer:sleep(5),
Expand All @@ -137,7 +133,7 @@ test_get_recent_forks() ->
height = 2,
block_ids = Orphans3
},
assert_forks_json_equal([ExpectedFork1, ExpectedFork2, ExpectedFork3]),
assert_forks_json_equal([ExpectedFork3, ExpectedFork2, ExpectedFork1]),

Orphans4 = [<<"e">>, <<"f">>, <<"g">>],
timer:sleep(5),
Expand All @@ -147,20 +143,20 @@ test_get_recent_forks() ->
height = 3,
block_ids = Orphans4
},
assert_forks_json_equal([ExpectedFork1, ExpectedFork2, ExpectedFork3, ExpectedFork4]),
assert_forks_json_equal([ExpectedFork4, ExpectedFork3, ExpectedFork2, ExpectedFork1]),

%% Same fork seen again - not sure this is possible, but since we're just tracking
%% forks based on when they occur, it should be handled.
timer:sleep(5),
ar_chain_stats:log_fork(Orphans3, ForkRootB1),
assert_forks_json_equal(
[ExpectedFork1, ExpectedFork2, ExpectedFork3, ExpectedFork4, ExpectedFork3]),
[ExpectedFork3, ExpectedFork4, ExpectedFork3, ExpectedFork2, ExpectedFork1]),

%% If the fork is empty, ignore it.
timer:sleep(5),
ar_chain_stats:log_fork([], ForkRootB2),
assert_forks_json_equal(
[ExpectedFork1, ExpectedFork2, ExpectedFork3, ExpectedFork4, ExpectedFork3]),
[ExpectedFork3, ExpectedFork4, ExpectedFork3, ExpectedFork2, ExpectedFork1]),
ok.

test_recent_forks() ->
Expand Down Expand Up @@ -243,7 +239,7 @@ test_recent_forks() ->
ar_test_node:disconnect_from(peer1),
ar_test_node:disconnect_from(peer2),

assert_forks_json_equal([Fork1, Fork2], get_recent(ar_test_node:peer_ip(peer1), forks)),
assert_forks_json_equal([Fork2, Fork1], get_recent(ar_test_node:peer_ip(peer1), forks)),
ok.

assert_forks_json_equal(ExpectedForks) ->
Expand Down

0 comments on commit 9e18942

Please sign in to comment.