decred / dcrwallet

A secure Decred wallet daemon written in Go (golang).
https://decred.org
ISC License
219 stars 155 forks source link

Track ticket spender directly instead of using output 0 credit checking #1366

Open matheusd opened 5 years ago

matheusd commented 5 years ago

Summary

The currently provided endpoints for gathering information about the wallet's tickets currently detect the spent status of a ticket by checking whether there is a spent credit for the ticket's submission output. This has a few drawbacks and we should change the db internals to track spent status directly.

Current Approach & Drawbacks

The grpc endpoints GetTicket/GetTickets eventually resolve into a call to txStore.TicketDetails.

This call verifies whether the transaction really is a ticket then checks whether the wallet has a credit (or unmined spent) for the ticket's submission output in order to find out the corresponding spender transaction (vote or revocation) for the given ticket.

This implementation works when the wallet is either a funding+hot voting wallet or is using VSP voting with a correctly imported script.

However, when the wallet does not have the voting rights for the ticket, the call fails to identify the corresponding spender transaction, even though it's very likely that the wallet does have the transaction recorded in the transaction store (eg: due to having a returning output in the vote/revocation).

This situation happens on solo voters setups, with a separate hot voting wallet, in split ticket transactions, when the wallet is not the selected voter or when the wallet has been restored from seed but the voting script has not yet been imported.

The main problem with not having the correct tx relationship information is that it makes it harder to present to users the correct view for a given ticket. For example, when calling GetTicket([hash]), the ticket's status might be "live", "missed", "expired" or "unknown" depending on the wallet's sync mode and other considerations, even though the wallet might have recorded a corresponding vote transaction.

There is also a subtle bug in the current implementation that I verified in simnet and can provide more details if needed: due to also checking for unmined inputs, when the wallet doesn't have the voting rights for the ticket, the spender information actually regresses for an online wallet after the vote/revocation is mined. Essentially:

Propposed Change

My idea is to change the db to track the spender transaction of a ticket directly (instead of indirectly via the credit 0 spender).

This would be roughly the process for implementing this change:

Let me know if you believe this is a reasonable approach and I can make a first attempt at fixing this.

jrick commented 5 years ago

I would like to continue using as much of the existing spend tracking as there is, since this can get very complicated to implement correctly across reorganizations and replacement transactions (consider a vote created but not mined, and replaced by a revocation later). Perhaps an alternative would be to modify the credit structures in a way to perform spend tracking on ticket output zero but without the implication that the wallet has voting rights for the ticket.

matheusd commented 5 years ago

In that case, how about: add a new flag (0x80 "Non-wallet controlled credit"), then modify transaction processing such that we always add output 0 as a credit (with the correct flag as appropriate). Spent status tracking would probably remain as is, without any changes. Just need to take care to not add an unspent amount for non-wallet controlled tickets, otherwise that would mess up the balances (and the flag needs to be accounted for when calculating the voting authority balance).

This would be a simple change to do on top of #1330 and wouldn't even require further db migrations, given it's only a new flag value.