diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py index df0c81dd3fabd08635220d44419dd1cf41177364..b68779cd300c848f38913f660e7c26f97397388a 100644 --- a/src/_zkapauthorizer/controller.py +++ b/src/_zkapauthorizer/controller.py @@ -212,12 +212,20 @@ class IndexedRedeemer(object): A ``IndexedRedeemer`` delegates redemption to a redeemer chosen to correspond to the redemption counter given. """ + _log = Logger() + redeemers = attr.ib() def random_tokens_for_voucher(self, voucher, counter, count): return dummy_random_tokens(voucher, counter, count) def redeemWithCounter(self, voucher, counter, random_tokens): + self._log.info( + "IndexedRedeemer redeeming {voucher}[{counter}] using {delegate}.", + voucher=voucher, + counter=counter, + delegate=self.redeemers[counter], + ) return self.redeemers[counter].redeemWithCounter( voucher, counter, @@ -796,14 +804,30 @@ class PaymentController(object): if num_tokens is None: num_tokens = self.default_token_count - # TODO: Actually count up from the voucher's current counter value to - # num_redemption_groups instead of only passing 0 here. Starting at 0 - # is fine for a new voucher but if we partially redeemed a voucher on - # a previous run and this call comes from `_check_pending_vouchers` - # then we should skip any already-redeemed counter values. - # - # https://github.com/PrivateStorageio/ZKAPAuthorizer/issues/124 - for counter in range(0, self.num_redemption_groups): + # Try to get an existing voucher object for the given number. + try: + voucher_obj = self.store.get(voucher) + except KeyError: + # This is our first time dealing with this number. + counter_start = 0 + else: + # Determine the starting point from the state. + if voucher_obj.state.should_start_redemption(): + counter_start = voucher_obj.state.counter + else: + raise ValueError( + "Cannot redeem voucher in state {}.".format( + voucher_obj.state, + ), + ) + + self._log.info( + "Starting redemption of {voucher}[{start}..{end}]", + voucher=voucher, + start=counter_start, + end=self.num_redemption_groups, + ) + for counter in range(counter_start, self.num_redemption_groups): # Pre-generate the random tokens to use when redeeming the voucher. # These are persisted with the voucher so the redemption can be made # idempotent. We don't want to lose the value if we fail after the diff --git a/src/_zkapauthorizer/tests/test_controller.py b/src/_zkapauthorizer/tests/test_controller.py index f86bc3ab3b6354e2fa641e39709b7b6764a4c04d..48cfb2fbfa2c333fb51e5bcbee7cc3ba3d46b28f 100644 --- a/src/_zkapauthorizer/tests/test_controller.py +++ b/src/_zkapauthorizer/tests/test_controller.py @@ -209,6 +209,7 @@ class TokenCountForGroupTests(TestCase): AllMatch(between(lower_bound, upper_bound)), ) + class PaymentControllerTests(TestCase): """ Tests for ``PaymentController``. @@ -247,14 +248,10 @@ class PaymentControllerTests(TestCase): # at least *one* run through so we'll bump this up to be sure we get # that. counter = num_successes + 1 - - success_redeemers = [DummyRedeemer()] * num_successes - hang_redeemers = [NonRedeemer()] - redeemers = success_redeemers + hang_redeemers - # A redeemer which will succeed `num_successes` times and then hang on - # the next attempt. - redeemer = IndexedRedeemer(redeemers) - + redeemer = IndexedRedeemer( + [DummyRedeemer()] * num_successes + + [NonRedeemer()], + ) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store controller = PaymentController( store, @@ -280,6 +277,84 @@ class PaymentControllerTests(TestCase): )), ) + @given( + tahoe_configs(), + datetimes(), + vouchers(), + voucher_counters(), + voucher_counters().map(lambda v: v + 1), + ) + def test_restart_redeeming(self, get_config, now, voucher, before_restart, after_restart): + """ + If some redemption groups for a voucher have succeeded but the process is + interrupted, redemption begins at the first incomplete redemption + group when it resumes. + + :parm int before_restart: The number of redemption groups which will + be allowed to succeed before making the redeemer hang. Redemption + will then be required to begin again from only database state. + + :param int after_restart: The number of redemption groups which will + be required to succeed after restarting the process. + """ + # Divide redemption into some groups that will succeed before a + # restart and some that must succeed after a restart. + num_redemption_groups = before_restart + after_restart + # Give it enough tokens so each group can have one. + num_tokens = num_redemption_groups + + store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store + + def first_try(): + controller = PaymentController( + store, + # It will let `before_restart` attempts succeed before hanging. + IndexedRedeemer( + [DummyRedeemer()] * before_restart + + [NonRedeemer()] * after_restart, + ), + default_token_count=num_tokens, + num_redemption_groups=num_redemption_groups, + ) + self.assertThat( + controller.redeem(voucher), + has_no_result(), + ) + + def second_try(): + # The controller will find the voucher in the voucher store and + # restart redemption on its own. + return PaymentController( + store, + # It will succeed only for the higher counter values which did + # not succeed or did not get started on the first try. + IndexedRedeemer( + [NonRedeemer()] * before_restart + + [DummyRedeemer()] * after_restart, + ), + # TODO: It shouldn't need a default token count. It should + # respect whatever was given on the first redemption attempt. + default_token_count=num_tokens, + # The number of redemption groups must not change for + # redemption of a particular voucher. + num_redemption_groups=num_redemption_groups, + ) + + first_try() + controller = second_try() + + persisted_voucher = controller.get_voucher(voucher) + self.assertThat( + persisted_voucher.state, + Equals( + model_Redeemed( + finished=now, + token_count=num_tokens, + public_key=None, + ), + ), + ) + @given(tahoe_configs(), dummy_ristretto_keys(), datetimes(), vouchers()) def test_redeemed_after_redeeming(self, get_config, public_key, now, voucher): """