swarm/pot: pot.remove fixed (#18431)

* swarm/pot: refactored pot.remove(), updated comments

* swarm/pot: comments updated

swarm/pot/address.go

@@ -162,7 +162,6 @@ func ToBytes(v Val) []byte {
 }
 
 // DefaultPof returns a proximity order comparison operator function
-// where all
 func DefaultPof(max int) func(one, other Val, pos int) (int, bool) {
 	return func(one, other Val, pos int) (int, bool) {
 		po, eq := proximityOrder(ToBytes(one), ToBytes(other), pos)
@@ -174,6 +173,9 @@ func DefaultPof(max int) func(one, other Val, pos int) (int, bool) {
 	}
 }
 
+// proximityOrder returns two parameters:
+// 1. relative proximity order of the arguments one & other;
+// 2. boolean indicating whether the full match occurred (one == other).
 func proximityOrder(one, other []byte, pos int) (int, bool) {
 	for i := pos / 8; i < len(one); i++ {
 		if one[i] == other[i] {

swarm/pot/pot.go

@@ -144,13 +144,10 @@ func add(t *Pot, val Val, pof Pof) (*Pot, int, bool) {
 	return r, po, found
 }
 
-// Remove called on (v) deletes v from the Pot and returns
+// Remove deletes element v from the Pot t and returns three parameters:
-// the proximity order of v and a boolean value indicating
+// 1. new Pot that contains all the elements of t minus the element v;
-// if the value was found
+// 2. proximity order of the removed element v;
-// Remove called on (t, v) returns a new Pot that contains all the elements of t
+// 3. boolean indicating whether the item was found.
-// minus the value v, using the applicative remove
-// the second return value is the proximity order of the inserted element
-// the third is boolean indicating if the item was found
 func Remove(t *Pot, v Val, pof Pof) (*Pot, int, bool) {
 	return remove(t, v, pof)
 }
@@ -161,10 +158,7 @@ func remove(t *Pot, val Val, pof Pof) (r *Pot, po int, found bool) {
 	if found {
 		size--
 		if size == 0 {
-			r = &Pot{
+			return &Pot{}, po, true
-				po: t.po,
-			}
-			return r, po, true
 		}
 		i := len(t.bins) - 1
 		last := t.bins[i]
@@ -201,7 +195,7 @@ func remove(t *Pot, val Val, pof Pof) (r *Pot, po int, found bool) {
 	}
 	bins = append(bins, t.bins[j:]...)
 	r = &Pot{
-		pin:  val,
+		pin:  t.pin,
 		size: size,
 		po:   t.po,
 		bins: bins,

swarm/pot/pot_test.go

@@ -82,6 +82,72 @@ func testAdd(t *Pot, pof Pof, j int, values ...string) (_ *Pot, n int, f bool) {
 	return t, n, f
 }
 
+// removing non-existing element from pot
+func TestPotRemoveNonExisting(t *testing.T) {
+	pof := DefaultPof(8)
+	n := NewPot(newTestAddr("00111100", 0), 0)
+	n, _, _ = Remove(n, newTestAddr("00000101", 0), pof)
+	exp := "00111100"
+	got := Label(n.Pin())
+	if got[:8] != exp {
+		t.Fatalf("incorrect pinned value. Expected %v, got %v", exp, got[:8])
+	}
+}
+
+// this test creates hierarchical pot tree, and therefore any child node will have
+// child_po = parent_po + 1.
+// then removes a node from the middle of the tree.
+func TestPotRemoveSameBin(t *testing.T) {
+	pof := DefaultPof(8)
+	n := NewPot(newTestAddr("11111111", 0), 0)
+	n, _, _ = testAdd(n, pof, 1, "00000000", "01000000", "01100000", "01110000", "01111000")
+	n, _, _ = Remove(n, newTestAddr("01110000", 0), pof)
+	inds, po := indexes(n)
+	goti := n.Size()
+	expi := 5
+	if goti != expi {
+		t.Fatalf("incorrect number of elements in Pot. Expected %v, got %v", expi, goti)
+	}
+	inds, po = indexes(n)
+	got := fmt.Sprintf("%v", inds)
+	exp := "[5 3 2 1 0]"
+	if got != exp {
+		t.Fatalf("incorrect indexes in iteration over Pot. Expected %v, got %v", exp, got)
+	}
+	got = fmt.Sprintf("%v", po)
+	exp = "[3 2 1 0 0]"
+	if got != exp {
+		t.Fatalf("incorrect po-s in iteration over Pot. Expected %v, got %v", exp, got)
+	}
+}
+
+// this test creates a flat pot tree (all the elements are leafs of one root),
+// and therefore they all have the same po.
+// then removes an arbitrary element from the pot.
+func TestPotRemoveDifferentBins(t *testing.T) {
+	pof := DefaultPof(8)
+	n := NewPot(newTestAddr("11111111", 0), 0)
+	n, _, _ = testAdd(n, pof, 1, "00000000", "10000000", "11000000", "11100000", "11110000")
+	n, _, _ = Remove(n, newTestAddr("11100000", 0), pof)
+	inds, po := indexes(n)
+	goti := n.Size()
+	expi := 5
+	if goti != expi {
+		t.Fatalf("incorrect number of elements in Pot. Expected %v, got %v", expi, goti)
+	}
+	inds, po = indexes(n)
+	got := fmt.Sprintf("%v", inds)
+	exp := "[1 2 3 5 0]"
+	if got != exp {
+		t.Fatalf("incorrect indexes in iteration over Pot. Expected %v, got %v", exp, got)
+	}
+	got = fmt.Sprintf("%v", po)
+	exp = "[0 1 2 4 0]"
+	if got != exp {
+		t.Fatalf("incorrect po-s in iteration over Pot. Expected %v, got %v", exp, got)
+	}
+}
+
 func TestPotAdd(t *testing.T) {
 	pof := DefaultPof(8)
 	n := NewPot(newTestAddr("00111100", 0), 0)
@@ -135,25 +201,29 @@ func TestPotRemove(t *testing.T) {
 		t.Fatalf("incorrect number of elements in Pot. Expected %v, got %v", expi, goti)
 	}
 	inds, po := indexes(n)
-	got = fmt.Sprintf("%v", inds)
-	exp = "[2 4 0]"
-	if got != exp {
-		t.Fatalf("incorrect indexes in iteration over Pot. Expected %v, got %v", exp, got)
-	}
 	got = fmt.Sprintf("%v", po)
 	exp = "[1 3 0]"
 	if got != exp {
 		t.Fatalf("incorrect po-s in iteration over Pot. Expected %v, got %v", exp, got)
 	}
-	// remove again
+	got = fmt.Sprintf("%v", inds)
-	n, _, _ = Remove(n, newTestAddr("00111100", 0), pof)
+	exp = "[2 4 1]"
+	if got != exp {
+		t.Fatalf("incorrect indexes in iteration over Pot. Expected %v, got %v", exp, got)
+	}
+	n, _, _ = Remove(n, newTestAddr("00111100", 0), pof) // remove again same element
+	inds, _ = indexes(n)
+	got = fmt.Sprintf("%v", inds)
+	if got != exp {
+		t.Fatalf("incorrect indexes in iteration over Pot. Expected %v, got %v", exp, got)
+	}
+	n, _, _ = Remove(n, newTestAddr("00000000", 0), pof) // remove the first element
 	inds, _ = indexes(n)
 	got = fmt.Sprintf("%v", inds)
 	exp = "[2 4]"
 	if got != exp {
 		t.Fatalf("incorrect indexes in iteration over Pot. Expected %v, got %v", exp, got)
 	}
-
 }
 
 func TestPotSwap(t *testing.T) {