diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py index f40ffef196fcb1072c592e775d3f16c8c81fc296..cdba78ec48c7c6200a4f2571903f88db0ad72949 100644 --- a/src/_zkapauthorizer/controller.py +++ b/src/_zkapauthorizer/controller.py @@ -766,9 +766,14 @@ class PaymentController(object): voucher=voucher.number, ) - def _get_random_tokens_for_voucher(self, voucher, counter, num_tokens): + def _get_random_tokens_for_voucher(self, voucher, counter, num_tokens, total_tokens): """ Generate or load random tokens for a redemption attempt of a voucher. + + :param int num_tokens: The number of tokens to get. + + :param int total_tokens: The total number of tokens for which this + voucher is expected to be redeemed. """ def get_tokens(): self._log.info( @@ -776,12 +781,23 @@ class PaymentController(object): voucher=voucher, ) return self.redeemer.random_tokens_for_voucher( - Voucher(voucher), + Voucher( + number=voucher, + # Unclear whether this information is useful to redeemers + # but we cannot construct a Voucher without some value + # here. + expected_tokens=total_tokens, + ), counter, num_tokens, ) - return self.store.add(voucher, counter, get_tokens) + return self.store.add( + voucher, + total_tokens, + counter, + get_tokens, + ) @inlineCallbacks def redeem(self, voucher, num_tokens=None): @@ -790,16 +806,16 @@ class PaymentController(object): :param int num_tokens: A number of tokens to redeem. """ - if num_tokens is None: - num_tokens = self.default_token_count - # 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 + if num_tokens is None: + num_tokens = self.default_token_count else: + num_tokens = voucher_obj.expected_tokens # Determine the starting point from the state. if voucher_obj.state.should_start_redemption(): counter_start = voucher_obj.state.counter @@ -811,10 +827,11 @@ class PaymentController(object): ) self._log.info( - "Starting redemption of {voucher}[{start}..{end}]", + "Starting redemption of {voucher}[{start}..{end}] for {num_tokens} tokens.", voucher=voucher, start=counter_start, end=self.num_redemption_groups, + num_tokens=num_tokens, ) for counter in range(counter_start, self.num_redemption_groups): # Pre-generate the random tokens to use when redeeming the voucher. @@ -827,7 +844,12 @@ class PaymentController(object): # number of passes that can be constructed is still only the size of # the set of random tokens. token_count = token_count_for_group(self.num_redemption_groups, num_tokens, counter) - tokens = self._get_random_tokens_for_voucher(voucher, counter, token_count) + tokens = self._get_random_tokens_for_voucher( + voucher, + counter, + num_tokens=token_count, + total_tokens=num_tokens, + ) # Reload state before each iteration. We expect it to change each time. voucher_obj = self.store.get(voucher) diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py index 8eb85ee47f3a645f110cd3d5de297a5576303c2f..329d0b8bc0159ba4fc367748b278cd7f51465801 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], [counter] + [number], [created], [expected-tokens], [state], [finished], [token-count], [public-key], [counter] FROM [vouchers] WHERE @@ -225,7 +225,7 @@ class VoucherStore(object): return Voucher.from_row(refs[0]) @with_cursor - def add(self, cursor, voucher, counter, get_tokens): + def add(self, cursor, voucher, expected_tokens, counter, get_tokens): """ Add random tokens associated with a voucher (possibly new, possibly existing) to the database. If the (voucher, counter) pair is already @@ -234,6 +234,16 @@ class VoucherStore(object): :param unicode voucher: The text value of a voucher with which to associate the tokens. + :param int expected_tokens: The total number of tokens for which this + voucher is expected to be redeemed. This is only respected the + first time a voucher is added. Subsequent calls with the same + voucher but a different count ignore the value because it is + already known (and the database knows better than the caller what + it should be). + + This probably means ``add`` is a broken interface for doing these + two things. Maybe it should be fixed someday. + :param int counter: The redemption counter for the given voucher with which to associate the tokens. @@ -275,9 +285,9 @@ class VoucherStore(object): ) cursor.execute( """ - INSERT OR IGNORE INTO [vouchers] ([number], [created]) VALUES (?, ?) + INSERT OR IGNORE INTO [vouchers] ([number], [expected-tokens], [created]) VALUES (?, ?, ?) """, - (voucher, self.now()) + (voucher, expected_tokens, self.now()) ) cursor.executemany( """ @@ -302,7 +312,7 @@ class VoucherStore(object): cursor.execute( """ SELECT - [number], [created], [state], [finished], [token-count], [public-key], [counter] + [number], [created], [expected-tokens], [state], [finished], [token-count], [public-key], [counter] FROM [vouchers] """, @@ -878,6 +888,11 @@ class Voucher(object): :ivar datetime created: The time at which this voucher was added to this node. + :ivar expected_tokens: The total number of tokens for which we expect to + be able to redeem this voucher. Tokens are redeemed in smaller + groups, progress of which is tracked in ``state``. This only gives + the total we expect to reach at completion. + :ivar state: An indication of the current state of this voucher. This is an instance of ``Pending``, ``Redeeming``, ``Redeemed``, ``DoubleSpend``, ``Unpaid``, or ``Error``. @@ -889,10 +904,21 @@ class Voucher(object): has_length(44), ), ) + + expected_tokens = attr.ib( + validator=attr.validators.optional( + attr.validators.and_( + attr.validators.instance_of((int, long)), + greater_than(0), + ), + ), + ) + created = attr.ib( default=None, validator=attr.validators.optional(attr.validators.instance_of(datetime)), ) + state = attr.ib( default=Pending(counter=0), validator=attr.validators.instance_of(( @@ -922,16 +948,21 @@ class Voucher(object): ) raise ValueError("Unknown voucher state {}".format(state)) - number, created, state = row[:3] + number, created, expected_tokens, state = row[:4] + + if expected_tokens is None: + raise ValueError("Bluib") + return cls( - number, + number=number, + expected_tokens=expected_tokens, # All Python datetime-based date/time libraries fail to handle # leap seconds. This parse call might raise an exception of the # value represents a leap second. However, since we also use # Python to generate the data in the first place, it should never # represent a leap second... I hope. - parse_datetime(created, delimiter=u" "), - state_from_row(state, row[3:]) + created=parse_datetime(created, delimiter=u" "), + state=state_from_row(state, row[4:]), ) @classmethod @@ -976,6 +1007,7 @@ class Voucher(object): return cls( number=values[u"number"], + expected_tokens=values[u"expected-tokens"], created=None if values[u"created"] is None else parse_datetime(values[u"created"]), state=state, ) @@ -993,6 +1025,7 @@ class Voucher(object): state = self.state.to_json_v1() return { u"number": self.number, + u"expected-tokens": self.expected_tokens, u"created": None if self.created is None else self.created.isoformat(), u"state": state, u"version": 1, diff --git a/src/_zkapauthorizer/schema.py b/src/_zkapauthorizer/schema.py index 4fa33b13ae1f1001816e269bfd0b49ded10951bb..596629950ededd1535cbf6a49a4788d1ea347ef1 100644 --- a/src/_zkapauthorizer/schema.py +++ b/src/_zkapauthorizer/schema.py @@ -144,6 +144,12 @@ _UPGRADES = { -- Reference to the counter these tokens go with. ALTER TABLE [tokens] ADD COLUMN [counter] integer NOT NULL DEFAULT 0 """, - + """ + -- Record the total number of tokens for which we expect to be able to + -- redeem this voucher. For rows in the database before this column + -- was added, we forgot how many tokens we expected. All rows added + -- after this column is added should have a non-NULL value. + ALTER TABLE [vouchers] ADD COLUMN [expected-tokens] integer + """, ], } diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index 3baf11475b7daa296d8ef621ba493175238a3a45..9a4c325df7cddf5db27e930be326748d196348fc 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -342,6 +342,7 @@ def voucher_objects(states=voucher_states()): Voucher, number=vouchers(), created=one_of(none(), datetimes()), + expected_tokens=integers(min_value=1), state=states, ) diff --git a/src/_zkapauthorizer/tests/test_client_resource.py b/src/_zkapauthorizer/tests/test_client_resource.py index 48ca9b960f1af29df554136543dd076461960b35..65b0286ed6cf839ddd0b7c78b983036cab98b340 100644 --- a/src/_zkapauthorizer/tests/test_client_resource.py +++ b/src/_zkapauthorizer/tests/test_client_resource.py @@ -991,7 +991,8 @@ class VoucherTests(TestCase): Equals({ u"vouchers": list( Voucher( - voucher, + number=voucher, + expected_tokens=NUM_TOKENS, created=now, state=Redeemed( finished=now, @@ -1022,7 +1023,8 @@ class VoucherTests(TestCase): Equals({ u"vouchers": list( Voucher( - voucher, + number=voucher, + expected_tokens=NUM_TOKENS, created=now, state=Unpaid( finished=now, diff --git a/src/_zkapauthorizer/tests/test_controller.py b/src/_zkapauthorizer/tests/test_controller.py index 963f84e55f4327092b461d2a872f9a48d475848b..643749d470d9e2c4cc529c8564a93bcf42b75e3e 100644 --- a/src/_zkapauthorizer/tests/test_controller.py +++ b/src/_zkapauthorizer/tests/test_controller.py @@ -333,11 +333,11 @@ class PaymentControllerTests(TestCase): [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. - # - # https://github.com/PrivateStorageio/ZKAPAuthorizer/issues/93 - default_token_count=num_tokens, + # The default token count for this new controller doesn't + # matter. The redemption attempt already started with some + # token count. That token count must be respected on + # resumption. + default_token_count=0, # The number of redemption groups must not change for # redemption of a particular voucher. num_redemption_groups=num_redemption_groups, diff --git a/src/_zkapauthorizer/tests/test_model.py b/src/_zkapauthorizer/tests/test_model.py index 0e5ebd3e971de668d3ca3de36e356d943d62531f..ca0d3c600ed8bff92593ffb1b08f81d1ffb15e73 100644 --- a/src/_zkapauthorizer/tests/test_model.py +++ b/src/_zkapauthorizer/tests/test_model.py @@ -119,18 +119,19 @@ class VoucherStoreTests(TestCase): raises(KeyError), ) - @given(tahoe_configs(), vouchers(), lists(random_tokens(), unique=True), datetimes()) + @given(tahoe_configs(), vouchers(), lists(random_tokens(), min_size=1, unique=True), datetimes()) def test_add(self, get_config, voucher, tokens, now): """ ``VoucherStore.get`` returns a ``Voucher`` representing a voucher previously added to the store with ``VoucherStore.add``. """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher, 0, lambda: tokens) + store.add(voucher, len(tokens), 0, lambda: tokens) self.assertThat( store.get(voucher), MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(len(tokens)), state=Equals(Pending(counter=0)), created=Equals(now), ), @@ -154,13 +155,16 @@ class VoucherStoreTests(TestCase): tokens_b = tokens[len(tokens) / 2:] store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - added_tokens_a = store.add(voucher, counter_a, lambda: tokens_a) - added_tokens_b = store.add(voucher, counter_b, lambda: tokens_b) + # We only have to get the expected_tokens value (len(tokens)) right on + # the first call. + added_tokens_a = store.add(voucher, len(tokens), counter_a, lambda: tokens_a) + added_tokens_b = store.add(voucher, 0, counter_b, lambda: tokens_b) self.assertThat( store.get(voucher), MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(len(tokens)), state=Equals(Pending(counter=0)), created=Equals(now), ), @@ -169,19 +173,20 @@ class VoucherStoreTests(TestCase): self.assertThat(tokens_a, Equals(added_tokens_a)) self.assertThat(tokens_b, Equals(added_tokens_b)) - @given(tahoe_configs(), vouchers(), datetimes(), lists(random_tokens(), unique=True)) + @given(tahoe_configs(), vouchers(), datetimes(), lists(random_tokens(), min_size=1, unique=True)) def test_add_idempotent(self, get_config, voucher, now, tokens): """ More than one call to ``VoucherStore.add`` with the same argument results in the same state as a single call. """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - first_tokens = store.add(voucher, 0, lambda: tokens) - second_tokens = store.add(voucher, 0, lambda: []) + first_tokens = store.add(voucher, len(tokens), 0, lambda: tokens) + second_tokens = store.add(voucher, 0, 0, lambda: []) self.assertThat( store.get(voucher), MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(len(tokens)), created=Equals(now), state=Equals(Pending(counter=0)), ), @@ -195,20 +200,33 @@ class VoucherStoreTests(TestCase): Equals(tokens), ) - @given(tahoe_configs(), datetimes(), lists(vouchers(), unique=True)) - def test_list(self, get_config, now, vouchers): + @given(tahoe_configs(), datetimes(), lists(vouchers(), unique=True), data()) + def test_list(self, get_config, now, vouchers, data): """ ``VoucherStore.list`` returns a ``list`` containing a ``Voucher`` object for each voucher previously added. """ + tokens = iter(data.draw( + lists( + random_tokens(), + unique=True, + min_size=len(vouchers), + max_size=len(vouchers), + ), + )) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store for voucher in vouchers: - store.add(voucher, 0, lambda: []) + store.add( + voucher, + expected_tokens=1, + counter=0, + get_tokens=lambda: [next(tokens)], + ) self.assertThat( store.list(), Equals(list( - Voucher(number, created=now) + Voucher(number, expected_tokens=1, created=now) for number in vouchers )), @@ -341,7 +359,7 @@ class VoucherStoreTests(TestCase): # Put some tokens in it that we can backup and extract random_tokens, unblinded_tokens = paired_tokens(data, integers(min_value=1, max_value=5)) - store.add(voucher_value, 0, lambda: random_tokens) + store.add(voucher_value, len(random_tokens), 0, lambda: random_tokens) store.insert_unblinded_tokens_for_voucher( voucher_value, public_key, @@ -517,7 +535,7 @@ class UnblindedTokenStoreTests(TestCase): """ random_tokens, unblinded_tokens = paired_tokens(data) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, 0, lambda: random_tokens) + store.add(voucher_value, len(random_tokens), 0, lambda: random_tokens) store.insert_unblinded_tokens_for_voucher(voucher_value, public_key, unblinded_tokens, completed) retrieved_tokens = store.extract_unblinded_tokens(len(random_tokens)) @@ -563,12 +581,13 @@ class UnblindedTokenStoreTests(TestCase): ) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, 0, lambda: random) + store.add(voucher_value, len(random), 0, lambda: random) store.insert_unblinded_tokens_for_voucher(voucher_value, public_key, unblinded, completed=True) loaded_voucher = store.get(voucher_value) self.assertThat( loaded_voucher, MatchesStructure( + expected_tokens=Equals(len(random)), state=Equals(Redeemed( finished=now, token_count=num_tokens, @@ -581,7 +600,7 @@ class UnblindedTokenStoreTests(TestCase): tahoe_configs(), datetimes(), vouchers(), - lists(random_tokens(), unique=True), + lists(random_tokens(), min_size=1, unique=True), ) def test_mark_vouchers_double_spent(self, get_config, now, voucher_value, random_tokens): """ @@ -589,7 +608,7 @@ class UnblindedTokenStoreTests(TestCase): such. """ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, 0, lambda: random_tokens) + store.add(voucher_value, len(random_tokens), 0, lambda: random_tokens) store.mark_voucher_double_spent(voucher_value) voucher = store.get(voucher_value) self.assertThat( @@ -630,7 +649,7 @@ class UnblindedTokenStoreTests(TestCase): ), ) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, 0, lambda: random) + store.add(voucher_value, len(random), 0, lambda: random) store.insert_unblinded_tokens_for_voucher(voucher_value, public_key, unblinded, completed=True) self.assertThat( lambda: store.mark_voucher_double_spent(voucher_value), @@ -684,7 +703,7 @@ class UnblindedTokenStoreTests(TestCase): ), ) store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store - store.add(voucher_value, 0, lambda: random) + store.add(voucher_value, len(random), 0, lambda: random) store.insert_unblinded_tokens_for_voucher(voucher_value, public_key, unblinded, completed) self.assertThat(