fix: fix some data corruption during packing

1. When writing entropy to the overlap zone between storage modules sometimes entropy could be written over chunk data. Fix is not to write to neighboring storage modules and instead waste a bit of entropy. Wasted entropy is about 0.07% of all entropy generated per partition. 2. When repacking in place make sure we wait for entropy to be written whenever moving to a new slice index. This is primarily an issue during tests where there are only 3 chunks per sector, but could hypothetically be an issue in production. 3. Remove all code related to sub-chunk iteration since we don't need it and it may impact performance.
ArweaveTeam · Jan 23, 2025 · 9365c3e · 9365c3e
1 parent 64356f3
commit 9365c3e
Show file tree

Hide file tree

Showing 10 changed files with 235 additions and 165 deletions.
diff --git a/apps/arweave/e2e/ar_e2e.erl b/apps/arweave/e2e/ar_e2e.erl
@@ -6,7 +6,7 @@
 -export([delayed_print/2, packing_type_to_packing/2,
 	start_source_node/3, source_node_storage_modules/3, max_chunk_offset/1,
 	assert_block/2, assert_syncs_range/3, assert_does_not_sync_range/3, 
-	assert_chunks/3, assert_no_chunks/2, assert_partition_size/4, assert_empty_partition/3]).
+	assert_chunks/3, assert_no_chunks/2, assert_partition_size/3, assert_empty_partition/3]).
 
 -include_lib("arweave/include/ar.hrl").
 -include_lib("arweave/include/ar_config.hrl").
@@ -85,8 +85,8 @@ start_source_node(Node, unpacked, _WalletFixture) ->
 		auto_join = true
 	}, true),
 
-	ar_e2e:assert_partition_size(Node, 0, unpacked, ?PARTITION_SIZE),
-	ar_e2e:assert_partition_size(Node, 1, unpacked, ?PARTITION_SIZE),
+	ar_e2e:assert_partition_size(Node, 0, unpacked),
+	ar_e2e:assert_partition_size(Node, 1, unpacked),
 
 	ar_e2e:assert_syncs_range(Node, ?PARTITION_SIZE, 2*?PARTITION_SIZE),
 	ar_e2e:assert_chunks(Node, unpacked, Chunks),
@@ -134,10 +134,12 @@ start_source_node(Node, PackingType, WalletFixture) ->
 
 	SourcePacking = ar_e2e:packing_type_to_packing(PackingType, RewardAddr),
 
-	ar_e2e:assert_partition_size(Node, 0, SourcePacking, ?PARTITION_SIZE),
-	ar_e2e:assert_partition_size(Node, 1, SourcePacking, ?PARTITION_SIZE),
+	ar_e2e:assert_partition_size(Node, 0, SourcePacking),
+	ar_e2e:assert_partition_size(Node, 1, SourcePacking),
 
-	ar_e2e:assert_syncs_range(Node, ?PARTITION_SIZE, 2*?PARTITION_SIZE),
+	ar_e2e:assert_syncs_range(Node, 
+		?PARTITION_SIZE,
+		2*?PARTITION_SIZE + ar_storage_module:get_overlap(SourcePacking)),
 	ar_e2e:assert_chunks(Node, SourcePacking, Chunks),
 
 	?LOG_INFO("Source node ~p assertions passed.", [Node]),
@@ -238,7 +240,10 @@ assert_does_not_sync_range(Node, StartOffset, EndOffset) ->
 			"~s synced range when it should not have: ~p - ~p", 
 			[Node, StartOffset, EndOffset]))).
 
-assert_partition_size(Node, PartitionNumber, Packing, Size) ->
+assert_partition_size(Node, PartitionNumber, Packing) ->
+	Size = ?PARTITION_SIZE,
+	?LOG_INFO("~p: Asserting partition ~p,~p is size ~p",
+		[Node, PartitionNumber, ar_serialize:encode_packing(Packing, true), Size]),
 	?assert(
 		ar_util:do_until(
 			fun() -> 
@@ -337,17 +342,18 @@ assert_chunk(Node, Packing, Block, EndOffset, ChunkSize) ->
 	{ok, ExpectedPackedChunk} = ar_e2e:load_chunk_fixture(Packing, EndOffset),
 	?assertEqual(ExpectedPackedChunk, Chunk,
 		iolist_to_binary(io_lib:format(
-			"Chunk at offset ~p, size ~p does not match previously packed chunk",
-			[EndOffset, ChunkSize]))),
+			"~p: Chunk at offset ~p, size ~p does not match previously packed chunk",
+			[Node, EndOffset, ChunkSize]))),
 
 	{ok, UnpackedChunk} = ar_packing_server:unpack(
 		Packing, EndOffset, Block#block.tx_root, Chunk, ?DATA_CHUNK_SIZE),
-	UnpaddedChunk = ar_packing_server:unpad_chunk(Packing, UnpackedChunk, ChunkSize, byte_size(Chunk)),
+	UnpaddedChunk = ar_packing_server:unpad_chunk(
+		Packing, UnpackedChunk, ChunkSize, byte_size(Chunk)),
 	ExpectedUnpackedChunk = ar_test_node:get_genesis_chunk(EndOffset),
 	?assertEqual(ExpectedUnpackedChunk, UnpaddedChunk,
 		iolist_to_binary(io_lib:format(
-			"Chunk at offset ~p, size ~p does not match unpacked chunk",
-			[EndOffset, ChunkSize]))).
+			"~p: Chunk at offset ~p, size ~p does not match unpacked chunk",
+			[Node, EndOffset, ChunkSize]))).
 
 assert_no_chunks(Node, Chunks) ->
 	lists:foreach(fun({_Block, EndOffset, _ChunkSize}) ->

diff --git a/apps/arweave/e2e/ar_repack_in_place_mine_tests.erl b/apps/arweave/e2e/ar_repack_in_place_mine_tests.erl
@@ -15,13 +15,9 @@
 repack_in_place_mine_test_() ->
 	Timeout = ?REPACK_IN_PLACE_MINE_TEST_TIMEOUT,
 	[
-		% XXX {timeout, Timeout, {with, {unpacked, replica_2_9}, [fun test_repack_in_place_mine/1]}},
-		% XXX {timeout, Timeout, {with, {unpacked, spora_2_6}, [fun test_repack_in_place_mine/1]}},
-		% XXX {timeout, Timeout, {with, {unpacked, composite_1}, [fun test_repack_in_place_mine/1]}},
+		% {timeout, Timeout, {with, {unpacked, replica_2_9}, [fun test_repack_in_place_mine/1]}},
 		{timeout, Timeout, {with, {spora_2_6, replica_2_9}, [fun test_repack_in_place_mine/1]}},
-		% % % % % XXX {timeout, Timeout, {with, {spora_2_6, unpacked}, [fun test_repack_in_place_mine/1]}},
 		{timeout, Timeout, {with, {composite_1, replica_2_9}, [fun test_repack_in_place_mine/1]}}
-		% % % % % XXX {timeout, Timeout, {with, {composite_1, unpacked}, [fun test_repack_in_place_mine/1]}}
 	].
 
 %% --------------------------------------------------------------------------------------------
@@ -57,8 +53,8 @@ test_repack_in_place_mine({FromPackingType, ToPackingType}) ->
 	}),
 	ar_test_node:restart(RepackerNode),
 
-	ar_e2e:assert_partition_size(RepackerNode, 0, ToPacking, ?PARTITION_SIZE),
-	ar_e2e:assert_partition_size(RepackerNode, 1, ToPacking, ?PARTITION_SIZE),
+	ar_e2e:assert_partition_size(RepackerNode, 0, ToPacking),
+	ar_e2e:assert_partition_size(RepackerNode, 1, ToPacking),
 
 	ar_test_node:stop(RepackerNode),
 

diff --git a/apps/arweave/e2e/ar_repack_mine_tests.erl b/apps/arweave/e2e/ar_repack_mine_tests.erl
@@ -58,14 +58,16 @@ test_repack_mine({FromPackingType, ToPackingType}) ->
 	}),
 	ar_test_node:restart(RepackerNode),
 
-	ar_e2e:assert_partition_size(RepackerNode, 1, ToPacking, ?PARTITION_SIZE),
+	ar_e2e:assert_partition_size(RepackerNode, 1, ToPacking),
 
 	ar_test_node:update_config(RepackerNode, Config#config{
 		storage_modules = StorageModules,
 		mining_addr = AddrB
 	}),
 	ar_test_node:restart(RepackerNode),
-	ar_e2e:assert_syncs_range(RepackerNode, ?PARTITION_SIZE, 2*?PARTITION_SIZE),
+	ar_e2e:assert_syncs_range(RepackerNode,
+		?PARTITION_SIZE,
+		2*?PARTITION_SIZE + ar_storage_module:get_overlap(ToPacking)),
 
 	ar_e2e:assert_chunks(RepackerNode, ToPacking, Chunks),
 

diff --git a/apps/arweave/e2e/ar_sync_pack_mine_tests.erl b/apps/arweave/e2e/ar_sync_pack_mine_tests.erl
@@ -86,14 +86,22 @@ test_sync_pack_mine({{Blocks, Chunks, SourcePackingType}, SinkPackingType}) ->
 	SinkNode = peer2,
 
 	SinkPacking = start_sink_node(SinkNode, SourceNode, B0, SinkPackingType),
-	ar_e2e:assert_syncs_range(SinkNode, ?PARTITION_SIZE, 2*?PARTITION_SIZE),
+	ar_e2e:assert_syncs_range(
+		SinkNode,
+		?PARTITION_SIZE,
+		2*?PARTITION_SIZE + ar_storage_module:get_overlap(SinkPacking)),
 	ar_e2e:assert_chunks(SinkNode, SinkPacking, Chunks),
 
 	case SinkPackingType of
 		unpacked ->
 			ok;
 		_ ->
-			CurrentHeight = ar_test_node:remote_call(SinkNode, ar_node, get_height, []),
+			CurrentHeight = max(
+				ar_test_node:remote_call(SourceNode, ar_node, get_height, []),
+				ar_test_node:remote_call(SinkNode, ar_node, get_height, [])
+			),
+			ar_test_node:wait_until_height(SourceNode, CurrentHeight),
+			ar_test_node:wait_until_height(SinkNode, CurrentHeight),
 			ar_test_node:mine(SinkNode),
 
 			SinkBI = ar_test_node:wait_until_height(SinkNode, CurrentHeight + 1),
@@ -123,9 +131,12 @@ test_unpacked_and_packed_sync_pack_mine({{Blocks, Chunks, SourcePackingType}, Pa
 	SinkNode = peer2,
 
 	{SinkPacking, unpacked} = start_sink_node(SinkNode, SourceNode, B0, PackingType, unpacked),
-	ar_e2e:assert_syncs_range(SinkNode, ?PARTITION_SIZE, 2*?PARTITION_SIZE),
-	ar_e2e:assert_partition_size(SinkNode, 1, SinkPacking, ?PARTITION_SIZE),
-	ar_e2e:assert_partition_size(SinkNode, 1, unpacked, ?PARTITION_SIZE),
+	ar_e2e:assert_syncs_range(
+		SinkNode,
+		?PARTITION_SIZE,
+		2*?PARTITION_SIZE + ar_storage_module:get_overlap(SinkPacking)),
+	ar_e2e:assert_partition_size(SinkNode, 1, SinkPacking),
+	ar_e2e:assert_partition_size(SinkNode, 1, unpacked),
 
 	CurrentHeight = ar_test_node:remote_call(SinkNode, ar_node, get_height, []),
 	ar_test_node:mine(SinkNode),