review followup

server/auth/auth.go

@@ -433,7 +433,7 @@ func (auth *AuthManager) ExpectUsers(users map[account.AccountID]struct{}, withi
 
 // RecordCancel records a user's executed cancel order, including the canceled
 // order ID, and the time when the cancel was executed.
-func (auth *AuthManager) RecordCancel(user account.AccountID, oid, target order.OrderID, epochGap int, t time.Time) {
+func (auth *AuthManager) RecordCancel(user account.AccountID, oid, target order.OrderID, epochGap int32, t time.Time) {
 	auth.recordOrderDone(user, oid, &target, epochGap, t.UnixMilli())
 }
 
@@ -449,7 +449,7 @@ func (auth *AuthManager) RecordCompletedOrder(user account.AccountID, oid order.
 // completed the swap negotiation. Note that in the case of a cancel, oid refers
 // to the ID of the cancel order itself, while target is non-nil for cancel
 // orders.
-func (auth *AuthManager) recordOrderDone(user account.AccountID, oid order.OrderID, target *order.OrderID, epochGap int, tMS int64) {
+func (auth *AuthManager) recordOrderDone(user account.AccountID, oid order.OrderID, target *order.OrderID, epochGap int32, tMS int64) {
 	client := auth.user(user)
 	if client == nil {
 		// It is likely that the user is gone if this is a revoked order.
@@ -1361,10 +1361,11 @@ func (auth *AuthManager) loadRecentFinishedOrders(aid account.AccountID, N int)
 	// Insert the executed cancels, popping off older orders that do not fit in
 	// the list.
 	for _, c := range cancels {
+		tid := c.TargetID
 		latestFinished.add(&oidStamped{
 			OrderID:  c.ID,
 			time:     c.MatchTime,
-			target:   &c.TargetID,
+			target:   &tid,
 			epochGap: c.EpochGap,
 		})
 	}

server/auth/auth_test.go

@@ -44,7 +44,7 @@ type ratioData struct {
 	oidsCancels    []order.OrderID
 	oidsCanceled   []order.OrderID
 	timesCanceled  []int64
-	epochGaps      []int
+	epochGaps      []int32
 }
 
 // TStorage satisfies the Storage interface
@@ -709,7 +709,7 @@ func TestConnect(t *testing.T) {
 	rig.storage.matches = []*db.MatchData{matchData}
 	defer func() { rig.storage.matches = nil }()
 
-	epochGaps := []int{1} // penalized
+	epochGaps := []int32{1} // penalized
 
 	rig.storage.setRatioData(&ratioData{
 		oidsCompleted:  []order.OrderID{{0x1}},

server/auth/cancel.go

@@ -15,8 +15,9 @@ import (
 // freeCancelThreshold is the minimum number of epochs a user should wait before
 // placing a cancel order, if they want to avoid penalization. It is set to 2,
 // which means if a user places a cancel order in the same epoch as its limit
-// order, or the next epoch, the user will be penalized. This ensures that the
-// order remains booked for at least one full epoch and one full match cycle.
+// order, or the next epoch, the user will be penalized. This value is chosen
+// because it is the minimum value such that the order remains booked for at
+// least one full epoch and one full match cycle.
 const freeCancelThreshold = 2
 
 // oidStamped is a time-stamped order ID, with a field for target order ID if
@@ -25,7 +26,7 @@ type oidStamped struct {
 	order.OrderID
 	time     int64
 	target   *order.OrderID
-	epochGap int
+	epochGap int32
 }
 
 // ordsByTimeThenID is used to sort an ord slice in ascending order by time and

server/db/driver/pg/orders.go

@@ -183,7 +183,7 @@ func (status pgOrderStatus) active() bool {
 
 // NewEpochOrder stores the given order with epoch status. This is equivalent to
 // StoreOrder with OrderStatusEpoch.
-func (a *Archiver) NewEpochOrder(ord order.Order, epochIdx, epochDur int64, epochGap int) error {
+func (a *Archiver) NewEpochOrder(ord order.Order, epochIdx, epochDur int64, epochGap int32) error {
 	return a.storeOrder(ord, epochIdx, epochDur, epochGap, orderStatusEpoch)
 }
 
@@ -538,7 +538,7 @@ func (a *Archiver) StoreOrder(ord order.Order, epochIdx, epochDur int64, status
 	return a.storeOrder(ord, epochIdx, epochDur, db.EpochGapNA, marketToPgStatus(status))
 }
 
-func (a *Archiver) storeOrder(ord order.Order, epochIdx, epochDur int64, epochGap int, status pgOrderStatus) error {
+func (a *Archiver) storeOrder(ord order.Order, epochIdx, epochDur int64, epochGap int32, status pgOrderStatus) error {
 	marketSchema, err := a.marketSchema(ord.Base(), ord.Quote())
 	if err != nil {
 		return err
@@ -1278,7 +1278,7 @@ func (a *Archiver) executedCancelsForUser(ctx context.Context, dbe *sql.DB, stmt
 	for rows.Next() {
 		var oid, target order.OrderID
 		var execTime int64
-		var epochGap int
+		var epochGap int32
 		err = rows.Scan(&oid, &target, &epochGap, &execTime)
 		if err != nil {
 			return
@@ -1711,7 +1711,7 @@ func moveOrder(dbe sqlExecutor, oldTableName, newTableName string, oid order.Ord
 
 // BEGIN cancel order functions
 
-func storeCancelOrder(dbe sqlExecutor, tableName string, co *order.CancelOrder, status pgOrderStatus, epochIdx, epochDur int64, epochGap int) (int64, error) {
+func storeCancelOrder(dbe sqlExecutor, tableName string, co *order.CancelOrder, status pgOrderStatus, epochIdx, epochDur int64, epochGap int32) (int64, error) {
 	stmt := fmt.Sprintf(internal.InsertCancelOrder, tableName)
 	return sqlExec(dbe, stmt, co.ID(), co.AccountID, co.ClientTime,
 		co.ServerTime, co.Commit, co.TargetOrderID, status, epochIdx, epochDur, epochGap)

server/db/driver/pg/upgrades.go

@@ -40,8 +40,8 @@ var upgrades = []func(db *sql.Tx) error{
 	// accommodate a 32-bit unsigned integer.
 	v4Upgrade,
 
-	// v5 upgrade adds an epoch_gap column to the cancel order tables in order
-	// to facilitate free cancels.
+	// v5 upgrade adds an epoch_gap column to the cancel order tables to
+	// facilitate free cancels.
 	v5Upgrade,
 }
 
@@ -306,7 +306,7 @@ func v5Upgrade(tx *sql.Tx) (err error) {
 		return err
 	}
 
-	log.Info("Adding epoch_gap column to %d market tables", len(mkts)*2)
+	log.Infof("Adding epoch_gap column to cancel tables for %d markets", len(mkts))
 
 	for _, mkt := range mkts {
 		if err := doTable(mkt.Name + "." + cancelsArchivedTableName); err != nil {

server/db/interface.go

@@ -152,8 +152,10 @@ type OrderArchiver interface {
 
 	// NewEpochOrder stores a new order with epoch status. Such orders are
 	// pending execution or insertion on a book (standing limit orders with a
-	// remaining unfilled amount).
-	NewEpochOrder(ord order.Order, epochIdx, epochDur int64, epochGap int) error
+	// remaining unfilled amount). For trade orders, the epoch gap should be
+	// db.EpochGapNA, while for cancel orders it is the number of epochs since
+	// the targeted order was placed, as described in the docs for CancelRecord.
+	NewEpochOrder(ord order.Order, epochIdx, epochDur int64, epochGap int32) error
 
 	// StorePreimage stores the preimage associated with an existing order.
 	StorePreimage(ord order.Order, pi order.Preimage) error
@@ -471,12 +473,15 @@ func ValidateOrder(ord order.Order, status order.OrderStatus, mkt *dex.MarketInf
 // cancel order) when such a designation doesn't apply in-context. For instance,
 // revocations are treated in many places like cancel orders, but there is no
 // reason to consider the epoch gap.
-const EpochGapNA int = -1
+const EpochGapNA int32 = -1
 
 // CancelRecord is info about a cancel order and when it matched.
 type CancelRecord struct {
 	ID        order.OrderID
 	TargetID  order.OrderID
 	MatchTime int64
-	EpochGap  int
+	// EpochGap is the number of epochs passed since the targeted trade order
+	// was placed, where 0 means canceled in the same epoch, 1 means canceled in
+	// the next epoch, etc.
+	EpochGap int32
 }

server/market/market.go

@@ -1799,7 +1799,7 @@ func (m *Market) processOrder(rec *orderRecord, epoch *EpochQueue, notifyChan ch
 			return nil
 		}
 
-		epochGap = int(epoch.Epoch - loTime.UnixMilli()/epoch.Duration)
+		epochGap = int32(epoch.Epoch - loTime.UnixMilli()/epoch.Duration)
 
 	} else if likelyTaker(ord) { // Likely-taker trade order. Check the quantity against user's limit.
 		// NOTE: We can entirely change this so that the taker limit is not
@@ -2352,7 +2352,7 @@ func (m *Market) processReadyEpoch(epoch *readyEpoch, notifyChan chan<- *updateS
 		// Update order settling amounts.
 		for _, match := range ms.Matches() {
 			if co, ok := match.Taker.(*order.CancelOrder); ok {
-				epochGap := int((co.ServerTime.UnixMilli() / epochDur) - (match.Maker.ServerTime.UnixMilli() / epochDur))
+				epochGap := int32((co.ServerTime.UnixMilli() / epochDur) - (match.Maker.ServerTime.UnixMilli() / epochDur))
 				m.auth.RecordCancel(co.User(), co.ID(), co.TargetOrderID, epochGap, matchTime)
 				canceled = append(canceled, co.TargetOrderID)
 				continue

server/market/market_test.go

@@ -115,7 +115,7 @@ func (ta *TArchivist) ExecutedCancelsForUser(aid account.AccountID, N int) ([]*d
 func (ta *TArchivist) OrderStatus(order.Order) (order.OrderStatus, order.OrderType, int64, error) {
 	return order.OrderStatusUnknown, order.UnknownOrderType, -1, errors.New("boom")
 }
-func (ta *TArchivist) NewEpochOrder(ord order.Order, epochIdx, epochDur int64, epochGap int) error {
+func (ta *TArchivist) NewEpochOrder(ord order.Order, epochIdx, epochDur int64, epochGap int32) error {
 	ta.mtx.Lock()
 	defer ta.mtx.Unlock()
 	if ta.poisonEpochOrder != nil && ord.ID() == ta.poisonEpochOrder.ID() {

server/market/orderrouter.go

@@ -35,7 +35,7 @@ type AuthManager interface {
 	RequestWithTimeout(account.AccountID, *msgjson.Message, func(comms.Link, *msgjson.Message), time.Duration, func()) error
 	PreimageSuccess(user account.AccountID, refTime time.Time, oid order.OrderID)
 	MissedPreimage(user account.AccountID, refTime time.Time, oid order.OrderID)
-	RecordCancel(user account.AccountID, oid, target order.OrderID, epochGap int, t time.Time)
+	RecordCancel(user account.AccountID, oid, target order.OrderID, epochGap int32, t time.Time)
 	RecordCompletedOrder(user account.AccountID, oid order.OrderID, t time.Time)
 	UserSettlingLimit(user account.AccountID, mkt *dex.MarketInfo) int64
 }

server/market/routers_test.go

@@ -249,7 +249,7 @@ func (a *TAuth) UserSettlingLimit(user account.AccountID, mkt *dex.MarketInfo) i
 }
 
 func (a *TAuth) RecordCompletedOrder(account.AccountID, order.OrderID, time.Time) {}
-func (a *TAuth) RecordCancel(aid account.AccountID, coid, oid order.OrderID, epochGap int, t time.Time) {
+func (a *TAuth) RecordCancel(aid account.AccountID, coid, oid order.OrderID, epochGap int32, t time.Time) {
 	a.cancelOrder = coid
 	a.canceledOrder = oid
 }

server/swap/swap_test.go

@@ -230,9 +230,6 @@ func (m *TAuthManager) penalize(id account.AccountID, rule account.Rule) {
 	}
 }
 
-func (m *TAuthManager) RecordCancel(user account.AccountID, oid, target order.OrderID, t time.Time) {}
-func (m *TAuthManager) RecordCompletedOrder(account.AccountID, order.OrderID, time.Time)            {}
-
 func (m *TAuthManager) flushPenalty(user account.AccountID) (found bool, rule account.Rule) {
 	m.mtx.Lock()
 	defer m.mtx.Unlock()