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

client/core,eth: Lock funds for refund #1479

Merged
merged 5 commits into from
Mar 5, 2022
Merged

client/core,eth: Lock funds for refund #1479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e0d31a1
core/eth: Lock funds for refund
martonp Feb 19, 2022
1219455
Fixes based on review
martonp Feb 22, 2022
6c8fcc1
Make eth fee rater and use server max fee rate for redemptions and re…
martonp Feb 26, 2022
8549c5e
Updates based on review
martonp Mar 4, 2022
bdb6960
Unlock reserves for ImmediateTiF and other minor updates
martonp Mar 5, 2022
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
284 changes: 184 additions & 100 deletions client/asset/eth/eth.go
View file
Open in desktop

Large diffs are not rendered by default.

289 changes: 215 additions & 74 deletions client/asset/eth/eth_test.go
View file
Open in desktop

Large diffs are not rendered by default.

40 changes: 27 additions & 13 deletions client/asset/interface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,20 +352,34 @@ type TokenMaster interface {
OpenTokenWallet(assetID uint32, settings map[string]string, tipChange func(error)) (Wallet, error)
}

// AccountRedeemer is a wallet in which redemptions require a wallet to have
// available balance to pay fees.
type AccountRedeemer interface {
// ReserveN is used when preparing funding for an order that redeems to an
// account-based asset. The wallet will set aside the appropriate amount of
// funds so that we can redeem. It is an error to request funds > spendable
// balance.
ReserveN(n, feeRate uint64, assetVer uint32) (uint64, error)
// ReReserve is used when reconstructing existing orders on startup. It is
// AccountLocker is a wallet in which redemptions and refunds require a wallet
// to have available balance to pay fees.
type AccountLocker interface {
// ReserveNRedemption is used when preparing funding for an order that
// redeems to an account-based asset. The wallet will set aside the
// appropriate amount of funds so that we can redeem N swaps on the
// specified version of the asset, at the specified fee rate. It is an
// error to request funds > spendable balance.
ReserveNRedemptions(n, feeRate uint64, assetVer uint32) (uint64, error)
// ReReserveRedemption is used when reconstructing existing orders on
// startup. It is an error to request funds > spendable balance.
ReReserveRedemption(amt uint64) error
// UnlockRedemptionReserves is used to return funds reserved for redemption
// when an order is canceled or otherwise completed unfilled.
UnlockRedemptionReserves(uint64)
// ReserveNRefunds is used when preparing funding for an order that
// refunds to an account-based asset. The wallet will set aside the
// appropriate amount of funds so that we can refund N swaps on the
// specified version of the asset, at the specified fee rate. It is
// an error to request funds > spendable balance.
ReReserve(amt uint64) error
// UnlockReserves is used to return funds when an order is canceled or
// otherwise completed unfilled.
UnlockReserves(uint64)
ReserveNRefunds(n, feeRate uint64, assetVer uint32) (uint64, error)
Copy link
Member

Choose a reason for hiding this comment

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

This method would be unneeded if we lock up the refund reserves in FundOrder.

// ReReserveRefund is used when reconstructing existing orders on
// startup. It is an error to request funds > spendable balance.
ReReserveRefund(uint64) error
// UnlockRefundReserves is used to return funds reserved for refunds
// when an order was cancelled or revoked before a swap was initiated,
// completed successully, or after a refund was done.
UnlockRefundReserves(uint64)
}

// Balance is categorized information about a wallet's balance.
Expand Down
56 changes: 44 additions & 12 deletions client/core/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -810,8 +810,10 @@ func (dc *dexConnection) reconcileTrades(srvOrderStatuses []*msgjson.OrderStatus
(!trade.isMarketBuy() || len(trade.matches) == 0) {
if trade.isMarketBuy() {
trade.unlockRedemptionFraction(1, 1)
trade.unlockRefundFraction(1, 1)
} else {
trade.unlockRedemptionFraction(trade.Trade().Remaining(), trade.Trade().Quantity)
trade.unlockRefundFraction(trade.Trade().Remaining(), trade.Trade().Quantity)
}
}
// Now update the trade.
Expand Down Expand Up @@ -4007,7 +4009,8 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e
}
fromWallet, toWallet := wallets.fromWallet, wallets.toWallet

accountRedeemer, isAccountRedemption := toWallet.Wallet.(asset.AccountRedeemer)
accountRedeemer, isAccountRedemption := toWallet.Wallet.(asset.AccountLocker)
accountRefunder, isAccountRefund := fromWallet.Wallet.(asset.AccountLocker)

prepareWallet := func(w *xcWallet) error {
// NOTE: If the wallet is already internally unlocked (the decrypted
Expand Down Expand Up @@ -4054,7 +4057,7 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e
if form.IsLimit && !form.Sell {
fundQty = calc.BaseToQuote(rate, fundQty)
}
redemptionLots := lots
redemptionRefundLots := lots

isImmediate := (!form.IsLimit || form.TifNow)

Expand All @@ -4077,7 +4080,7 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e
if err == nil {
baseQty := calc.QuoteToBase(midGap, fundQty)
lots = baseQty / lotSize
redemptionLots = lots * marketBuyRedemptionSlippageBuffer
redemptionRefundLots = lots * marketBuyRedemptionSlippageBuffer
if lots == 0 {
err = newError(orderParamsErr,
"order quantity is too low for current market rates. "+
Expand Down Expand Up @@ -4192,7 +4195,7 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e
// Everything is ready. Send the order.
route, msgOrder, msgTrade := messageOrder(ord, msgCoins)

// If the to asset is an AccountRedeemer, we need to lock up redemption
// If the to asset is an AccountLocker, we need to lock up redemption
// funds.
var redemptionReserves uint64
if isAccountRedemption {
Expand All @@ -4203,17 +4206,31 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e
if len(pubKeys) == 0 || len(sigs) == 0 {
return nil, 0, newError(signatureErr, "wrong number of pubkeys or signatures, %d & %d", len(pubKeys), len(sigs))
}
redemptionReserves, err = accountRedeemer.ReserveN(redemptionLots, wallets.toAsset.MaxFeeRate, wallets.toAsset.Version)
redemptionReserves, err = accountRedeemer.ReserveNRedemptions(redemptionRefundLots, wallets.toAsset.MaxFeeRate, wallets.toAsset.Version)
Copy link
Member

Choose a reason for hiding this comment

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

Works for me. Only additional thought about this matter is that the rate given to ReserveNRedemptions and ReserveNRefunds could be the larger of MaxFeeRate and the rate from FeeRate (if also a FeeRater). That would be pretty screwy if the server had a rate set that low, so whatever, let's not change anything now. This PR looks good to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the swap cannot be initiated, then it doesn't matter if too little is reserved for refunds and redemptions.

Copy link
Member

@chappjc chappjc Feb 28, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

The swap tx could still be mined with a gas fee cap less than what FeeRate would recommend. It would just be dicey for the refund (that happens in several hours). For the redeem, you're redeeming the other person's swap, so who knows what they used.

if err != nil {
return nil, 0, codedError(walletErr, fmt.Errorf("ReserveN error: %w", err))
return nil, 0, codedError(walletErr, fmt.Errorf("ReserveNRedemptions error: %w", err))
}
msgTrade.RedeemSig = &msgjson.RedeemSig{
PubKey: pubKeys[0],
Sig: sigs[0],
}
defer func() {
if !success {
accountRedeemer.UnlockReserves(redemptionReserves)
accountRedeemer.UnlockRedemptionReserves(redemptionReserves)
}
}()
}

// If the from asset is an AccountLocker, we need to lock up refund funds.
var refundReserves uint64
if isAccountRefund {
refundReserves, err = accountRefunder.ReserveNRefunds(redemptionRefundLots, wallets.fromAsset.MaxFeeRate, wallets.fromAsset.Version)
if err != nil {
return nil, 0, codedError(walletErr, fmt.Errorf("ReserveNRefunds error: %w", err))
}
defer func() {
if !success {
accountRefunder.UnlockRefundReserves(refundReserves)
}
}()
}
Expand Down Expand Up @@ -4259,9 +4276,10 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e
// Store the order.
dbOrder := &db.MetaOrder{
MetaData: &db.OrderMetaData{
Status: order.OrderStatusEpoch,
Host: dc.acct.host,
MaxFeeRate: wallets.fromAsset.MaxFeeRate,
Status: order.OrderStatusEpoch,
Host: dc.acct.host,
MaxFeeRate: wallets.fromAsset.MaxFeeRate,
RedeemMaxFeeRate: wallets.toAsset.MaxFeeRate,
Proof: db.OrderProof{
DEXSig: result.Sig,
Preimage: preImg[:],
Expand All @@ -4282,9 +4300,10 @@ func (c *Core) prepareTrackedTrade(dc *dexConnection, form *TradeForm, crypter e

// Prepare and store the tracker and get the core.Order to return.
tracker := newTrackedTrade(dbOrder, preImg, dc, dc.marketEpochDuration(mktID), c.lockTimeTaker, c.lockTimeMaker,
c.db, c.latencyQ, wallets, coins, c.notify, c.formatDetails, form.Options, redemptionReserves)
c.db, c.latencyQ, wallets, coins, c.notify, c.formatDetails, form.Options, redemptionReserves, refundReserves)

tracker.redemptionLocked = tracker.redemptionReserves
Copy link
Member

Choose a reason for hiding this comment

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

You've got another one of these three lines down.

tracker.refundLocked = tracker.refundReserves

if recoveryCoin != nil {
tracker.change = recoveryCoin
Expand Down Expand Up @@ -4897,7 +4916,7 @@ func (c *Core) dbTrackers(dc *dexConnection) (map[order.OrderID]*trackedTrade, e
copy(preImg[:], dbOrder.MetaData.Proof.Preimage)
tracker := newTrackedTrade(dbOrder, preImg, dc, dc.marketEpochDuration(mktID), c.lockTimeTaker,
c.lockTimeMaker, c.db, c.latencyQ, nil, nil, c.notify, c.formatDetails,
dbOrder.MetaData.Options, dbOrder.MetaData.RedemptionReserves)
dbOrder.MetaData.Options, dbOrder.MetaData.RedemptionReserves, dbOrder.MetaData.RefundReserves)
trackers[dbOrder.Order.ID()] = tracker

// Get matches.
Expand Down Expand Up @@ -5106,6 +5125,14 @@ func (c *Core) resumeTrades(dc *dexConnection, trackers []*trackedTrade) assetMa
}
}

lockMatchRefund := func(match *matchTracker) {
if tracker.isMarketBuy() {
tracker.lockRefundFraction(1, uint64(len(tracker.matches)))
} else {
tracker.lockRefundFraction(match.Quantity, trade.Quantity)
}
}

martonp marked this conversation as resolved.
Show resolved Hide resolved
// If matches haven't redeemed, but the counter-swap has been received,
// reload the audit info.
isActive := tracker.metaData.Status == order.OrderStatusBooked || tracker.metaData.Status == order.OrderStatusEpoch
Expand All @@ -5123,6 +5150,7 @@ func (c *Core) resumeTrades(dc *dexConnection, trackers []*trackedTrade) assetMa
}
if match.Status < order.MakerRedeemed {
lockMatchRedemption(match)
lockMatchRefund(match)
}
} else { // Taker
if match.Status < order.TakerSwapCast {
Expand All @@ -5132,6 +5160,9 @@ func (c *Core) resumeTrades(dc *dexConnection, trackers []*trackedTrade) assetMa
needsAuditInfo = true // taker needs AuditInfo for maker's contract
counterSwap = match.MetaData.Proof.MakerSwap
}
if match.Status < order.MakerRedeemed {
lockMatchRefund(match)
}
if match.Status < order.MatchComplete {
lockMatchRedemption(match)
}
Expand Down Expand Up @@ -5250,6 +5281,7 @@ func (c *Core) resumeTrades(dc *dexConnection, trackers []*trackedTrade) assetMa

if isActive {
tracker.lockRedemptionFraction(trade.Remaining(), trade.Quantity)
tracker.lockRefundFraction(trade.Remaining(), trade.Quantity)
}

// Balances should be updated for any orders with locked wallet coins,
Expand Down
Loading