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.

Sign up for GitHub

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

Jump to bottom

server/swap: inaction check fixes #680

Merged
merged 12 commits into from
Sep 21, 2020
Merged

server/swap: inaction check fixes #680

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
7589939
server/swap: inaction check fixes
chappjc Sep 9, 2020
0aebdcc
harness tweaks and optimizations
chappjc Sep 15, 2020
bf91bf5
simplify inaction tickers/timers/triggers
chappjc Sep 15, 2020
831ec23
TESTING logs and timeout tweaks
chappjc Sep 16, 2020
a2897ce
that was a recipie for deadlock
chappjc Sep 16, 2020
a7611e2
fix data race on swapStatus time accesses
chappjc Sep 17, 2020
0dd6524
TESTS no longer sleep on new block, recv signal instead
chappjc Sep 17, 2020
06fe8a1
dcr harness: use sendmany to fund wallets
chappjc Sep 18, 2020
5420c42
review items
chappjc Sep 18, 2020
0ed20a1
review
chappjc Sep 18, 2020
9780f88
revise comment in latencyQ ExpireFunc
chappjc Sep 21, 2020
659fc3d
grammer
chappjc Sep 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 21 additions & 14 deletions client/core/trade_simnet_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ func testNoMakerRedeem(t *testing.T) {
}

// testMakerGhostingAfterTakerRedeem places simple orders for clients 1 and 2,
// neogiates the resulting trades smoothly till TakerSwapCast, then Maker goes
// negotiates the resulting trades smoothly till TakerSwapCast, then Maker goes
// AWOL after redeeming taker's swap without notifying Taker. This test ensures
// that Taker auto-finds Maker's redeem, extracts the secret key and redeems
// Maker's swap to complete the trade.
Expand Down Expand Up @@ -543,7 +543,7 @@ func monitorTrackedTrade(ctx context.Context, client *tClient, tracker *trackedT
}

if assetToMine != nil {
assetID, nBlocks := assetToMine.ID, uint16(assetToMine.SwapConf)+1 // mine 1 extra to ensure tx gets req. confs
assetID, nBlocks := assetToMine.ID, uint16(assetToMine.SwapConf)
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity... does mining an additional block (i.e. 2 blocks in quick succession) cause issues for the swap inaction checker?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, but the extra blocks were added in (pretty sure) because we were grappling with issues getting txns mined on simnet, then we figured out we needed the regentemplate RPC in mine-alpha, which seems to have entirely resolved the mining issue. Do you think we still need an extra block in any of these cases? The appropriate tests would only mine the blocks that reach inaction (just SwapConf).

err := mineBlocks(assetID, nBlocks)
if err == nil {
var actor order.MatchSide
Expand Down Expand Up @@ -693,10 +693,10 @@ func checkAndWaitForRefunds(ctx context.Context, client *tClient, orderID string
// confirm that balance changes are as expected.
for assetID, expectedBalanceDiff := range refundAmts {
if expectedBalanceDiff > 0 {
mineBlocks(assetID, 2)
mineBlocks(assetID, 1)
}
}
time.Sleep(5 * time.Second)
time.Sleep(2 * time.Second)

client.expectBalanceDiffs = refundAmts
err = client.assertBalanceChanges()
Expand Down Expand Up @@ -826,7 +826,7 @@ func (client *tClient) connectDEX(ctx context.Context) error {

// mine drc block(s) to mark fee as paid
// sometimes need to mine an extra block for fee tx to get req. confs
err = mineBlocks(dcr.BipID, regRes.ReqConfirms+1)
err = mineBlocks(dcr.BipID, regRes.ReqConfirms)
if err != nil {
return err
}
Expand Down Expand Up @@ -995,27 +995,25 @@ func (client *tClient) lockWallets() error {
client.log("locking wallets")
dcrw := client.dcrw()
lockCmd := fmt.Sprintf("./%s walletlock", dcrw.daemon)
if err := tmuxSendKeys("dcr-harness:0", lockCmd); err != nil {
if err := tmuxRun("dcr-harness:0", lockCmd); err != nil {
return err
}
time.Sleep(500 * time.Millisecond)
btcw := client.btcw()
lockCmd = fmt.Sprintf("./%s -rpcwallet=%s walletlock", btcw.daemon, btcw.walletName)
return tmuxSendKeys("btc-harness:2", lockCmd)
return tmuxRun("btc-harness:2", lockCmd)
}

func (client *tClient) unlockWallets() error {
client.log("unlocking wallets")
dcrw := client.dcrw()
unlockCmd := fmt.Sprintf("./%s walletpassphrase %q 600", dcrw.daemon, string(dcrw.pass))
if err := tmuxSendKeys("dcr-harness:0", unlockCmd); err != nil {
if err := tmuxRun("dcr-harness:0", unlockCmd); err != nil {
return err
}
time.Sleep(500 * time.Millisecond)
btcw := client.btcw()
unlockCmd = fmt.Sprintf("./%s -rpcwallet=%s walletpassphrase %q 600",
btcw.daemon, btcw.walletName, string(btcw.pass))
return tmuxSendKeys("btc-harness:2", unlockCmd)
return tmuxRun("btc-harness:2", unlockCmd)
}

func mineBlocks(assetID uint32, blocks uint16) error {
Expand All @@ -1028,11 +1026,20 @@ func mineBlocks(assetID uint32, blocks uint16) error {
default:
return fmt.Errorf("can't mine blocks for unknown asset %d", assetID)
}
return tmuxSendKeys(harnessID, fmt.Sprintf("./mine-alpha %d", blocks))
return tmuxRun(harnessID, fmt.Sprintf("./mine-alpha %d", blocks))
}

func tmuxSendKeys(tmuxWindow, cmd string) error {
return exec.Command("tmux", "send-keys", "-t", tmuxWindow, cmd, "C-m").Run()
func tmuxRun(tmuxWindow, cmd string) error {
tStart := time.Now()
defer func() {
fmt.Printf("********** TIMING: Took %v to run %q", time.Since(tStart), cmd)
}()
cmd += "; tmux wait-for -S harnessdone"
err := exec.Command("tmux", "send-keys", "-t", tmuxWindow, cmd, "C-m").Run() // ; wait-for harnessdone
if err != nil {
return nil
}
return exec.Command("tmux", "wait-for", "harnessdone").Run()
chappjc marked this conversation as resolved.
Show resolved Hide resolved
}

func fmtAmt(anyAmt interface{}) float64 {
Expand Down
19 changes: 12 additions & 7 deletions dex/testing/dcr/harness.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,11 @@ cat > "${NODES_ROOT}/harness-ctl/mine-alpha" <<EOF
esac
for i in \$(seq \$NUM) ; do
dcrctl -C ${NODES_ROOT}/alpha/alpha-ctl.conf regentemplate
sleep 0.1
sleep 0.05
dcrctl -C ${NODES_ROOT}/alpha/alpha-ctl.conf generate 1
sleep 0.5
if [ $i != $NUM ]; then
sleep 0.5
fi
done
EOF
chmod +x "${NODES_ROOT}/harness-ctl/mine-alpha"
Expand All @@ -79,9 +81,11 @@ NUM=1
esac
for i in \$(seq \$NUM) ; do
dcrctl -C ${NODES_ROOT}/beta/beta-ctl.conf regentemplate
sleep 0.1
sleep 0.05
dcrctl -C ${NODES_ROOT}/beta/beta-ctl.conf generate 1
sleep 0.5
if [ $i != $NUM ]; then
sleep 0.5
fi
done
EOF
chmod +x "${NODES_ROOT}/harness-ctl/mine-beta"
Expand Down Expand Up @@ -221,10 +225,11 @@ sleep 5
# Have alpha send some credits to the other wallets
for i in 10 18 5 7 1 15 3 25
do
tmux send-keys -t $SESSION:0 "./fund ${BETA_MINING_ADDR} ${i}${WAIT}" C-m\; wait-for donedcr
tmux send-keys -t $SESSION:0 "./fund ${TRADING_WALLET1_ADDRESS} ${i}${WAIT}" C-m\; wait-for donedcr
tmux send-keys -t $SESSION:0 "./fund ${TRADING_WALLET2_ADDRESS} ${i}${WAIT}" C-m\; wait-for donedcr
RECIPIENTS="{\"${BETA_MINING_ADDR}\":${i},\"${TRADING_WALLET1_ADDRESS}\":${i},\"${TRADING_WALLET2_ADDRESS}\":${i}}"
tmux send-keys -t $SESSION:0 "./alpha sendmany default '${RECIPIENTS}'${WAIT}" C-m\; wait-for donedcr
done
sleep 0.5
tmux send-keys -t $SESSION:0 "./mine-alpha 1${WAIT}" C-m\; wait-for donedcr

# Create fee account on alpha wallet for use by dcrdex simnet instances.
tmux send-keys -t $SESSION:0 "./alpha createnewaccount server_fees${WAIT}" C-m\; wait-for donedcr
Expand Down
1 change: 1 addition & 0 deletions dex/testing/dcrdex/harness.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ signingkeypass=keypass
adminsrvon=1
adminsrvpass=adminpass
adminsrvaddr=127.0.0.1:16542
bcasttimeout=1m
EOF

# Set the postgres user pass if provided.
Expand Down
2 changes: 1 addition & 1 deletion server/cmd/dcrdex/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const (
defaultCancelThresh = 0.95 // 19 cancels : 1 success
defaultRegFeeConfirms = 4
defaultRegFeeAmount = 1e8
defaultBroadcastTimeout = time.Minute
defaultBroadcastTimeout = 5 * time.Minute
)

var (
Expand Down
2 changes: 1 addition & 1 deletion server/db/driver/pg/orders.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -796,7 +796,7 @@ func (a *Archiver) updateOrderStatusByID(oid order.OrderID, base, quote uint32,
}

if initStatus == status && filled == initFilled {
log.Debugf("Not updating order with no status or filled amount change.")
log.Tracef("Not updating order with no status or filled amount change: %v.", oid)
return nil
}
if filled == -1 {
Expand Down
4 changes: 2 additions & 2 deletions server/market/orderrouter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ func (r *OrderRouter) handleLimit(user account.AccountID, msg *msgjson.Message)
}

if _, suspended := r.auth.Suspended(user); suspended {
return msgjson.NewError(msgjson.MarketNotRunningError, "suspended account may not submit trade orders")
return msgjson.NewError(msgjson.MarketNotRunningError, "suspended account %v may not submit trade orders", user)
}

tunnel, coins, sell, rpcErr := r.extractMarketDetails(&limit.Prefix, &limit.Trade)
Expand Down Expand Up @@ -290,7 +290,7 @@ func (r *OrderRouter) handleMarket(user account.AccountID, msg *msgjson.Message)
}

if _, suspended := r.auth.Suspended(user); suspended {
return msgjson.NewError(msgjson.MarketNotRunningError, "suspended account may not submit trade orders")
return msgjson.NewError(msgjson.MarketNotRunningError, "suspended account %v may not submit trade orders", user)
}

tunnel, assets, sell, rpcErr := r.extractMarketDetails(&market.Prefix, &market.Trade)
Expand Down
2 changes: 1 addition & 1 deletion server/matcher/match.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ func (m *Matcher) Match(book Booker, queue []*OrderRevealed) (seed []byte, match
removed, ok := book.Remove(o.TargetOrderID)
if !ok {
// The targeted order might be down queue or non-existent.
log.Debugf("Failed to remove order %v set by a cancel order %v",
log.Debugf("Order %v not removed by a cancel order %v (target either non-existent or down queue in this epoch)",
Copy link
Member

Choose a reason for hiding this comment

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

What does down queue in this context mean? Could the target order be booked at this point but not matched to this cancel order?

Copy link
Member Author

@chappjc chappjc Sep 21, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Down queue means after shuffle of an epoch queue that contains both a trade and a cancel order targeting that trade, the target order would get processed after the cancel order, so the cancel order would not be able to cancel it since it hasn't been booked yet.

o.ID(), o.TargetOrderID)
failed = append(failed, q)
updates.CancelsFailed = append(updates.CancelsFailed, o)
Expand Down
Loading