PR feedback

data/evmrepo/get_tx.go

@@ -48,13 +48,13 @@ func (e *evmRepository) GetTx(ctx context.Context, id string) (*models.Tx, error
 	} else {
 		// If receipt is found, check the status
 		if receipt.Status == 1 {
-			logctx.Info(ctx, "Transaction %q succeeded", logger.String("txHash", txHash.String()))
+			logctx.Info(ctx, "Transaction succeeded", logger.String("txHash", txHash.String()))
 			return &models.Tx{
 				Status: models.TX_SUCCESS,
 				TxHash: receipt.TxHash.Hex(),
 			}, nil
 		} else {
-			logctx.Info(ctx, "Transaction %q failed", logger.String("txHash", txHash.String()))
+			logctx.Info(ctx, "Transaction failed", logger.String("txHash", txHash.String()))
 			return &models.Tx{
 				Status: models.TX_FAILURE,
 				TxHash: receipt.TxHash.Hex(),

data/redisrepo/get_pending_swaps.go

@@ -8,16 +8,16 @@ import (
 )
 
 // GetPendingSwaps returns all pending swaps that are waiting to be checked for completion
-func (r *redisRepository) GetPendingSwaps(ctx context.Context) ([]models.Pending, error) {
-	var pendingSwaps []models.Pending
+func (r *redisRepository) GetPendingSwaps(ctx context.Context) ([]models.SwapTx, error) {
+	var pendingSwaps []models.SwapTx
 
 	pendings, err := r.client.LRange(ctx, CreatePendingSwapTxsKey(), 0, -1).Result()
 	if err != nil {
 		return nil, fmt.Errorf("failed to get pending swaps: %s", err)
 	}
 
 	for _, pending := range pendings {
-		var p models.Pending
+		var p models.SwapTx
 		err := p.FromJson([]byte(pending))
 		if err != nil {
 			return nil, fmt.Errorf("failed to unmarshal pending: %s", err)

data/redisrepo/process_completed_swap_orders.go

@@ -46,7 +46,7 @@ func (r *redisRepository) ProcessCompletedSwapOrders(ctx context.Context, orders
 
 	// 2. Remove the swap
 	swapKey := CreateSwapKey(swapId)
-	transaction.Del(ctx, swapKey).Err()
+	transaction.Del(ctx, swapKey)
 
 	// --- END TRANSACTION ---
 	_, err := transaction.Exec(ctx)

data/redisrepo/store_new_pending_swap.go

@@ -11,7 +11,7 @@ import (
 )
 
 // StoreNewPendingSwap stores a new pending swap in order for its status (pending/complete) to be checked later
-func (r *redisRepository) StoreNewPendingSwap(ctx context.Context, p models.Pending) error {
+func (r *redisRepository) StoreNewPendingSwap(ctx context.Context, p models.SwapTx) error {
 	key := CreatePendingSwapTxsKey()
 
 	jsonData, err := json.Marshal(p)

data/redisrepo/store_new_pending_swap_test.go

@@ -19,11 +19,11 @@ func TestRedisRepo_StoreNewPendingSwap(t *testing.T) {
 			client: db,
 		}
 
-		pendingJson, _ := mocks.Pending.ToJson()
+		pendingJson, _ := mocks.SwapTx.ToJson()
 
 		mock.ExpectRPush(CreatePendingSwapTxsKey(), pendingJson).SetVal(1)
 
-		err := repo.StoreNewPendingSwap(ctx, mocks.Pending)
+		err := repo.StoreNewPendingSwap(ctx, mocks.SwapTx)
 
 		assert.NoError(t, err)
 	})
@@ -35,9 +35,9 @@ func TestRedisRepo_StoreNewPendingSwap(t *testing.T) {
 			client: db,
 		}
 
-		mock.ExpectRPush(CreatePendingSwapTxsKey(), mocks.Pending.PendingToMap()).SetErr(assert.AnError)
+		mock.ExpectRPush(CreatePendingSwapTxsKey(), mocks.SwapTx.ToMap()).SetErr(assert.AnError)
 
-		err := repo.StoreNewPendingSwap(ctx, mocks.Pending)
+		err := repo.StoreNewPendingSwap(ctx, mocks.SwapTx)
 
 		assert.ErrorContains(t, err, "failed to store pending swap tx")
 	})

data/redisrepo/store_open_order.go

@@ -13,7 +13,8 @@ import (
 // These methods should be used to store UNFILLED or PARTIALLY FILLED orders in Redis.
 //
 // `StoreFilledOrders` should be used to store completely filled orders.
-
+//
+// TODO: combine `StoreOpenOrder` and `StoreFilledOrder` into a single `StoreOrder` method that checks order status and stores accordingly.
 func (r *redisRepository) StoreOpenOrder(ctx context.Context, order models.Order) error {
 
 	// --- START TRANSACTION ---

data/redisrepo/store_pending_swaps.go

@@ -11,7 +11,7 @@ import (
 )
 
 // StorePendingSwaps stores all swaps that are still pending completion in order for their status (pending/complete) to be checked again later
-func (r *redisRepository) StorePendingSwaps(ctx context.Context, pendingSwaps []models.Pending) error {
+func (r *redisRepository) StorePendingSwaps(ctx context.Context, pendingSwaps []models.SwapTx) error {
 	key := CreatePendingSwapTxsKey()
 
 	transaction := r.client.TxPipeline()

data/store/store.go

@@ -29,9 +29,9 @@ type OrderBookStore interface {
 	GetSwap(ctx context.Context, swapId uuid.UUID) ([]models.OrderFrag, error)
 	StoreSwap(ctx context.Context, swapId uuid.UUID, frags []models.OrderFrag) error
 	RemoveSwap(ctx context.Context, swapId uuid.UUID) error
-	// Pending transactions
-	StoreNewPendingSwap(ctx context.Context, pendingSwap models.Pending) error
-	GetPendingSwaps(ctx context.Context) ([]models.Pending, error)
-	StorePendingSwaps(ctx context.Context, pendingSwaps []models.Pending) error
+	// Pending transactions (TODO: rename)
+	StoreNewPendingSwap(ctx context.Context, pendingSwap models.SwapTx) error
+	GetPendingSwaps(ctx context.Context) ([]models.SwapTx, error)
+	StorePendingSwaps(ctx context.Context, pendingSwaps []models.SwapTx) error
 	ProcessCompletedSwapOrders(ctx context.Context, orders []*models.Order, swapId uuid.UUID, isSuccessful bool) error
 }

mocks/service.go

@@ -26,7 +26,7 @@ type MockOrderBookStore struct {
 	// re-entrance
 	Sets map[string]map[string]struct{}
 	// Pending swaps
-	PendingSwaps []models.Pending
+	PendingSwaps []models.SwapTx
 }
 
 func (m *MockOrderBookStore) StoreOpenOrder(ctx context.Context, order models.Order) error {
@@ -140,15 +140,15 @@ func (m *MockOrderBookStore) UpdateSwapTracker(ctx context.Context, swapStatus m
 	return m.Error
 }
 
-func (m *MockOrderBookStore) StoreNewPendingSwap(ctx context.Context, pendingSwap models.Pending) error {
+func (m *MockOrderBookStore) StoreNewPendingSwap(ctx context.Context, pendingSwap models.SwapTx) error {
 	return m.Error
 }
 
-func (m *MockOrderBookStore) GetPendingSwaps(ctx context.Context) ([]models.Pending, error) {
+func (m *MockOrderBookStore) GetPendingSwaps(ctx context.Context) ([]models.SwapTx, error) {
 	return m.PendingSwaps, m.Error
 }
 
-func (m *MockOrderBookStore) StorePendingSwaps(ctx context.Context, pendingSwaps []models.Pending) error {
+func (m *MockOrderBookStore) StorePendingSwaps(ctx context.Context, pendingSwaps []models.SwapTx) error {
 	return m.Error
 }
 

mocks/pending.go → mocks/swap_tx.go

@@ -5,7 +5,7 @@ import (
 	"github.com/orbs-network/order-book/models"
 )
 
-var Pending = models.Pending{
+var SwapTx = models.SwapTx{
 	SwapId: uuid.MustParse("b3f9e3a0-5b7a-4b7a-8b0a-9b9b9b9b9b9b"),
 	TxHash: "0x5dcbfe934287c50363e5c82502739aadd4d535a1f7c0ccd7a8088fb4dfd800da",
 }

models/errors.go

@@ -19,7 +19,8 @@ var ErrOrderFilled = errors.New("order is filled")
 var ErrInvalidInput = errors.New("invalid input")
 var ErrSignatureVerificationError = errors.New("signature verification error")
 var ErrSignatureVerificationFailed = errors.New("signature verification failed")
-var ErrInvalidSize = errors.New("updated sizeFilled is greater than size")
+var ErrUnexpectedSizeFilled = errors.New("unexpected sizeFilled")
+var ErrUnexpectedSizePending = errors.New("unexpected sizePending")
 
 // store generic errors
 var ErrValAlreadyInSet = errors.New("the value is already a member of the set")

models/order.go

@@ -7,6 +7,8 @@ import (
 	"time"
 
 	"github.com/google/uuid"
+	"github.com/orbs-network/order-book/utils/logger"
+	"github.com/orbs-network/order-book/utils/logger/logctx"
 	"github.com/shopspring/decimal"
 )
 
@@ -215,21 +217,30 @@ func (o *Order) Status() string {
 	return "OPEN"
 }
 
-func (o *Order) MarkSwapSuccess() (isFilled bool, err error) {
-
-	newSizeFilled := o.SizeFilled.Add(o.SizePending)
+func (o *Order) Fill(ctx context.Context, fillSize decimal.Decimal) (isFilled bool, err error) {
+	newSizeFilled := o.SizeFilled.Add(fillSize)
 	if newSizeFilled.GreaterThan(o.Size) {
-		return false, ErrInvalidSize
+		logctx.Error(ctx, "total size is less than requested fill size", logger.String("orderId", o.Id.String()), logger.String("orderSize", o.Size.String()), logger.String("requestedFillSize", fillSize.String()))
+		return false, ErrUnexpectedSizeFilled
 	}
 
-	o.SizeFilled = newSizeFilled
-	o.SizePending = decimal.Zero
+	if fillSize.GreaterThan(o.SizePending) {
+		logctx.Error(ctx, "fillSize is greater than sizePending", logger.String("orderId", o.Id.String()), logger.String("pendingSize", o.SizePending.String()), logger.String("requestedFillSize", fillSize.String()))
+		return false, ErrUnexpectedSizePending
+	}
 
+	o.SizeFilled = o.SizeFilled.Add(fillSize)
+	o.SizePending = o.SizePending.Sub(fillSize)
 	return o.IsFilled(), nil
 }
 
-func (o *Order) MarkSwapFailed() error {
-	o.SizePending = decimal.Zero
+func (o *Order) Kill(ctx context.Context, size decimal.Decimal) error {
+	if o.SizePending.LessThan(size) {
+		logctx.Error(ctx, "size to be rolled back is greater than sizePending", logger.String("orderId", o.Id.String()), logger.String("pendingSize", o.SizePending.String()), logger.String("requestedKillSize", size.String()))
+		return ErrUnexpectedSizeFilled
+	}
+
+	o.SizePending = o.SizePending.Sub(size)
 	return nil
 }
 

models/order_test.go

@@ -1,6 +1,7 @@
 package models
 
 import (
+	"context"
 	"testing"
 	"time"
 
@@ -126,12 +127,15 @@ func TestOrder_MapToOrder(t *testing.T) {
 	})
 }
 
-func TestOrder_MarkSwapComplete(t *testing.T) {
-	// generate table test
+func TestOrder_Fill(t *testing.T) {
+
+	ctx := context.Background()
+
 	tests := []struct {
 		name     string
 		order    Order
 		expected Order
+		fillSize decimal.Decimal
 		isFilled bool
 		error    error
 	}{
@@ -147,6 +151,7 @@ func TestOrder_MarkSwapComplete(t *testing.T) {
 				SizeFilled:  decimal.NewFromInt(1000),
 				SizePending: decimal.Zero,
 			},
+			fillSize: decimal.NewFromInt(1000),
 			isFilled: true,
 		},
 		{
@@ -161,42 +166,61 @@ func TestOrder_MarkSwapComplete(t *testing.T) {
 				SizeFilled:  decimal.NewFromInt(1000),
 				SizePending: decimal.Zero,
 			},
+			fillSize: decimal.NewFromInt(500),
 			isFilled: true,
 		},
 		{
-			name: "size 23782378.50, sizeFilled 2.38, sizePending 1238.12",
+			name: "partial fill",
 			order: Order{
 				Size:        decimal.NewFromFloat(23782378.50),
 				SizeFilled:  decimal.NewFromFloat(2.38),
 				SizePending: decimal.NewFromFloat(1238.12),
 			},
 			expected: Order{
 				Size:        decimal.NewFromFloat(23782378.50),
-				SizeFilled:  decimal.NewFromFloat(1240.50),
-				SizePending: decimal.Zero,
+				SizeFilled:  decimal.NewFromFloat(2.38).Add(decimal.NewFromFloat(10)),
+				SizePending: decimal.NewFromFloat(1238.12).Sub(decimal.NewFromFloat(10)),
 			},
+			fillSize: decimal.NewFromFloat(10),
 			isFilled: false,
 		},
 		{
-			name: "size 1, sizeFilled 0.5, sizePending 1",
+			name: "total size is less than requested fill size",
 			order: Order{
-				Size:        decimal.NewFromInt(1),
-				SizeFilled:  decimal.NewFromFloat(0.5),
-				SizePending: decimal.NewFromInt(1),
+				Size:        decimal.NewFromFloat(10.00),
+				SizeFilled:  decimal.NewFromFloat(9.00),
+				SizePending: decimal.NewFromFloat(2.89),
 			},
 			expected: Order{
-				Size:        decimal.NewFromInt(1),
-				SizeFilled:  decimal.NewFromFloat(0.5),
-				SizePending: decimal.NewFromInt(1),
+				Size:        decimal.NewFromFloat(10.00),
+				SizeFilled:  decimal.NewFromFloat(9.00),
+				SizePending: decimal.NewFromFloat(2.89),
 			},
+			fillSize: decimal.NewFromFloat(2.00),
 			isFilled: false,
-			error:    ErrInvalidSize,
+			error:    ErrUnexpectedSizeFilled,
+		},
+		{
+			name: "size to be filled is greater than size pending",
+			order: Order{
+				Size:        decimal.NewFromFloat(10.00),
+				SizeFilled:  decimal.NewFromFloat(2.00),
+				SizePending: decimal.NewFromFloat(2.00),
+			},
+			expected: Order{
+				Size:        decimal.NewFromFloat(10.00),
+				SizeFilled:  decimal.NewFromFloat(2.00),
+				SizePending: decimal.NewFromFloat(2.00),
+			},
+			fillSize: decimal.NewFromFloat(4.00),
+			isFilled: false,
+			error:    ErrUnexpectedSizePending,
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			isFilled, err := test.order.MarkSwapSuccess()
+			isFilled, err := test.order.Fill(ctx, test.fillSize)
 			assert.Equal(t, test.expected.Size.String(), test.order.Size.String(), "size should be equal")
 			assert.Equal(t, test.expected.SizeFilled.String(), test.order.SizeFilled.String(), "sizeFilled should be equal")
 			assert.Equal(t, test.expected.SizePending.String(), test.order.SizePending.String(), "sizePending should be equal")
@@ -205,3 +229,46 @@ func TestOrder_MarkSwapComplete(t *testing.T) {
 		})
 	}
 }
+
+func TestOrder_Kill(t *testing.T) {
+
+	ctx := context.Background()
+
+	tests := []struct {
+		name     string
+		order    Order
+		expected Order
+		killSize decimal.Decimal
+		error    error
+	}{
+		{
+			name: "sizePending 1000, killSize 1000",
+			order: Order{
+				SizePending: decimal.NewFromFloat(1000),
+			},
+			expected: Order{
+				SizePending: decimal.Zero,
+			},
+			killSize: decimal.NewFromInt(1000),
+		},
+		{
+			name: "sizePending 500, killSize 1000",
+			order: Order{
+				SizePending: decimal.NewFromInt(500),
+			},
+			expected: Order{
+				SizePending: decimal.NewFromInt(500),
+			},
+			killSize: decimal.NewFromInt(1000),
+			error:    ErrUnexpectedSizeFilled,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			err := test.order.Kill(ctx, test.killSize)
+			assert.Equal(t, test.expected.SizePending.String(), test.order.SizePending.String(), "sizePending should be equal")
+			assert.Equal(t, test.error, err, "error should be equal")
+		})
+	}
+}