multi: Select managed tickets outside VSP client

[jholdstock]

The first two commits are bug fixes which I feel warrant review individually.

The third is simply moving some logic around following precedent set in #2257

[jrick]

I'm unsure about this change. The non-nil non-NotExist errors are not being unhandled; they are still logged.

If we return an error to the caller (ForUnspentUnexpiredTickets), now it will error out earlier and not loop over all of the unspent tickets, which doesn't seem to be what we would want to do even if we did propagate errors up to the caller of ProcessManagedTickets.

[jrick]

collecting the errors here and then doing a final return of them with errors.Join seems like the best way to handle this, but we'll need to wait until go 1.21 at least, and drop support for go <=1.19, because that was introduced in go 1.20.

[jholdstock]

I've pushed a commit which approximates the errors.Join behaviour

[jholdstock]

I realise I have almost exactly duplicated logic which already existed here:

dcrwallet/internal/rpc/jsonrpc/methods.go

Lines 4677 to 4703 in bfed19f

	var firstErr error
	err := w.ForUnspentUnexpiredTickets(ctx, func(hash *chainhash.Hash) error {
	vspHost, err := w.VSPHostForTicket(ctx, hash)
	if err != nil && firstErr == nil {
	if errors.Is(err, errors.NotExist) {
	// Ticket is not registered with a VSP, nothing more to do here.
	return nil
	}
	firstErr = err
	return nil
	}
	vspClient, err := loader.LookupVSP(vspHost)
	if err != nil && firstErr == nil {
	firstErr = err
	return nil
	}
	// Never return errors here, so all tickets are tried.
	// The first error will be returned to the user.
	err = vspClient.SetVoteChoice(ctx, hash, choices, tspendPolicy, treasuryPolicy)
	if err != nil && firstErr == nil {
	firstErr = err
	}
	return nil
	})
	if err != nil {
	return err
	}

What do you think about moving this logic down into ForUnspentUnexpiredTickets itself? So that func will always iterate over all tickets, and all callers get the benefit of that behaviour for free without having to reimplement it.

[jrick]

makes sense, i like it.

i don't like your errors.Join replacement though, since it breaks error detection when using errors.As. We should either use the real errors.Join here (wrapping it under our own errors package) or just yoink the stdlib code directly into our errors.

[jrick]

#2261

[jholdstock]

Please review #2266 before proceeding with this PR, then I will rebase this one.

[jrick]

the second/middle commit message needs to be wrapped.