refactor: reuse protobuf Vertex definition (#164)

Co-authored-by: Oak <[email protected]>

packages/blueprints/src/AddWinsSet/index.ts

@@ -44,9 +44,12 @@ export class AddWinsSet<T> implements CRO {
 
 	// in this case is an array of length 2 and there are only two possible operations
 	resolveConflicts(vertices: Vertex[]): ResolveConflictsType {
+		// Both must have operations, if not return no-op
 		if (
-			vertices[0].operation.type !== vertices[1].operation.type &&
-			vertices[0].operation.value === vertices[1].operation.value
+			vertices[0].operation &&
+			vertices[1].operation &&
+			vertices[0].operation?.type !== vertices[1].operation?.type &&
+			vertices[0].operation?.value === vertices[1].operation?.value
 		) {
 			return vertices[0].operation.type === "add"
 				? { action: ActionType.DropRight }

packages/object/src/hashgraph/index.ts

@@ -1,13 +1,15 @@
 import * as crypto from "node:crypto";
 import { linearizeMultiple } from "../linearize/multipleSemantics.js";
 import { linearizePair } from "../linearize/pairSemantics.js";
+import { Vertex_Operation as Operation, Vertex } from "../proto/object_pb.js";
 import { BitSet } from "./bitset.js";
 
+// Reexporting the Vertex and Operation types from the protobuf file
+export { Vertex, Operation };
+
 export type Hash = string;
-// biome-ignore lint: value can't be unknown because of protobuf
-export type Operation = { type: string; value: any | null };
 
-enum OperationType {
+export enum OperationType {
 	NOP = "-1",
 }
 
@@ -30,15 +32,6 @@ export type ResolveConflictsType = {
 	vertices?: Hash[];
 };
 
-export interface Vertex {
-	hash: Hash;
-	nodeId: string;
-	// internal Operation type enum converted to number
-	// -1 for NOP
-	operation: Operation;
-	dependencies: Hash[];
-}
-
 export class HashGraph {
 	nodeId: string;
 	resolveConflicts: (vertices: Vertex[]) => ResolveConflictsType;
@@ -89,10 +82,11 @@ export class HashGraph {
 	addToFrontier(operation: Operation): Vertex {
 		const deps = this.getFrontier();
 		const hash = computeHash(this.nodeId, operation, deps);
+
 		const vertex: Vertex = {
 			hash,
 			nodeId: this.nodeId,
-			operation,
+			operation: operation ?? { type: OperationType.NOP },
 			dependencies: deps,
 		};
 

packages/object/src/index.ts

@@ -1,6 +1,5 @@
 import * as crypto from "node:crypto";
 import {
-	type ActionType,
 	HashGraph,
 	type Operation,
 	type ResolveConflictsType,
@@ -91,10 +90,7 @@ export class TopologyObject implements ITopologyObject {
 		const serializedVertex = ObjectPb.Vertex.create({
 			hash: vertex.hash,
 			nodeId: vertex.nodeId,
-			operation: {
-				type: vertex.operation.type,
-				value: vertex.operation.value,
-			},
+			operation: vertex.operation,
 			dependencies: vertex.dependencies,
 		});
 		this.vertices.push(serializedVertex);
@@ -103,6 +99,11 @@ export class TopologyObject implements ITopologyObject {
 
 	merge(vertices: Vertex[]) {
 		for (const vertex of vertices) {
+			// Check to avoid manually crafted `undefined` operations
+			if (!vertex.operation) {
+				continue;
+			}
+
 			this.hashGraph.addVertex(
 				vertex.operation,
 				vertex.dependencies,
@@ -111,17 +112,7 @@ export class TopologyObject implements ITopologyObject {
 		}
 
 		const operations = this.hashGraph.linearizeOperations();
-		this.vertices = this.hashGraph.getAllVertices().map((vertex) => {
-			return {
-				hash: vertex.hash,
-				nodeId: vertex.nodeId,
-				operation: {
-					type: vertex.operation.type,
-					value: vertex.operation.value,
-				},
-				dependencies: vertex.dependencies,
-			};
-		});
+		this.vertices = this.hashGraph.getAllVertices();
 
 		(this.cro as CRO).mergeCallback(operations);
 		this._notify("merge", this.vertices);

packages/object/tests/hashgraph.test.ts

@@ -1,7 +1,7 @@
 import { beforeEach, describe, expect, test } from "vitest";
 import { AddWinsSet } from "../../blueprints/src/AddWinsSet/index.js";
 import { PseudoRandomWinsSet } from "../../blueprints/src/PseudoRandomWinsSet/index.js";
-import { TopologyObject } from "../src/index.js";
+import { type Operation, OperationType, TopologyObject } from "../src/index.js";
 
 describe("HashGraph for AddWinSet tests", () => {
 	let obj1: TopologyObject;
@@ -349,3 +349,41 @@ describe("HashGraph for PseudoRandomWinsSet tests", () => {
 		expect(linearOps).toEqual([{ type: "add", value: 3 }]);
 	});
 });
+
+describe("HashGraph for undefined operations tests", () => {
+	let obj1: TopologyObject;
+	let obj2: TopologyObject;
+
+	beforeEach(async () => {
+		obj1 = new TopologyObject("peer1", new AddWinsSet<number>());
+		obj2 = new TopologyObject("peer2", new AddWinsSet<number>());
+	});
+
+	test("Test: merge should skip undefined operations", () => {
+		const cro1 = obj1.cro as AddWinsSet<number>;
+		const cro2 = obj2.cro as AddWinsSet<number>;
+
+		cro1.add(1);
+		cro2.add(2);
+
+		// Set one of the vertice from cro1 to have undefined operation
+		obj1.hashGraph.getAllVertices()[1].operation = undefined;
+
+		obj2.merge(obj1.hashGraph.getAllVertices());
+
+		const linearOps = obj2.hashGraph.linearizeOperations();
+		// Should only have one, since we skipped the undefined operations
+		expect(linearOps).toEqual([{ type: "add", value: 2 }]);
+	});
+
+	test("Test: addToFrontier with undefined operation return Vertex with NoOp operation", () => {
+		// Forcefully pass an undefined value
+		const createdVertex = obj1.hashGraph.addToFrontier(
+			undefined as unknown as Operation,
+		);
+
+		expect(createdVertex.operation).toEqual({
+			type: OperationType.NOP,
+		} as Operation);
+	});
+});