handle more packets, fix major flaw in diffing algo (again) and add extra tests

version0/crdt/text_document.hpp

@@ -214,7 +214,6 @@ struct TextDocument {
 		if (!differ && list_start == state.list.size() && text_start == text.size()) {
 			return {};
 		}
-
 		//std::cout << "list.size: " << state.list.size() << "(" << getText().size() << ")" << " text.size: " << text.size() << "\n";
 		//std::cout << "list_start: " << list_start << " text_start: " << text_start << "\n";
 
@@ -224,7 +223,9 @@ struct TextDocument {
 		//for (; list_end > 0 && text_end > 0 && list_end >= list_start && text_end >= text_start;) {
 		//while (list_end >= list_start && text_end >= text_start) {
 		size_t list_end_counted = 0;
-		while (list_start_counted - list_end_counted > state.doc_size && text_end >= text_start) {
+		differ = false; // var reuse
+		//while (list_start_counted - list_end_counted > state.doc_size && text_end >= text_start) {
+		while (state.doc_size - list_start_counted > list_end_counted && text_end >= text_start) {
 			// jump over tombstones
 			if (!state.list[list_end-1].value.has_value()) {
 				list_end--;
@@ -232,6 +233,7 @@ struct TextDocument {
 			}
 
 			if (state.list[list_end-1].value.value() != text[text_end-1]) {
+				differ = true;
 				break;
 			}
 
@@ -240,20 +242,29 @@ struct TextDocument {
 			list_end_counted++;
 		}
 
+		if (!differ && text_start == text_end+1) {
+			// we ran into eachother without seeing the different char
+			// TODO: do we need to increment list_end? text_end?
+			list_end++;
+		}
+
 		//std::cout << "list_end: " << list_end << " text_end: " << text_end << "\n";
+		//std::cout << "substring before: " << text.substr(text_start, text.size() - state.doc_size) << "\n";
 
 		std::vector<Op> ops;
 
 		// 1. clear range (del all list_start - list_end)
 		if (list_start <= list_end && list_start < state.list.size()) {
+			//list_end += list_start == list_end;
 			ops = delRange(
 				state.list[list_start].id,
-				(list_start == list_end ? list_end+1 : list_end) < state.list.size() ? std::make_optional(state.list[list_end].id) : std::nullopt
+				list_end < state.list.size() ? std::make_optional(state.list[list_end].id) : std::nullopt
 			);
 			//std::cout << "deleted: " << ops.size() << "\n";
 		}
 
 		//std::cout << "text between: " << getText() << "\n";
+		//std::cout << "substring between: " << text.substr(text_start, text.size() - state.doc_size) << "\n";
 
 		// 2. add range (add all text_start - text_end)
 		if (state.doc_size < text.size()) {
@@ -266,7 +277,6 @@ struct TextDocument {
 			ops.insert(ops.end(), tmp_add_ops.begin(), tmp_add_ops.end());
 		}
 
-		//assert(false && "implement me");
 		return ops;
 	}
 };

version0/test2.cpp

@@ -418,8 +418,9 @@ void testBugDoubleDel(void) {
 
 	{
 		std::string_view new_text{"a"};
-		doc.merge(new_text);
+		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
@@ -445,20 +446,23 @@ void testBugSameDel(void) {
 
 	{
 		std::string_view new_text{"a"};
-		doc.merge(new_text);
+		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
 		std::string_view new_text{"aa"};
 		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
 		std::string_view new_text{"a"};
 		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 }
 
@@ -468,32 +472,122 @@ void testBugSameDel2(void) {
 
 	{
 		std::string_view new_text{"a"};
-		doc.merge(new_text);
+		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
 		std::string_view new_text{"aa"};
 		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
 		std::string_view new_text{"aaa"};
 		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
 		std::string_view new_text{"aa"};
 		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
 	}
 
 	{
 		std::string_view new_text{"a"};
 		const auto ops = doc.merge(new_text);
 		assert(doc.getText() == new_text);
+		assert(ops.size() == 1);
+	}
+}
+
+void testMulti1(void) {
+	Doc docA;
+	docA.local_agent = 'A';
+
+	Doc docB;
+	docB.local_agent = 'B';
+
+	// state A
+	{
+		std::string_view new_text{"iiiiiii"};
+		const auto ops = docA.merge(new_text);
+		assert(docA.getText() == new_text);
+
+		assert(docB.apply(ops));
+
+		assert(docB.getText() == new_text);
+		assert(docB.state.doc_size == docA.state.doc_size);
+		assert(docB.state.list.size() == docA.state.list.size());
+	}
+
+	// now B inserts b
+	{
+		std::string_view new_text{"iiibiiii"};
+		const auto ops = docB.merge(new_text);
+		assert(docB.getText() == new_text);
+		assert(ops.size() == 1); // 1 new inserted char, nothing to delete
+
+		assert(docA.apply(ops));
+
+		assert(docA.getText() == new_text);
+	}
+}
+
+void testPaste1(void) {
+	Doc docA;
+	docA.local_agent = 'A';
+
+	{
+		std::string_view new_text{"iiiiiii"};
+		const auto ops = docA.merge(new_text);
+		assert(ops.size() == 7);
+		assert(docA.getText() == new_text);
+	}
+
+	{
+		std::string_view new_text{"iiiiiii\n"};
+		const auto ops = docA.merge(new_text);
+		assert(ops.size() == 1);
+		assert(docA.getText() == new_text);
+	}
+
+	{
+		std::string_view new_text{"iiiiiii\niiiiiii"};
+		const auto ops = docA.merge(new_text);
+		assert(ops.size() == 7);
+		assert(docA.getText() == new_text);
+	}
+}
+
+void testPaste2(void) {
+	Doc docA;
+	docA.local_agent = 'A';
+
+	{
+		std::string_view new_text{"aiiiiib"};
+		const auto ops = docA.merge(new_text);
+		assert(ops.size() == 7);
+		assert(docA.getText() == new_text);
+	}
+
+	{
+		std::string_view new_text{"aiiiiib\n"};
+		const auto ops = docA.merge(new_text);
+		assert(ops.size() == 1);
+		assert(docA.getText() == new_text);
+	}
+
+	{
+		std::string_view new_text{"aiiiiib\naiiiiib"};
+		const auto ops = docA.merge(new_text);
+		assert(ops.size() == 7);
+		assert(docA.getText() == new_text);
 	}
 }
 
@@ -580,6 +674,27 @@ int main(void) {
 		testBugSameDel2();
 	}
 
+	std::cout << std::string(40, '=') << "\n";
+
+	{
+		std::cout << "testMulti1:\n";
+		testMulti1();
+	}
+
+	std::cout << std::string(40, '=') << "\n";
+
+	{
+		std::cout << "testPaste1:\n";
+		testPaste1();
+	}
+
+	std::cout << std::string(40, '=') << "\n";
+
+	{
+		std::cout << "testPaste2:\n";
+		testPaste2();
+	}
+
 	return 0;
 }
 

vim_research/test2.cpp

@@ -1,4 +1,3 @@
-#include "toxcore/tox.h"
 #include <crdt/text_document.hpp>
 #include <nlohmann/json.hpp>
 
@@ -286,6 +285,26 @@ echo 'setup done'
 
 } // namespace vim
 
+// visibility hack
+struct RequestCommands {
+	Agent agent;
+	uint64_t after_seq{0};
+	uint64_t until_seq{0};
+};
+NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(RequestCommands,
+	agent,
+	after_seq,
+	until_seq
+)
+
+// hash for unordered_set
+template<>
+struct std::hash<std::pair<uint32_t, Agent>> {
+	std::size_t operator()(std::pair<uint32_t, Agent> const& s) const noexcept {
+		return std::hash<uint32_t>{}(s.first) << 3 ^ std::hash<Agent>{}(s.second);
+	}
+};
+
 struct SharedContext {
 	std::atomic_bool should_quit {false};
 
@@ -310,6 +329,12 @@ struct SharedContext {
 	std::unordered_set<ToxPubKey> should_gossip_remote; // list of ids we have new seq for (only modified by tox thread)
 	std::unordered_map<ToxPubKey, uint64_t> heard_gossip; // seq frontiers we have heard about
 
+	// peer ids that requested the last known seq for agent
+	std::unordered_set<std::pair<uint32_t, Agent>> requested_frontier;
+
+	// peer ids that requested a command (range)
+	std::vector<std::pair<uint32_t, RequestCommands>> requested_commands;
+
 	Tox* tox {nullptr};
 	bool tox_dht_online {false};
 	bool tox_group_online {false};
@@ -350,16 +375,19 @@ namespace pkg {
 
 	using Command = ::Command;
 
-	// request every command for agent after seq (inclusive)
-	struct RequestCommands {
-		Agent agent;
-		uint64_t seq{0};
-	};
+	// request every command for agent after_seq - until_seq (inclusive)
+	//struct RequestCommands {
+		//Agent agent;
+		//uint64_t after_seq{0};
+		//uint64_t until_seq{0};
+	//};
+	using RequestCommands = ::RequestCommands;
 
-	NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(RequestCommands,
-		agent,
-		seq
-	)
+	//NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(RequestCommands,
+		//agent,
+		//after_seq,
+		//until_seq
+	//)
 
 } // namespace pkg
 
@@ -560,8 +588,10 @@ void toxThread(SharedContext* ctx) {
 						// prepend pkgid
 						data.emplace(data.begin(), static_cast<uint8_t>(pkg::PKGID::FRONTIER));
 
-						if (!tox_group_send_custom_packet(ctx->tox, 0, true, data.data(), data.size(), nullptr)) {
-							std::cerr << "failed to send gossip packet of local agent\n";
+						Tox_Err_Group_Send_Custom_Packet send_err{TOX_ERR_GROUP_SEND_CUSTOM_PACKET_OK};
+						if (!tox_group_send_custom_packet(ctx->tox, 0, true, data.data(), data.size(), &send_err)) {
+							std::cerr << "failed to send gossip packet of local agent" << send_err << "\n";
+							assert(send_err != TOX_ERR_GROUP_SEND_CUSTOM_PACKET_TOO_LONG);
 							// TODO: set should_gossip_local back to true?
 						} else {
 							std::cout << "sent gossip of local agent\n";
@@ -572,8 +602,10 @@ void toxThread(SharedContext* ctx) {
 						// prepend pkgid
 						data.emplace(data.begin(), static_cast<uint8_t>(pkg::PKGID::COMMAND));
 
-						if (!tox_group_send_custom_packet(ctx->tox, 0, true, data.data(), data.size(), nullptr)) {
+						Tox_Err_Group_Send_Custom_Packet send_err{TOX_ERR_GROUP_SEND_CUSTOM_PACKET_OK};
+						if (!tox_group_send_custom_packet(ctx->tox, 0, true, data.data(), data.size(), &send_err)) {
 							std::cerr << "failed to send command packet of local agent\n";
+							assert(send_err != TOX_ERR_GROUP_SEND_CUSTOM_PACKET_TOO_LONG);
 						} else {
 							std::cout << "sent command of local agent\n";
 						}
@@ -963,7 +995,7 @@ static void self_connection_status_cb(Tox*, TOX_CONNECTION connection_status, vo
 	std::cout << "self_connection_status_cb " << connection_status << "\n";
 }
 
-static void handle_pkg(SharedContext& ctx, const uint8_t* data, size_t length) {
+static void handle_pkg(SharedContext& ctx, const uint8_t* data, size_t length, uint32_t peer_id) {
 	if (length < 2) {
 		std::cerr << "got too short pkg " << length << "\n";
 		return;
@@ -990,6 +1022,7 @@ static void handle_pkg(SharedContext& ctx, const uint8_t* data, size_t length) {
 		}
 		case pkg::PKGID::REQUEST_FRONTIER: {
 			pkg::RequestFrontier pkg = p_j;
+			ctx.requested_frontier.emplace(peer_id, pkg.agent);
 			break;
 		}
 		case pkg::PKGID::COMMAND: {
@@ -1005,6 +1038,9 @@ static void handle_pkg(SharedContext& ctx, const uint8_t* data, size_t length) {
 		}
 		case pkg::PKGID::REQUEST_COMMANDS: {
 			pkg::RequestCommands pkg = p_j;
+			// TODO: this can lead to double requests
+			// TODO: maybe settle for single seq requests for now?, since they are indivitual packets anyway
+			ctx.requested_commands.push_back(std::make_pair(peer_id, pkg));
 			break;
 		}
 		default:
@@ -1016,13 +1052,13 @@ static void handle_pkg(SharedContext& ctx, const uint8_t* data, size_t length) {
 static void group_custom_packet_cb(Tox*, uint32_t group_number, uint32_t peer_id, const uint8_t* data, size_t length, void* user_data) {
 	std::cout << "group_custom_packet_cb\n";
 	SharedContext& ctx = *static_cast<SharedContext*>(user_data);
-	handle_pkg(ctx, data, length);
+	handle_pkg(ctx, data, length, peer_id);
 }
 
 static void group_custom_private_packet_cb(Tox*, uint32_t group_number, uint32_t peer_id, const uint8_t* data, size_t length, void* user_data) {
 	std::cout << "group_custom_private_packet_cb\n";
 	SharedContext& ctx = *static_cast<SharedContext*>(user_data);
-	handle_pkg(ctx, data, length);
+	handle_pkg(ctx, data, length, peer_id);
 }
 
 }