diff --git a/docs/source/interface.rst b/docs/source/interface.rst index 8b035cfac0813dacf423da714102b7e39f4b0529..678d5c45973b040cf45e34f94f415674887b9574 100644 --- a/docs/source/interface.rst +++ b/docs/source/interface.rst @@ -42,32 +42,41 @@ If the voucher is not known then the response is **NOT FOUND**. For any voucher which has previously been submitted, the response is **OK** with an ``application/json`` content-type response body like:: - { "value": <string> + { "version": 1 + , "number": <string> + , "expected-tokens": <integer> , "created": <iso8601 timestamp> , "state": <state object> - , "version": 1 } -The ``value`` property merely indicates the voucher which was requested. -The ``created`` property indicates when the voucher was first added to the node. -The ``redeemed`` property indicates whether or not the voucher has successfully been redeemed with a payment server yet. -The ``token-count`` property gives the number of blinded token signatures the client received in exchange for redemption of the voucher -(each blinded token signature can be used to construct a one ZKAP), -if it has been redeemed. -If it has not been redeemed then it is ``null``. +The ``version`` property indicates the semantic version of the data being returned. +When properties are removed or the meaning of a property is changed, +the value of the ``version`` property will be incremented. +The addition of new properties is **not** accompanied by a bumped version number. +The ``number`` property merely indicates the voucher which was requested. +The ``expected-tokens`` property indicates the total number of ZKAPs for which the client intends to redeem the voucher. +Vouchers created using old versions of ZKAPAuthorizer will have a best-guess value here because the real value was not recorded. +The ``created`` property indicates when the voucher was first added to the node. The ``state`` property is an object that gives more details about the current state of the voucher. The following values are possible:: { "name": "pending" + , "counter": <integer> } +The integer *counter* value indicates how many successful sub-redemptions have completed for this voucher. + :: { "name": "redeeming" , "started": <iso8601 timestamp> + , "counter": <integer> } +The *started* timestamp gives the time when the most recent redemption attempt began. +The integer *counter* value has the same meaning as it does for the *pending* state. + :: { "name": "redeemed" @@ -75,18 +84,25 @@ The following values are possible:: , "token-count": <number> } +The *finished* timestamp gives the time when redemption completed successfully. +The integer *token-count* gives the number tokens for which the voucher was redeemed. + :: { "name": "double-spend" , "finished": <iso8601 timestamp> } +The *finished* timestamp gives the time when the double-spend error was encountered. + :: { "name": "unpaid" , "finished": <iso8601 timestamp> } +The *finished* timestamp gives the time when the unpaid error was encountered. + :: { "name": "error" @@ -94,10 +110,8 @@ The following values are possible:: , "details": <text> } -The ``version`` property indicates the semantic version of the data being returned. -When properties are removed or the meaning of a property is changed, -the value of the ``version`` property will be incremented. -The addition of new properties is **not** accompanied by a bumped version number. +The *finished* timestamp gives the time when this other error condition was encountered. +The *details* string may give additional details about what the error was. ``GET /storage-plugins/privatestorageio-zkapauthz-v1/voucher`` ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py index 77436a8bdebbbcd1c2b79631f8f614bdcf023009..3b0511c914144a70506a738097c9bdd36d2c16fd 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 baff206c28c14931f9d403d47b4938cccb756b10..b7d590bec3e26bf7ac8f9c288ed92fad88079e62 100644 --- a/src/_zkapauthorizer/model.py +++ b/src/_zkapauthorizer/model.py @@ -218,7 +218,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 @@ -232,7 +232,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 @@ -241,6 +241,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. @@ -282,9 +292,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( """ @@ -309,7 +319,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] """, @@ -753,6 +763,7 @@ class Redeeming(object): return { u"name": u"redeeming", u"started": self.started.isoformat(), + u"counter": self.counter, } @@ -848,6 +859,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``. @@ -859,10 +875,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(( @@ -892,16 +919,18 @@ class Voucher(object): ) raise ValueError("Unknown voucher state {}".format(state)) - number, created, state = row[:3] + number, created, expected_tokens, state = row[:4] + 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 @@ -920,7 +949,7 @@ class Voucher(object): elif state_name == u"redeeming": state = Redeeming( started=parse_datetime(state_json[u"started"]), - counter=0, + counter=state_json[u"counter"], ) elif state_name == u"double-spend": state = DoubleSpend( @@ -946,6 +975,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, ) @@ -963,6 +993,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..a23d3373c9a230d874710183e046d9e9cef954e6 100644 --- a/src/_zkapauthorizer/schema.py +++ b/src/_zkapauthorizer/schema.py @@ -144,6 +144,16 @@ _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. We don't want to allow NULL values here at + -- all because that allows insertion of garbage data going forward. + -- However to add a non-NULL column to a table we have to supply a + -- default value. Since no real vouchers have ever been issued at the + -- time of this upgrade we'll just make up some value. It doesn't + -- particularly matter if it is wrong for some testing voucher someone + -- used. + ALTER TABLE [vouchers] ADD COLUMN [expected-tokens] integer NOT NULL DEFAULT 32768 + """, ], } diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py index a438fa27ae10bd937f6e30421297da297fc753f6..28028fd87ad78348725343f9ac19bf710c6eb040 100644 --- a/src/_zkapauthorizer/tests/strategies.py +++ b/src/_zkapauthorizer/tests/strategies.py @@ -77,6 +77,7 @@ from ..model import ( DoubleSpend, Unpaid, Error, + Redeeming, Redeemed, ) @@ -297,6 +298,7 @@ def vouchers(): lambda voucher: voucher.decode("ascii"), ) + def redeemed_states(): """ Build ``Redeemed`` instances. @@ -308,6 +310,20 @@ def redeemed_states(): public_key=dummy_ristretto_keys(), ) + +def voucher_counters(): + """ + Build integers usable as counters in the voucher redemption process. + """ + return integers( + min_value=0, + # This may or may not be the actual maximum value accepted by a + # PaymentServer. If it is not exactly the maximum, it's probably at + # least in the same ballpark. + max_value=256, + ) + + def voucher_states(): """ Build Python objects representing states a Voucher can be in. @@ -315,7 +331,12 @@ def voucher_states(): return one_of( builds( Pending, - integers(min_value=0), + counter=integers(min_value=0), + ), + builds( + Redeeming, + started=datetimes(), + counter=voucher_counters(), ), redeemed_states(), builds( @@ -342,23 +363,11 @@ def voucher_objects(states=voucher_states()): Voucher, number=vouchers(), created=one_of(none(), datetimes()), + expected_tokens=integers(min_value=1), state=states, ) -def voucher_counters(): - """ - Build integers usable as counters in the voucher redemption process. - """ - return integers( - min_value=0, - # This may or may not be the actual maximum value accepted by a - # PaymentServer. If it is not exactly the maximum, it's probably at - # least in the same ballpark. - max_value=256, - ) - - def redemption_group_counts(): """ Build integers which can represent the number of groups in the redemption diff --git a/src/_zkapauthorizer/tests/test_client_resource.py b/src/_zkapauthorizer/tests/test_client_resource.py index a6a96f9c82a4b9615a58b11a68445c094a92506a..7aabbdb359b3ad9a70644cfd806520884d84fdb3 100644 --- a/src/_zkapauthorizer/tests/test_client_resource.py +++ b/src/_zkapauthorizer/tests/test_client_resource.py @@ -823,6 +823,7 @@ class VoucherTests(TestCase): voucher, MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(NUM_TOKENS), created=Equals(now), state=Equals(Redeeming( started=now, @@ -845,6 +846,7 @@ class VoucherTests(TestCase): voucher, MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(NUM_TOKENS), created=Equals(now), state=Equals(Redeemed( finished=now, @@ -869,6 +871,7 @@ class VoucherTests(TestCase): voucher, MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(NUM_TOKENS), created=Equals(now), state=Equals(DoubleSpend( finished=now, @@ -891,6 +894,7 @@ class VoucherTests(TestCase): voucher, MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(NUM_TOKENS), created=Equals(now), state=Equals(Unpaid( finished=now, @@ -913,6 +917,7 @@ class VoucherTests(TestCase): voucher, MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(NUM_TOKENS), created=Equals(now), state=Equals(Error( finished=now, @@ -990,7 +995,8 @@ class VoucherTests(TestCase): Equals({ u"vouchers": list( Voucher( - voucher, + number=voucher, + expected_tokens=NUM_TOKENS, created=now, state=Redeemed( finished=now, @@ -1021,7 +1027,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 77d7e90abccd49ee5fb07e9afc148689844377c5..3e8d6fce78ef80963977ce3137121c325b34c415 100644 --- a/src/_zkapauthorizer/tests/test_controller.py +++ b/src/_zkapauthorizer/tests/test_controller.py @@ -368,11 +368,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, @@ -393,7 +393,6 @@ class PaymentControllerTests(TestCase): ), ) - @given(tahoe_configs(), datetimes(), vouchers(), voucher_counters(), integers(min_value=0, max_value=100)) def test_stop_redeeming_on_error(self, get_config, now, voucher, counter, extra_tokens): """ diff --git a/src/_zkapauthorizer/tests/test_model.py b/src/_zkapauthorizer/tests/test_model.py index 5bd3e3145d12b90a67ef27ce4aaa7dc382864ef4..e13856f349176a47cd1f4347cc1f471d38a66945 100644 --- a/src/_zkapauthorizer/tests/test_model.py +++ b/src/_zkapauthorizer/tests/test_model.py @@ -115,18 +115,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), ), @@ -150,13 +151,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), ), @@ -165,19 +169,35 @@ 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, + expected_tokens=len(tokens), + counter=0, + get_tokens=lambda: tokens, + ) + second_tokens = store.add( + voucher, + # The voucher should already exists in the store so the + # expected_tokens value supplied here is ignored. + expected_tokens=0, + counter=0, + # Likewise, no need to generate tokens here because counter value + # 0 was already added and tokens were generated then. If + # get_tokens were called here, it would be an error. + get_tokens=None, + ) self.assertThat( store.get(voucher), MatchesStructure( number=Equals(voucher), + expected_tokens=Equals(len(tokens)), created=Equals(now), state=Equals(Pending(counter=0)), ), @@ -191,20 +211,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 )), @@ -337,7 +370,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 +550,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 +596,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 +615,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 +623,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 +664,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 +718,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(