dcr: Allow ticket purchasing for rpc spv wallets #2756
Conversation
JoeGruffins
force-pushed
the
suppressstakestatus
branch
from
cdfa4ea
to
ea5b703
Compare
|
I guess the rpcwallet also needs to be a |
JoeGruffins
force-pushed
the
suppressstakestatus
branch
5 times, most recently
from
a6713e3
to
b7484aa
Compare
| var _ ticketPager = (*rpcWallet)(nil) | ||
|
|
||
| func (w *rpcWallet) TicketPage(ctx context.Context, scanStart int32, n, skipN int) ([]*asset.Ticket, error) { | ||
| return make([]*asset.Ticket, 0), nil | ||
| } |
There was a problem hiding this comment.
Without this there will be an error:
2024-05-07 08:50:36.970 [ERR] WEB: error retrieving ticket page for 42: ticket pagination not supported for this wallet
But it doesn't look like the pager is needed. Should this error be silence somewhere else?
Also the statuses are all unknown, which I think is a separate issue.
There was a problem hiding this comment.
Without TicketPage will it show the tickets that have already voted?
Also the statuses are all unknown, which I think is a separate issue.
The native wallet doesn't show undefined.
There was a problem hiding this comment.
I guess we should consider using the tx history db for paginating ticket history. At least for spv wallet but maybe for all. No need to do it in this PR though. Sounds like a bit of work.
There was a problem hiding this comment.
Without TicketPage will it show the tickets that have already voted?
The already voted tickets always seem to go away. Without being a TicketPage you can't view any pages with under 10 tickets.
client/asset/dcr/rpcwallet.go
Outdated
| w.ticketCache.Lock() | ||
| defer w.ticketCache.Unlock() | ||
| if w.ticketCache.stamp.Add(ticketCacheExpiry).After(time.Now()) { | ||
| return w.ticketCache.tickets, nil | ||
| } |
There was a problem hiding this comment.
Sometimes this was returning tickets then none if called close together. I guess a cache is alright to have?
Maybe the cache can be made more intelligent to get statuses more correct.
|
This looks ok for purchasing tickets but viewing them afterwards seems to have a lot of problems still. |
client/asset/dcr/rpcwallet.go
Outdated
| func (w *rpcWallet) tickets(ctx context.Context, includeImmature bool) ([]*asset.Ticket, error) { | ||
| w.ticketCache.Lock() | ||
| defer w.ticketCache.Unlock() | ||
| if w.ticketCache.stamp.Add(ticketCacheExpiry).After(time.Now()) { |
| if errors.Is(err, oldSPVWalletErr) { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
In this case the UI doesn't show any information that the current version of dcrwallet is too low. Might be helpful to show that.
There was a problem hiding this comment.
Is there a reasonably easy way to do that? I don't expect the number of rpc+spv users will be high. If it's more that a few lines of work to properly communicate this message to the front end, I'm okay with how you have it, @JoeGruffins.
| var _ ticketPager = (*rpcWallet)(nil) | ||
|
|
||
| func (w *rpcWallet) TicketPage(ctx context.Context, scanStart int32, n, skipN int) ([]*asset.Ticket, error) { | ||
| return make([]*asset.Ticket, 0), nil | ||
| } |
There was a problem hiding this comment.
Without TicketPage will it show the tickets that have already voted?
Also the statuses are all unknown, which I think is a separate issue.
The native wallet doesn't show undefined.
| if errors.Is(err, oldSPVWalletErr) { | ||
| return nil, nil | ||
| } |
There was a problem hiding this comment.
Is there a reasonably easy way to do that? I don't expect the number of rpc+spv users will be high. If it's more that a few lines of work to properly communicate this message to the front end, I'm okay with how you have it, @JoeGruffins.
client/asset/dcr/rpcwallet.go
Outdated
| // Calling w.rpcClient.GetTickets too close together will return no | ||
| // tickets for the second call. A ticket cache alleviates this. |
There was a problem hiding this comment.
I think I was wrong about this. Removing for now.
| var _ ticketPager = (*rpcWallet)(nil) | ||
|
|
||
| func (w *rpcWallet) TicketPage(ctx context.Context, scanStart int32, n, skipN int) ([]*asset.Ticket, error) { | ||
| return make([]*asset.Ticket, 0), nil | ||
| } |
There was a problem hiding this comment.
I guess we should consider using the tx history db for paginating ticket history. At least for spv wallet but maybe for all. No need to do it in this PR though. Sounds like a bit of work.
JoeGruffins
force-pushed
the
suppressstakestatus
branch
from
b7484aa
to
ed4c676
Compare
|
Configuring beta wallet with the vsp https://github.com/decred/dcrdex/compare/b7484aa8719d11b1c97285d7410a58c035b7971a..ed4c676d50d7467941a233d26a7ba9664d3700f4 |
JoeGruffins
force-pushed
the
suppressstakestatus
branch
from
ed4c676
to
916f249
Compare
|
Looking again today I'm not seeing blank results. Removing the cache for now... |
JoeGruffins
force-pushed
the
suppressstakestatus
branch
3 times, most recently
from
65739da
to
a8d7dbf
Compare
|
@buck54321 updated the dcrwallet version to latest |
JoeGruffins
force-pushed
the
suppressstakestatus
branch
from
a8d7dbf
to
ced0579
Compare
|
Just rebased. |
JoeGruffins
force-pushed
the
suppressstakestatus
branch
from
ced0579
to
e4a9b00
Compare
|
Removed the interface methods and updated vspd version in the harness https://github.com/decred/dcrdex/compare/ced0579ac849baf5dd086af4df2ca36d796ea7a3..e4a9b00143d071d040ac282750e4031696aeabc4 |
closes #2611
Testers must update their system's
vspdto latest or it will panic in the harness.