Add param to prevent overpriced ticket purchases #2461
Conversation
|
Can anyone review or comment on this for me? |
| @@ -238,6 +238,7 @@ | |||
|
|
|||
| ; Amount of funds to keep in wallet when stake mining | |||
| ; ticketbuyer.balancetomaintainabsolute=0 | |||
| ; ticketbuyer.maxticketprice=0 | |||
There was a problem hiding this comment.
Every other item in this file has a comment - the description from config.go.
| @@ -65,6 +65,8 @@ const ( | |||
| // ticket buyer options | |||
| defaultBalanceToMaintainAbsolute = 0 | |||
| defaultTicketbuyerLimit = 1 | |||
| // If max ticket price is set to 0, purchase will still be made even if ticket price is too high | |||
There was a problem hiding this comment.
None of the other default values have comments, so this one seems a bit out of place.
Perhaps we move it to the description tag in the config struct? Then it will appear in the output of dcrwallet --help which is useful for end-users.
| @@ -187,6 +189,7 @@ type ticketBuyerOptions struct { | |||
| BalanceToMaintainAbsolute *cfgutil.AmountFlag `long:"balancetomaintainabsolute" description:"Amount of funds to keep in wallet when purchasing tickets"` | |||
| Limit uint `long:"limit" description:"Buy no more than specified number of tickets per block"` | |||
| VotingAccount string `long:"votingaccount" description:"Account used to derive addresses specifying voting rights"` | |||
| MaxTicketPrice *cfgutil.AmountFlag `long:"maxticketprice" description:"Don't buy when the price is too high"` | |||
There was a problem hiding this comment.
| MaxTicketPrice *cfgutil.AmountFlag `long:"maxticketprice" description:"Don't buy when the price is too high"` | |
| MaxTicketPrice *cfgutil.AmountFlag `long:"maxticketprice" description:"Don't buy tickets when the price is above this limit; no limit if unset or zero"` |
| @@ -1204,6 +1204,11 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op, | |||
| return nil, err | |||
| } | |||
|
|
|||
| // If the ticket price exceeds the max ticket price, skipping purchase | |||
| if req.MaxTicketPrice > 0 && ticketPrice > req.MaxTicketPrice { | |||
| return nil, errors.E(op, errors.Invalid, "Skipping purchase: Ticket price exceeds max ticket price.") | |||
There was a problem hiding this comment.
"Error strings should not be capitalized (unless beginning with an exported name, a proper noun or an acronym) and should not end with punctuation."
https://google.github.io/styleguide/go/decisions.html#error-strings
| @@ -1204,6 +1204,11 @@ func (w *Wallet) purchaseTickets(ctx context.Context, op errors.Op, | |||
| return nil, err | |||
| } | |||
|
|
|||
| // If the ticket price exceeds the max ticket price, skipping purchase | |||
| if req.MaxTicketPrice > 0 && ticketPrice > req.MaxTicketPrice { | |||
There was a problem hiding this comment.
Given that the new config item is a TicketBuyer config, I don't think this check should be buried deep in the generic purchaseTickets func, it should really live in the ticketbuyer code. If the ticket price is too high, ticketbuyer should not be calling purchaseTickets at all. For reference, see how and where the check for balance to maintain is handled.
|
I don't think this feature should exist at all. If you want more control over what price you are willing to buy tickets at, buy them manually. See discussion in linked issue. |
|
to further elaborate: because you implemented this as a config setting, there is no way to change it at runtime without restarting the wallet with a different config. the ticket price, if stake participation remains a constant percent of the total supply, is expected to gradually increase. it doesn't make sense to put a maximum purchase value into the autobuyer for this reason alone. if stake participation does increase, then the maximum value you set may prevent you from purchasing when you, in retrospect, would have wanted the purchases to go through. if stake participation decreases, then any delta over the steady state price that you consider "overpriced" would no longer be relevant, because the overall demand has also dropped. the autobuyer is dumb on purpose. it exists to "DCA" at any price, high or low. the complexity needed to address all three concerns above is not something we want to maintain. you have the option to buy manually instead. use it. |
Related issue: #2455
Add param maxticketprice
Add the parameter maxticketprice to enable control over halting ticket purchases when ticket prices are excessively high
The parameter can be configured in the configuration file with the name: ticketbuyer.maxticketprice
If value = 0 (default value): The system will proceed with ticket purchases as it did previously
if value < 0: param error
if value > 0: The system will cease ticket purchases if the ticket price surpasses the specified threshold