Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

server/{market,swap,db}: free cancels #1682

Merged
merged 3 commits into from Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
29 changes: 16 additions & 13 deletions server/auth/auth.go
Expand Up @@ -54,7 +54,7 @@ type Storage interface {
UserOrderStatuses(aid account.AccountID, base, quote uint32, oids []order.OrderID) ([]*db.OrderStatus, error)
ActiveUserOrderStatuses(aid account.AccountID) ([]*db.OrderStatus, error)
CompletedUserOrders(aid account.AccountID, N int) (oids []order.OrderID, compTimes []int64, err error)
ExecutedCancelsForUser(aid account.AccountID, N int) (oids, targets []order.OrderID, execTimes []int64, err error)
ExecutedCancelsForUser(aid account.AccountID, N int) ([]*db.CancelRecord, error)
CompletedAndAtFaultMatchStats(aid account.AccountID, lastN int) ([]*db.MatchOutcome, error)
PreimageStats(user account.AccountID, lastN int) ([]*db.PreimageResult, error)
AllActiveUserMatches(aid account.AccountID) ([]*db.MatchData, error)
Expand Down Expand Up @@ -433,23 +433,23 @@ 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, t time.Time) {
auth.recordOrderDone(user, oid, &target, t.UnixMilli())
func (auth *AuthManager) RecordCancel(user account.AccountID, oid, target order.OrderID, epochGap int32, t time.Time) {
auth.recordOrderDone(user, oid, &target, epochGap, t.UnixMilli())
}

// RecordCompletedOrder records a user's completed order, where completed means
// a swap involving the order was successfully completed and the order is no
// longer on the books if it ever was.
func (auth *AuthManager) RecordCompletedOrder(user account.AccountID, oid order.OrderID, t time.Time) {
auth.recordOrderDone(user, oid, nil, t.UnixMilli())
auth.recordOrderDone(user, oid, nil, db.EpochGapNA, t.UnixMilli())
}

// recordOrderDone an order that has finished processing. This can be a cancel
// order, which matched and unbooked another order, or a trade order that
// 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, 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.
Expand All @@ -460,9 +460,10 @@ func (auth *AuthManager) recordOrderDone(user account.AccountID, oid order.Order
// Update recent orders and check/set suspended status atomically.
client.mtx.Lock()
client.recentOrders.add(&oidStamped{
OrderID: oid,
time: tMS,
target: target,
OrderID: oid,
time: tMS,
target: target,
epochGap: epochGap,
})

log.Debugf("Recorded order %v that has finished processing: user=%v, time=%v, target=%v",
Expand Down Expand Up @@ -1342,7 +1343,7 @@ func (auth *AuthManager) loadRecentFinishedOrders(aid account.AccountID, N int)
}

// Load the N latest executed cancel orders for the user.
cancelOids, targetOids, cancelTimes, err := auth.storage.ExecutedCancelsForUser(aid, N)
cancels, err := auth.storage.ExecutedCancelsForUser(aid, N)
if err != nil {
return nil, err
}
Expand All @@ -1359,11 +1360,13 @@ 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 i := range cancelOids {
for _, c := range cancels {
tid := c.TargetID
latestFinished.add(&oidStamped{
OrderID: cancelOids[i],
time: cancelTimes[i],
target: &targetOids[i],
OrderID: c.ID,
time: c.MatchTime,
target: &tid,
epochGap: c.EpochGap,
})
}

Expand Down
38 changes: 35 additions & 3 deletions server/auth/auth_test.go
Expand Up @@ -44,6 +44,7 @@ type ratioData struct {
oidsCancels []order.OrderID
oidsCanceled []order.OrderID
timesCanceled []int64
epochGaps []int32
}

// TStorage satisfies the Storage interface
Expand Down Expand Up @@ -122,8 +123,16 @@ func (s *TStorage) setRatioData(dat *ratioData) {
func (s *TStorage) CompletedUserOrders(aid account.AccountID, _ int) (oids []order.OrderID, compTimes []int64, err error) {
return s.ratio.oidsCompleted, s.ratio.timesCompleted, nil
}
func (s *TStorage) ExecutedCancelsForUser(aid account.AccountID, _ int) (oids, targets []order.OrderID, execTimes []int64, err error) {
return s.ratio.oidsCancels, s.ratio.oidsCanceled, s.ratio.timesCanceled, nil
func (s *TStorage) ExecutedCancelsForUser(aid account.AccountID, _ int) (cancels []*db.CancelRecord, err error) {
for i := range s.ratio.oidsCanceled {
cancels = append(cancels, &db.CancelRecord{
ID: s.ratio.oidsCancels[i],
TargetID: s.ratio.oidsCanceled[i],
MatchTime: s.ratio.timesCanceled[i],
EpochGap: s.ratio.epochGaps[i],
})
}
return cancels, nil
}

// TSigner satisfies the Signer interface
Expand Down Expand Up @@ -700,12 +709,15 @@ func TestConnect(t *testing.T) {
rig.storage.matches = []*db.MatchData{matchData}
defer func() { rig.storage.matches = nil }()

epochGaps := []int32{1} // penalized

rig.storage.setRatioData(&ratioData{
oidsCompleted: []order.OrderID{{0x1}},
timesCompleted: []int64{1234},
oidsCancels: []order.OrderID{{0x2}},
oidsCanceled: []order.OrderID{{0x1}},
timesCanceled: []int64{1235},
epochGaps: epochGaps,
}) // 1:1 = 50%
defer rig.storage.setRatioData(&ratioData{}) // clean slate

Expand All @@ -716,6 +728,15 @@ func TestConnect(t *testing.T) {
t.Fatalf("Expected account %v to be closed on connect, got %v", user.acctID, rig.storage.closedID)
}

// Make it a free cancel.
rig.storage.closedID = account.AccountID{} // unclose the account in db
epochGaps[0] = 2
connectUser(t, user)
if rig.storage.closedID == user.acctID {
t.Fatalf("Expected account %v to NOT be closed with free cancels, but it was.", user)
}
epochGaps[0] = 1

// Try again just meeting cancel ratio.
rig.storage.closedID = account.AccountID{} // unclose the account in db
rig.mgr.cancelThresh = 0.6 // passable threshold for 1 cancel : 1 completion (0.5)
Expand All @@ -731,12 +752,22 @@ func TestConnect(t *testing.T) {
rig.storage.ratio.oidsCanceled = append(rig.storage.ratio.oidsCanceled, order.OrderID{0x3})
rig.storage.ratio.oidsCancels = append(rig.storage.ratio.oidsCancels, order.OrderID{0x4})
rig.storage.ratio.timesCanceled = append(rig.storage.ratio.timesCanceled, 12341234)
rig.storage.ratio.epochGaps = append(rig.storage.ratio.epochGaps, 1)

tryConnectUser(t, user, false)
if rig.storage.closedID != user.acctID {
t.Fatalf("Expected account %v to be closed on connect, got %v", user.acctID, rig.storage.closedID)
}

// Make one a free cancel.
rig.storage.closedID = account.AccountID{} // unclose the account in db
rig.storage.ratio.epochGaps[1] = 2
connectUser(t, user)
if rig.storage.closedID == user.acctID {
t.Fatalf("Expected account %v to NOT be closed with free cancels, but it was.", user)
}
rig.storage.ratio.epochGaps[1] = 0

// Try again just meeting cancel ratio.
rig.storage.closedID = account.AccountID{} // unclose the account in db
rig.mgr.cancelThresh = 0.7 // passable threshold for 2 cancel : 1 completion (0.6666..)
Expand All @@ -761,6 +792,7 @@ func TestConnect(t *testing.T) {
rig.storage.ratio.oidsCanceled = append(rig.storage.ratio.oidsCanceled, order.OrderID{0x4})
rig.storage.ratio.oidsCancels = append(rig.storage.ratio.oidsCancels, order.OrderID{0x5})
rig.storage.ratio.timesCanceled = append(rig.storage.ratio.timesCanceled, 12341239)
rig.storage.ratio.epochGaps = append(rig.storage.ratio.epochGaps, 1)

tryConnectUser(t, user, false)
if rig.storage.closedID == user.acctID {
Expand Down Expand Up @@ -1630,7 +1662,7 @@ func TestAuthManager_RecordCancel_RecordCompletedOrder(t *testing.T) {
// now a cancel
coid := newOrderID()
tCompleted = tCompleted.Add(time.Millisecond) // newer
rig.mgr.RecordCancel(user.acctID, coid, oid, tCompleted)
rig.mgr.RecordCancel(user.acctID, coid, oid, 1, tCompleted)

client.mtx.Lock()
total, cancels = client.recentOrders.counts()
Expand Down
15 changes: 12 additions & 3 deletions server/auth/cancel.go
Expand Up @@ -12,12 +12,21 @@ import (
"decred.org/dcrdex/dex/order"
)

// 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 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
// the order is a cancel order.
type oidStamped struct {
order.OrderID
time int64
target *order.OrderID
time int64
target *order.OrderID
epochGap int32
}

// ordsByTimeThenID is used to sort an ord slice in ascending order by time and
Expand Down Expand Up @@ -116,7 +125,7 @@ func (lo *latestOrders) counts() (total, cancels int) {

total = len(lo.orders)
for _, o := range lo.orders {
if o.target != nil {
if o.target != nil && o.epochGap >= 0 && o.epochGap < freeCancelThreshold {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the line that matters. This is a rule change. Free cancels after the order sits for one full epoch. If the order is submitted in epoch 10, and the cancel order submitted in epoch 10 or 11, that's a "short" cancel (not a free cancel), and the cancellation can hurt to the user's score.

cancels++
}
}
Expand Down
40 changes: 21 additions & 19 deletions server/auth/cancel_test.go
Expand Up @@ -6,6 +6,7 @@ import (
"testing"

"decred.org/dcrdex/dex/order"
"decred.org/dcrdex/server/db"
)

func randomOrderID() (oid order.OrderID) {
Expand Down Expand Up @@ -46,19 +47,19 @@ func Test_latestOrders(t *testing.T) {
// add one cancel
ts := int64(1234)
coid := randomOrderID()
ordList.add(&oidStamped{order.OrderID{0x1}, ts, &coid})
ordList.add(&oidStamped{order.OrderID{0x1}, ts, &coid, 1})
checkSort()
total, cancels = ordList.counts()
if total != 1 {
t.Errorf("expected 1 orders, got %d", total)
}
if cancels != 1 {
t.Errorf("expected 1 cancels, got %d", total)
t.Errorf("expected 1 cancels, got %d", cancels)
}

// add one non-cancel
ts++
ordList.add(&oidStamped{order.OrderID{0x2}, ts, nil})
ordList.add(&oidStamped{order.OrderID{0x2}, ts, nil, db.EpochGapNA})
checkSort()
total, cancels = ordList.counts()
if total != 2 {
Expand All @@ -69,7 +70,7 @@ func Test_latestOrders(t *testing.T) {
}

// add one that is the smallest
ordList.add(&oidStamped{order.OrderID{0x3}, ts - 10, nil})
ordList.add(&oidStamped{order.OrderID{0x3}, ts - 10, nil, db.EpochGapNA})
checkSort()
total, cancels = ordList.counts()
if total != 3 {
Expand All @@ -84,9 +85,10 @@ func Test_latestOrders(t *testing.T) {
for i := total; i < int(cap); i++ {
ts++
ordList.add(&oidStamped{
OrderID: randomOrderID(),
time: ts,
target: maybeCancel(),
OrderID: randomOrderID(),
time: ts,
target: maybeCancel(),
epochGap: db.EpochGapNA,
})
checkSort()
}
Expand Down Expand Up @@ -185,34 +187,34 @@ func Test_ordsByTimeThenID_Sort(t *testing.T) {
{
name: "unique, no swap",
ords: []*oidStamped{
{order.OrderID{0x1}, 1234, nil},
{order.OrderID{0x2}, 1235, nil},
{order.OrderID{0x1}, 1234, nil, db.EpochGapNA},
{order.OrderID{0x2}, 1235, nil, db.EpochGapNA},
},
wantOrds: []*oidStamped{
{order.OrderID{0x1}, 1234, nil},
{order.OrderID{0x2}, 1235, nil},
{order.OrderID{0x1}, 1234, nil, db.EpochGapNA},
{order.OrderID{0x2}, 1235, nil, db.EpochGapNA},
},
},
{
name: "unique, one swap",
ords: []*oidStamped{
{order.OrderID{0x2}, 1235, nil},
{order.OrderID{0x1}, 1234, nil},
{order.OrderID{0x2}, 1235, nil, db.EpochGapNA},
{order.OrderID{0x1}, 1234, nil, db.EpochGapNA},
},
wantOrds: []*oidStamped{
{order.OrderID{0x1}, 1234, nil},
{order.OrderID{0x2}, 1235, nil},
{order.OrderID{0x1}, 1234, nil, db.EpochGapNA},
{order.OrderID{0x2}, 1235, nil, db.EpochGapNA},
},
},
{
name: "time tie, swap by order ID",
ords: []*oidStamped{
{order.OrderID{0x2}, 1234, nil},
{order.OrderID{0x1}, 1234, nil},
{order.OrderID{0x2}, 1234, nil, db.EpochGapNA},
{order.OrderID{0x1}, 1234, nil, db.EpochGapNA},
},
wantOrds: []*oidStamped{
{order.OrderID{0x1}, 1234, nil},
{order.OrderID{0x2}, 1234, nil},
{order.OrderID{0x1}, 1234, nil, db.EpochGapNA},
{order.OrderID{0x2}, 1234, nil, db.EpochGapNA},
},
},
}
Expand Down
10 changes: 6 additions & 4 deletions server/db/driver/pg/internal/orders.go
Expand Up @@ -180,6 +180,7 @@ const (
target_order BYTEA, -- cancel orders ref another order
status INT2,
epoch_idx INT8, epoch_dur INT4, -- 0 for rule-based revocations, -1 for exempt (e.g. book purge)
epoch_gap INT4 DEFAULT -1, -- epochs between order and cancel order. -1 for revocations
preimage BYTEA UNIQUE -- null before preimage collection, and all server-generated cancels (revocations)
);`

Expand Down Expand Up @@ -221,16 +222,17 @@ const (
// the match_time directly instead of the epoch_idx and epoch_dur. The
// cancels table, with full market schema, is %[1]s, while the epochs table
// is %[2]s.
RetrieveCancelTimesForUserByStatus = `SELECT oid, target_order, match_time
RetrieveCancelTimesForUserByStatus = `SELECT oid, target_order, epoch_gap, match_time
FROM %[1]s -- a cancels table
JOIN %[2]s ON %[2]s.epoch_idx = %[1]s.epoch_idx AND %[2]s.epoch_dur = %[1]s.epoch_dur -- join on epochs table PK
WHERE account_id = $1 AND status = $2
ORDER BY match_time DESC
LIMIT $3;` // NOTE: find revoked orders via SelectRevokeCancels

// InsertCancelOrder inserts a cancel order row into the specified table.
InsertCancelOrder = `INSERT INTO %s (oid, account_id, client_time, server_time, commit, target_order, status, epoch_idx, epoch_dur)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9);`
InsertCancelOrder = `INSERT INTO %s (oid, account_id, client_time, server_time,
commit, target_order, status, epoch_idx, epoch_dur, epoch_gap)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10);`

// CancelOrderStatus retrieves an order's status
CancelOrderStatus = `SELECT status FROM %s WHERE oid = $1;`
Expand All @@ -240,7 +242,7 @@ const (
MoveCancelOrder = `WITH moved AS (
DELETE FROM %s
WHERE oid = $1
RETURNING oid, account_id, client_time, server_time, commit, target_order, %d, epoch_idx, epoch_dur, preimage
RETURNING oid, account_id, client_time, server_time, commit, target_order, %d, epoch_idx, epoch_dur, epoch_gap, preimage
)
INSERT INTO %s
SELECT * FROM moved;`
Expand Down