Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
matheusd opened this issue Dec 28, 2018 · 2 comments
Open

Comments

@matheusd
Copy link
Member

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:

  • Ticket is mined and matures, but wallet does not have the credit for the ticket's output 0
  • Ticket is selected to vote and a (different) hot voting wallet publishes the vote
  • The funding wallet detects the unmined vote and adds a rawUnminedInput. Requesting the GetTicket call for the ticket in this stage correctly lists the spender transaction
  • Vote is mined in the next block. The wallet receives the new block and deletes the rawUnminedInput (but since it didn't have the original credit, it doesn't store the spend information). Requesting the GetTicket call in this stage does not list the spender transaction

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:

  • Change the ticket record bucket to store the spender transaction hash
  • Adjust the transaction processing logic ({Insert,Remove}{Mined,Unmined}) to correctly record the spender transaction
  • Change the TicketDetails function to use the tickets bucket to figure out if the ticket is spent or not.
  • Implement a db migration and tests to account for the new db layout

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

@jrick
Copy link
Member

jrick commented Dec 31, 2018

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
Copy link
Member Author

matheusd commented Jan 2, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants