diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py index 5768b34bf8e9bf3e26aef80c027c6d589bf64e3d..142d626e40f928d78130a993c9b4949dcbd5ef1f 100644 --- a/src/_zkapauthorizer/controller.py +++ b/src/_zkapauthorizer/controller.py @@ -581,9 +581,9 @@ class PaymentController(object): redeeming a voucher, if no other count is given when the redemption is started. - :ivar dict[unicode, datetime] _active: A mapping from voucher identifiers - which currently have redemption attempts in progress to timestamps - when the attempt began. + :ivar dict[unicode, Redeeming] _active: A mapping from voucher identifiers + which currently have redemption attempts in progress to a + ``Redeeming`` state representing the attempt. :ivar dict[unicode, datetime] _error: A mapping from voucher identifiers which have recently failed with an unrecognized, transient error. @@ -664,18 +664,32 @@ class PaymentController(object): This will not persist the voucher or random tokens but it will persist the result. """ + if not isinstance(voucher.state, model_Pending): + raise ValueError( + "Cannot redeem voucher in state {} instead of Pending.".format( + voucher.state, + ), + ) + # Ask the redeemer to do the real task of redemption. self._log.info("Redeeming random tokens for a voucher ({voucher}).", voucher=voucher) d = bracket( - lambda: setitem(self._active, voucher, self.store.now()), - lambda: delitem(self._active, voucher), - lambda: self.redeemer.redeemWithCounter(Voucher(voucher), counter, random_tokens), + lambda: setitem( + self._active, + voucher.number, + model_Redeeming( + started=self.store.now(), + counter=voucher.state.counter, + ), + ), + lambda: delitem(self._active, voucher.number), + lambda: self.redeemer.redeemWithCounter(voucher.number, counter, random_tokens), ) d.addCallbacks( - partial(self._redeemSuccess, voucher), - partial(self._redeemFailure, voucher), + partial(self._redeemSuccess, voucher.number), + partial(self._redeemFailure, voucher.number), ) - d.addErrback(partial(self._finalRedeemError, voucher)) + d.addErrback(partial(self._finalRedeemError, voucher.number)) return d def _get_random_tokens_for_voucher(self, voucher, num_tokens): @@ -713,7 +727,7 @@ class PaymentController(object): # we should skip any already-redeemed counter values. # # https://github.com/PrivateStorageio/ZKAPAuthorizer/issues/124 - return self._perform_redeem(voucher, 0, tokens) + return self._perform_redeem(self.store.get(voucher), 0, tokens) def _redeemSuccess(self, voucher, result): """ @@ -776,7 +790,7 @@ class PaymentController(object): if voucher.number in self._active: return attr.evolve( voucher, - state=model_Redeeming(started=self._active[voucher.number]), + state=self._active[voucher.number], ) if voucher.number in self._unpaid: return attr.evolve( diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py index 8615b43955dd64b54a7ea0b2827bb283f6fc02e7..fed05e1d489b12864512628bb60e01b0173b25b9 100644 --- a/src/_zkapauthorizer/model.py +++ b/src/_zkapauthorizer/model.py @@ -211,7 +211,7 @@ class VoucherStore(object): cursor.execute( """ SELECT - [number], [created], [state], [finished], [token-count], [public-key] + [number], [created], [state], [finished], [token-count], [public-key], [counter] FROM [vouchers] WHERE @@ -295,7 +295,7 @@ class VoucherStore(object): cursor.execute( """ SELECT - [number], [created], [state], [finished], [token-count], [public-key] + [number], [created], [state], [finished], [token-count], [public-key], [counter] FROM [vouchers] """, @@ -637,6 +637,20 @@ def has_length(expected): ) return validate_has_length +def greater_than(expected): + def validate_relation(inst, attr, value): + if value > expected: + return None + + raise ValueError( + "{name!r} must be greater than {expected}, instead it was {actual}".format( + name=attr.name, + expected=expected, + actual=value, + ), + ) + return validate_relation + @attr.s(frozen=True) class UnblindedToken(object): @@ -708,14 +722,32 @@ class RandomToken(object): ) +def _counter_attribute(): + return attr.ib( + validator=attr.validators.and_( + attr.validators.instance_of((int, long)), + greater_than(-1), + ), + ) + + @attr.s(frozen=True) class Pending(object): + """ + The voucher has not yet been completely redeemed for ZKAPs. + + :ivar int counter: The number of partial redemptions which have been + successfully performed for the voucher. + """ + counter = _counter_attribute() + def should_start_redemption(self): return True def to_json_v1(self): return { u"name": u"pending", + u"counter": self.counter, } @@ -727,6 +759,7 @@ class Redeeming(object): progress. """ started = attr.ib(validator=attr.validators.instance_of(datetime)) + counter = _counter_attribute() def should_start_redemption(self): return False @@ -821,7 +854,7 @@ class Error(object): } -@attr.s +@attr.s(frozen=True) class Voucher(object): """ :ivar unicode number: The text string which gives this voucher its @@ -845,13 +878,25 @@ class Voucher(object): default=None, validator=attr.validators.optional(attr.validators.instance_of(datetime)), ) - state = attr.ib(default=Pending()) + state = attr.ib( + default=Pending(counter=0), + validator=attr.validators.instance_of(( + Pending, + Redeeming, + Redeemed, + DoubleSpend, + Unpaid, + Error, + )), + ) @classmethod def from_row(cls, row): def state_from_row(state, row): if state == u"pending": - return Pending() + # TODO: The 0 here should be row[3] but I can't write a test + # to prove it yet. + return Pending(counter=0) if state == u"double-spend": return DoubleSpend( parse_datetime(row[0], delimiter=u" "), @@ -888,10 +933,11 @@ class Voucher(object): state_json = values[u"state"] state_name = state_json[u"name"] if state_name == u"pending": - state = Pending() + state = Pending(counter=state_json[u"counter"]) elif state_name == u"redeeming": state = Redeeming( started=parse_datetime(state_json[u"started"]), + counter=0, ) elif state_name == u"double-spend": state = DoubleSpend( diff --git a/src/_zkapauthorizer/schema.py b/src/_zkapauthorizer/schema.py index fc078c2a102f75decc5a21db4592468bf5e3f1e7..e3a37eb2023cfe890fb08a7500fdd519e9335bd0 100644 --- a/src/_zkapauthorizer/schema.py +++ b/src/_zkapauthorizer/schema.py @@ -131,4 +131,11 @@ _UPGRADES = { ALTER TABLE [vouchers] ADD COLUMN [public-key] text """, ], + + 2: [ + """ + -- Keep track of progress through redemption of each voucher. + ALTER TABLE [vouchers] ADD COLUMN [counter] integer DEFAULT 0 + """, + ], } diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index b648c5c5778e9d7d3d44572ecf5f05657e85c228..7776ff739b3ec50b88b37f8ecf1a307a7eba1318 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -310,10 +310,13 @@ def redeemed_states(): def voucher_states(): """ - Build unicode strings giving states a Voucher can be in. + Build Python objects representing states a Voucher can be in. """ return one_of( - just(Pending()), + builds( + Pending, + integers(min_value=0), + ), redeemed_states(), builds( DoubleSpend, diff --git a/src/_zkapauthorizer/tests/test_client_resource.py b/src/_zkapauthorizer/tests/test_client_resource.py index f6d2abde71ebfdb238ce37477f4f0af4f657e926..6c2089a999b53ae1a655b824de86424144c28752 100644 --- a/src/_zkapauthorizer/tests/test_client_resource.py +++ b/src/_zkapauthorizer/tests/test_client_resource.py @@ -795,6 +795,7 @@ class VoucherTests(TestCase): created=Equals(now), state=Equals(Redeeming( started=now, + counter=0, )), ), ) diff --git a/src/_zkapauthorizer/tests/test_controller.py b/src/_zkapauthorizer/tests/test_controller.py index 608ef72679232b29a3681b224d2eb61f98f2fe2e..9d97ce8b75054deac17e99272ad30ddbeba4996d 100644 --- a/src/_zkapauthorizer/tests/test_controller.py +++ b/src/_zkapauthorizer/tests/test_controller.py @@ -112,6 +112,7 @@ from ..controller import ( from ..model import ( UnblindedToken, Pending as model_Pending, + Redeeming as model_Redeeming, DoubleSpend as model_DoubleSpend, Redeemed as model_Redeemed, Unpaid as model_Unpaid, @@ -153,7 +154,30 @@ class PaymentControllerTests(TestCase): persisted_voucher = store.get(voucher) self.assertThat( persisted_voucher.state, - Equals(model_Pending()), + Equals(model_Pending(counter=0)), + ) + + @given(tahoe_configs(), datetimes(), vouchers()) + def test_redeeming(self, get_config, now, voucher): + """ + A ``Voucher`` is marked redeeming while ``IRedeemer.redeem`` is actively + working on redeeming it. + """ + store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store + controller = PaymentController( + store, + NonRedeemer(), + default_token_count=10, + ) + controller.redeem(voucher) + + controller_voucher = controller.get_voucher(voucher) + self.assertThat( + controller_voucher.state, + Equals(model_Redeeming( + started=now, + counter=0, + )), ) @given(tahoe_configs(), dummy_ristretto_keys(), datetimes(), vouchers()) diff --git a/src/_zkapauthorizer/tests/test_model.py b/src/_zkapauthorizer/tests/test_model.py index 27fbcab25f7d9597c28ee61372c080c86df6cf07..e6df76c1ad9921552af6abb0667f341d76f7c57c 100644 --- a/src/_zkapauthorizer/tests/test_model.py +++ b/src/_zkapauthorizer/tests/test_model.py @@ -129,7 +129,7 @@ class VoucherStoreTests(TestCase): store.get(voucher), MatchesStructure( number=Equals(voucher), - state=Equals(Pending()), + state=Equals(Pending(counter=0)), created=Equals(now), ), ) @@ -148,7 +148,7 @@ class VoucherStoreTests(TestCase): MatchesStructure( number=Equals(voucher), created=Equals(now), - state=Equals(Pending()), + state=Equals(Pending(counter=0)), ), ) self.assertThat(