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

udb: Track ticket commitments for balance purposes #1330

Merged
merged 2 commits into from
Jun 13, 2019

Conversation

matheusd
Copy link
Member

@matheusd matheusd commented Nov 22, 2018

This will perform a database migration into v12. Therefore, if you want to go back to old versions, be sure to backup the wallet dirs.

Close #883

This adds a ticket commitment bucket to the database to separately track wallet-owned commitments, such that the locked balance can be correctly calculated in situations where the wallet does not have voting authority over the ticket. This addresses:

  • Solo voters that haven't imported the keys of their voting wallet into their funding wallet
  • VSP voters, where the pool fee is now correctly discounted (given they are not expected to have control over the pool fee key anymore)
  • Split Ticket voters (where the total balance does not depend on the ticket submission anymore, but on the lockedByTickets balance)
  • Split Ticket non-voters (same situation of solo voters that didn't have the voting rights)

This is a big change in how balances for tickets are tracked in wallets, so while I tried testing all relevant cases (reorgs, dropping out of tickets due to changing sdiff, etc) I expect some iteration and careful review will be needed.

At least the first commit (the v11 testdata generator) needs to be separately merged due to having to be run before the upgrade is merged, so let me know whether I should create a separate PR only for that commit or if you'll merge this without squashing.

Outstanding issues for discussion:

  • Should I care about the minConf parameter for ticket commitments when calculating the balance?

- I'm not 100% sold on using a single bucket for tracking the commitments (due to this causing churning on the database during a rescan). Maybe I should use the same strategy as the credits/unspent tracking and use one bucket for always storing the commitment and a second one for storing the unredeemed commitments)? (Modified to use two buckets, one for storing the commitment info, one as index of unspent commitments)

@matheusd
Copy link
Member Author

Updated to use two buckets to reduce churning during rescans: one to track commitments and one as index of unspent commitments.

wallet/udb/txmined.go Outdated Show resolved Hide resolved
wallet/udb/txmined.go Outdated Show resolved Hide resolved
wallet/udb/txmined.go Outdated Show resolved Hide resolved
wallet/udb/txmined.go Outdated Show resolved Hide resolved
wallet/udb/upgrades.go Outdated Show resolved Hide resolved
@matheusd
Copy link
Member Author

Gofmt all files and fixed the commenting nits.

I'll squash, rebase and (if needed) regenerate the v11 test database once this is considered ready for merging.

This adds the testdata routines needed to generate a udb v11 database
for use with the v12 upgrade tests.

This is supposed to be run on the commit immediately before the v12
upgrade is introduced.
This commit changes the handling of ticket commitments so that the
wallet can correctly account for its share in tickets instead of always
considering it has access to the full funds.

It aims to fix balance issues by decoding the commitment addresses in
tickets processed by the wallet and individually tracking those only if
the address belong to the wallet.

This introduces database changes and therefore upgrades the wallet udb
to version 12.
@jrick jrick merged commit 5a7d096 into decred:master Jun 13, 2019
@matheusd matheusd deleted the track-sstx-commitments branch January 22, 2020 11:24
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

Successfully merging this pull request may close these issues.

Ticket buying wallets should have a lockedbytickets balance (if they own the commitment addresses)
2 participants