dcrdex icon indicating copy to clipboard operation
dcrdex copied to clipboard

client: Check server config values are not too low.

Open JoeGruffins opened this issue 2 years ago • 3 comments

Some server config values are blindly trusted by the client when they could be detrimental to the user if too low.

They are swapConf and maxFeeRate and to a lesser extent lotSize and rateStep.

swapConf can be set to zero. This is sort of ok for utxo assets but no good for account assets as transactions are very easy to replace and our contracts do not work without confirmations.

maxFeeRate can be set too low causing swaps to fail, and possibly redeems if things align, since it is a fallback.

To a lesser extent, lotSize and rateStep being too small can cause swaps to fail due to our own code checking for dust. Related to #1610

JoeGruffins avatar Oct 31 '22 06:10 JoeGruffins

Closely related, mainly pertaining to the swapConf values: https://github.com/decred/dcrdex/issues/1900 A global sanity check for swapConf > 0 is reasonable in addition to user settings for the upper and lower bounds.

While we have feeRateLimit user setting as the upper bound for the acceptable maxFeeRate, a lower bound can also be defined. That value can be a user setting too, but an internal threshold enforced by the wallet seems warranted.

Strict internal lower bounds enforced internally to the wallet also make sense for lotSize to prevent dust swaps. I'm not sure about rateStep though. Also not sure if user-configurable settings for these two make sense since you are looking right at these values when placing an orders, so just internal wallet limits are probably good for these.

chappjc avatar Oct 31 '22 13:10 chappjc

Some ideas for the minimums, for both confs and fee rate:

  • Core can enforce > 0 for both, at least for non-token assets.
  • wallet (internally) can enforce some lowest allowable value for that asset. For confs probably just 1, and for fee rate, probably the network's min relay fee.
  • User setting. For fee rate, we're probably ok with just the existing max setting to prevent burning funds, but a min fee setting is conceivable although less useful. For swapConf, there could be just a single "swap confirmations override" that acts as both a minimum and an override, because it is OK to swap sooner than the server's required confs. e.g. If server says "2", and this value is set to "3", the market would be disabled for the user. If server says "2" and this value is "1", market is usable and the client swaps or redeems at 1 conf.

Finally, to ensure we don't hit these limits after a match, like when trying to broadcast or redeem a swap, https://github.com/decred/dcrdex/issues/1900

maxFeeRate can be set too low causing swaps to fail, and possibly redeems if things align, since it is a fallback.

@JoeGruffins note that the issue goes beyond maxFeeRate, to whatever fee is prescribed at match time since it's generally lower than maxFeeRate.

chappjc avatar Oct 31 '22 13:10 chappjc

I'd like to work on this as it is partially included on https://github.com/decred/dcrdex/pull/1941

vctt94 avatar Jan 27 '23 17:01 vctt94