Add an interface for discarding some ZKAPs

Fixes #235

As I was implementing this, I did realize that this still doesn't provide a full backup. "Spent" is one terminal state for unblinded tokens but it isn't the only one. "Some spending error" is also possible. The checkpoint scheme here will not keep such tokens backed up. Only the full copy of the database, done after each purchase, will cause that state to be backed up.

In other words, if some tokens cannot be spent for some reason, that reason will be lost unless a purchase-triggered full backup happens after the fact.

There are similar issues with other non-unblinded-token state. The lease maintenance spending table will not be kept up to date by the checkpoint either, for example.

Despite that, this is still a small incremental improvement (it allows a third-party like GridSync to at least back up the whole database sometimes whereas currently it never does) so I still think it should land. I'm filing other tickets to get us all the way to a good backup scheme.

Here's the other part @crwood

Created by: codecov[bot]

Codecov Report

Merging #237 (a0c189e) into main (ceddcbe8) will increase coverage by 0.02%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #237      +/-   ##
==========================================
+ Coverage   96.97%   96.99%   +0.02%     
==========================================
  Files          49       49              
  Lines        3999     4026      +27     
  Branches      510      510              
==========================================
+ Hits         3878     3905      +27     
  Misses         79       79              
  Partials       42       42              

Impacted Files	Coverage Δ
src/_zkapauthorizer/model.py	95.81% <100.00%> (+0.07%)
src/_zkapauthorizer/resource.py	100.00% <100.00%> (ø)
src/_zkapauthorizer/tests/test_client_resource.py	98.11% <100.00%> (+0.07%)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ceddcbe...a0c189e. Read the comment docs.

Created by: tomprince

Despite that, this is still a small incremental improvement (it allows a third-party like GridSync to at least back up the whole database sometimes whereas currently it never does) so I still think it should land. I'm filing other tickets to get us all the way to a good backup scheme.

In reviewing this, I was wondering if this is an incremental improvement. As mentioned in #237, it is currently possible to backup the entire database. And restoring that database works, and ZKAPAuthorizer will deal correctly with that database, though it may take some time to work through the backlog of spent tickets.

If I remember the code correctly, currently, the only tokens that are deleted from the database are ones that have definitely been spent. All others (either unspent, or ones that failed to be spent) are kept in the database. With this change, we will start unrecoverably deleting unspent tokens that had some sort of failure.

I think it would probably be more prudent to mark discard passes as such, rather than deleting them. That would at least let us recover the passes in the future, if it turns out we were incorrectly invalidating them.

From #236:

ZKAPAuthorizer spends ZKAPs in a deterministic order.

I'm curious if this is something we want to guarantee? Things that occur to me are having multiple requests in flight, or (in the future) having some sort of retryable error spending tokens. That said, as long as we:

1. always return the first unspent token in the database
2. never reorder the tokens in the database

then discarding all tokens up to the first unspent will not discard any unspent tokens. Thus, even if the guaranteed isn't maintained, the behavior may be inefficient, but not incorrect.

That said, it looks like we depend on sqlite maintaining a consistent order, which I'm not sure is the case. Looking at the docs on autoincrement and vacuuming suggests that we don't currently have a guarantee about that. I think we'd need to add a column, to ensure that we maintain an ordering.

Created by: tomprince

Review: Changes requested

I think the code here looks fine, for what it is aiming to do, but as mentioned in https://github.com/PrivateStorageio/ZKAPAuthorizer/pull/237#issuecomment-946142191, I think we should preserve the discarded tokens, rather that delete them outright.

---

Also, I notice that the database doesn't track any kind of notion of time, for any of the tables. Thinking from our side, thinking about about what we might want to do if we do need to recover some incorrectly discarded or invalidated tokens, I could imagine wanting some notion of when the tokens were invalidated or discarded (though I guess when discarded isn't particularly meaningful). That said, adding dates is definitely out-of-scope for this PR. (I was thinking that dates might also be useful if we want to provide someway to refresh tokens after a key-rotation on our part, but I guess an associated public-key would be more useful for that)

Based on @tomprince 's excellent points above, this line of development doesn't seem worth pursuing. We'll come up with a different plan to support improved durability of the internal state.