diff --git a/src/_zkapauthorizer/controller.py b/src/_zkapauthorizer/controller.py
index 21e0189b2e899564daebf3a07e9e6c3d27e5bbe6..429805b5bb222830b5b3cd0b65bebe508712b74a 100644
--- a/src/_zkapauthorizer/controller.py
+++ b/src/_zkapauthorizer/controller.py
@@ -30,6 +30,10 @@ from functools import (
from json import (
dumps,
)
+from datetime import (
+ timedelta,
+)
+
import attr
from zope.interface import (
@@ -37,6 +41,9 @@ from zope.interface import (
implementer,
)
+from twisted.python.reflect import (
+ namedAny,
+)
from twisted.logger import (
Logger,
)
@@ -50,6 +57,9 @@ from twisted.internet.defer import (
inlineCallbacks,
returnValue,
)
+from twisted.internet.task import (
+ LoopingCall,
+)
from twisted.web.client import (
Agent,
)
@@ -73,6 +83,10 @@ from .model import (
Error as model_Error,
)
+# The number of tokens to submit with a voucher redemption.
+NUM_TOKENS = 100
+
+RETRY_INTERVAL = timedelta(minutes=3)
class AlreadySpent(Exception):
"""
@@ -492,14 +506,14 @@ class PaymentController(object):
the voucher. The data store marks the voucher as redeemed and stores
the unblinded tokens for use by the storage client.
- :ivar dict[voucher, datetime] _active: A mapping from voucher identifiers
+ :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[voucher, datetime] _error: A mapping from voucher identifiers
+ :ivar dict[unicode, datetime] _error: A mapping from voucher identifiers
which have recently failed with an unrecognized, transient error.
- :ivar dict[voucher, datetime] _unpaid: A mapping from voucher identifiers
+ :ivar dict[unicode, datetime] _unpaid: A mapping from voucher identifiers
which have recently failed a redemption attempt due to an unpaid
response from the redemption server to timestamps when the failure was
observed.
@@ -509,38 +523,77 @@ class PaymentController(object):
store = attr.ib()
redeemer = attr.ib()
+ _clock = attr.ib(
+ default=attr.Factory(partial(namedAny, "twisted.internet.reactor")),
+ )
+
_error = attr.ib(default=attr.Factory(dict))
_unpaid = attr.ib(default=attr.Factory(dict))
_active = attr.ib(default=attr.Factory(dict))
- def redeem(self, voucher, num_tokens=100):
+ def __attrs_post_init__(self):
"""
- :param unicode voucher: A voucher to redeem.
+ Check the voucher store for any vouchers in need of redemption.
- :param int num_tokens: A number of tokens to redeem.
+ This is an initialization-time hook called by attrs.
"""
- # 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
- # server deems the voucher redeemed but before we persist the result.
- # With a stable set of tokens, we can re-submit them and the server
- # can re-sign them without fear of issuing excess passes. Whether the
- # server signs a given set of random tokens once or many times, the
- # number of passes that can be constructed is still only the size of
- # the set of random tokens.
- self._log.info("Generating random tokens for a voucher ({voucher}).", voucher=voucher)
- tokens = self.redeemer.random_tokens_for_voucher(Voucher(voucher), num_tokens)
+ self._check_pending_vouchers()
+ # Also start a time-based polling loop to retry redemption of vouchers
+ # in retryable error states.
+ self._schedule_retries()
+
+ def _schedule_retries(self):
+ # TODO: should not eagerly schedule calls. If there are no vouchers
+ # in an error state we shouldn't wake up at all.
+ #
+ # TODO: should schedule retries on a bounded exponential backoff
+ # instead, perhaps on a per-voucher basis.
+ self._retry_task = LoopingCall(self._retry_redemption)
+ self._retry_task.clock = self._clock
+ self._retry_task.start(
+ RETRY_INTERVAL.total_seconds(),
+ now=False,
+ )
- # Persist the voucher and tokens so they're available if we fail.
- self._log.info("Persistenting random tokens for a voucher ({voucher}).", voucher=voucher)
- self.store.add(voucher, tokens)
+ def _retry_redemption(self):
+ for voucher in self._error.keys() + self._unpaid.keys():
+ if voucher in self._active:
+ continue
+ if self.get_voucher(voucher).state.should_start_redemption():
+ self.redeem(voucher)
+ def _check_pending_vouchers(self):
+ """
+ Find vouchers in the voucher store that need to be redeemed and try to
+ redeem them.
+ """
+ vouchers = self.store.list()
+ for voucher in vouchers:
+ if voucher.state.should_start_redemption():
+ self._log.info(
+ "Controller found voucher ({}) at startup that needs redemption.",
+ voucher=voucher.number,
+ )
+ self.redeem(voucher.number)
+ else:
+ self._log.info(
+ "Controller found voucher ({}) at startup that does not need redemption.",
+ voucher=voucher.number,
+ )
+
+ def _perform_redeem(self, voucher, random_tokens):
+ """
+ Use the redeemer to redeem the given voucher and random tokens.
+
+ This will not persist the voucher or random tokens but it will persist
+ the result.
+ """
# 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.redeem(Voucher(voucher), tokens),
+ lambda: self.redeemer.redeem(Voucher(voucher), random_tokens),
)
d.addCallbacks(
partial(self._redeemSuccess, voucher),
@@ -549,6 +602,43 @@ class PaymentController(object):
d.addErrback(partial(self._finalRedeemError, voucher))
return d
+ def _get_random_tokens_for_voucher(self, voucher, num_tokens):
+ """
+ Generate or load random tokens for a redemption attempt of a voucher.
+ """
+ self._log.info("Generating random tokens for a voucher ({voucher}).", voucher=voucher)
+ tokens = self.redeemer.random_tokens_for_voucher(Voucher(voucher), num_tokens)
+
+ self._log.info("Persistenting random tokens for a voucher ({voucher}).", voucher=voucher)
+ self.store.add(voucher, tokens)
+
+ # XXX If the voucher is already in the store then the tokens passed to
+ # `add` are dropped and we need to get the tokens already persisted in
+ # the store. add should probably return them so we can use them. Or
+ # maybe we should pass the generator in so it can be called only if
+ # necessary.
+ return tokens
+
+ def redeem(self, voucher, num_tokens=None):
+ """
+ :param unicode voucher: A voucher to redeem.
+
+ :param int num_tokens: A number of tokens to redeem.
+ """
+ # 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
+ # server deems the voucher redeemed but before we persist the result.
+ # With a stable set of tokens, we can re-submit them and the server
+ # can re-sign them without fear of issuing excess passes. Whether the
+ # server signs a given set of random tokens once or many times, the
+ # number of passes that can be constructed is still only the size of
+ # the set of random tokens.
+ if num_tokens is None:
+ num_tokens = NUM_TOKENS
+ tokens = self._get_random_tokens_for_voucher(voucher, num_tokens)
+ return self._perform_redeem(voucher, tokens)
+
def _redeemSuccess(self, voucher, unblinded_tokens):
"""
Update the database state to reflect that a voucher was redeemed and to
@@ -587,6 +677,11 @@ class PaymentController(object):
self._log.failure("Redeeming random tokens for a voucher ({voucher}) encountered error.", reason, voucher=voucher)
return None
+ def get_voucher(self, number):
+ return self.incorporate_transient_state(
+ self.store.get(number),
+ )
+
def incorporate_transient_state(self, voucher):
"""
Create a new ``Voucher`` which represents the given voucher but which also
diff --git a/src/_zkapauthorizer/model.py b/src/_zkapauthorizer/model.py
index 8af6d83234719a7525b802a3c2d00173a6f55c48..65d653ddf94ceaa3435fbaf9a2bb690029f4de4b 100644
--- a/src/_zkapauthorizer/model.py
+++ b/src/_zkapauthorizer/model.py
@@ -466,6 +466,9 @@ class RandomToken(object):
@attr.s(frozen=True)
class Pending(object):
+ def should_start_redemption(self):
+ return True
+
def to_json_v1(self):
return {
u"name": u"pending",
@@ -481,6 +484,9 @@ class Redeeming(object):
"""
started = attr.ib(validator=attr.validators.instance_of(datetime))
+ def should_start_redemption(self):
+ return False
+
def to_json_v1(self):
return {
u"name": u"redeeming",
@@ -493,6 +499,9 @@ class Redeemed(object):
finished = attr.ib(validator=attr.validators.instance_of(datetime))
token_count = attr.ib(validator=attr.validators.instance_of((int, long)))
+ def should_start_redemption(self):
+ return False
+
def to_json_v1(self):
return {
u"name": u"redeemed",
@@ -505,6 +514,9 @@ class Redeemed(object):
class DoubleSpend(object):
finished = attr.ib(validator=attr.validators.instance_of(datetime))
+ def should_start_redemption(self):
+ return False
+
def to_json_v1(self):
return {
u"name": u"double-spend",
@@ -521,6 +533,9 @@ class Unpaid(object):
"""
finished = attr.ib(validator=attr.validators.instance_of(datetime))
+ def should_start_redemption(self):
+ return True
+
def to_json_v1(self):
return {
u"name": u"unpaid",
@@ -538,6 +553,9 @@ class Error(object):
finished = attr.ib(validator=attr.validators.instance_of(datetime))
details = attr.ib(validator=attr.validators.instance_of(unicode))
+ def should_start_redemption(self):
+ return True
+
def to_json_v1(self):
return {
u"name": u"error",
diff --git a/src/_zkapauthorizer/tests/fixtures.py b/src/_zkapauthorizer/tests/fixtures.py
index dbb49e6568cae998024e71c866e67e40c88b089f..eb64887b6798e2d2b164e289bf095b5777276f66 100644
--- a/src/_zkapauthorizer/tests/fixtures.py
+++ b/src/_zkapauthorizer/tests/fixtures.py
@@ -20,6 +20,8 @@ from __future__ import (
absolute_import,
)
+import attr
+
from fixtures import (
Fixture,
TempDir,
@@ -33,6 +35,11 @@ from allmydata.storage.server import (
StorageServer,
)
+from ..model import (
+ VoucherStore,
+ memory_connect,
+)
+
class AnonymousStorageServer(Fixture):
"""
Supply an instance of allmydata.storage.server.StorageServer which
@@ -50,3 +57,28 @@ class AnonymousStorageServer(Fixture):
self.tempdir.asBytesMode().path,
b"x" * 20,
)
+
+
+@attr.s
+class TemporaryVoucherStore(Fixture):
+ """
+ Create a ``VoucherStore`` in a temporary directory associated with the
+ given test case.
+
+ :ivar get_config: A function like the one built by ``tahoe_configs``.
+ :ivar get_now: A no-argument callable that returns a datetime giving a
+ time to consider as "now".
+
+ :ivar store: A newly created temporary store.
+ """
+ get_config = attr.ib()
+ get_now = attr.ib()
+
+ def _setUp(self):
+ self.tempdir = self.useFixture(TempDir())
+ self.config = self.get_config(self.tempdir.join(b"node"), b"tub.port")
+ self.store = VoucherStore.from_node_config(
+ self.config,
+ self.get_now,
+ memory_connect,
+ )
diff --git a/src/_zkapauthorizer/tests/test_controller.py b/src/_zkapauthorizer/tests/test_controller.py
index 86bcd8fa54d0ee95c13eefb6b748a8df3bd40f9b..672c95fa682d25dbe8ed90e484a2f6596ecc7d89 100644
--- a/src/_zkapauthorizer/tests/test_controller.py
+++ b/src/_zkapauthorizer/tests/test_controller.py
@@ -27,6 +27,10 @@ from json import (
from functools import (
partial,
)
+from datetime import (
+ datetime,
+ timedelta,
+)
from zope.interface import (
implementer,
)
@@ -50,10 +54,6 @@ from testtools.twistedsupport import (
failed,
)
-from fixtures import (
- TempDir,
-)
-
from hypothesis import (
given,
)
@@ -64,6 +64,9 @@ from hypothesis.strategies import (
from twisted.python.url import (
URL,
)
+from twisted.internet.task import (
+ Clock,
+)
from twisted.internet.defer import (
fail,
)
@@ -97,6 +100,7 @@ from ..controller import (
NonRedeemer,
DummyRedeemer,
DoubleSpendRedeemer,
+ UnpaidRedeemer,
RistrettoRedeemer,
PaymentController,
AlreadySpent,
@@ -104,12 +108,11 @@ from ..controller import (
)
from ..model import (
- memory_connect,
- VoucherStore,
UnblindedToken,
- Pending,
- DoubleSpend,
- Redeemed,
+ Pending as model_Pending,
+ DoubleSpend as model_DoubleSpend,
+ Redeemed as model_Redeemed,
+ Unpaid as model_Unpaid,
)
from .strategies import (
@@ -120,6 +123,11 @@ from .strategies import (
from .matchers import (
Provides,
)
+from .fixtures import (
+ TemporaryVoucherStore,
+)
+
+POSIX_EPOCH = datetime.utcfromtimestamp(0)
class PaymentControllerTests(TestCase):
"""
@@ -131,15 +139,7 @@ class PaymentControllerTests(TestCase):
A ``Voucher`` is not marked redeemed before ``IRedeemer.redeem``
completes.
"""
- tempdir = self.useFixture(TempDir())
- store = VoucherStore.from_node_config(
- get_config(
- tempdir.join(b"node"),
- b"tub.port",
- ),
- now=lambda: now,
- connect=memory_connect,
- )
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
controller = PaymentController(
store,
NonRedeemer(),
@@ -149,7 +149,7 @@ class PaymentControllerTests(TestCase):
persisted_voucher = store.get(voucher)
self.assertThat(
persisted_voucher.state,
- Equals(Pending()),
+ Equals(model_Pending()),
)
@given(tahoe_configs(), datetimes(), vouchers())
@@ -157,15 +157,7 @@ class PaymentControllerTests(TestCase):
"""
A ``Voucher`` is marked as redeemed after ``IRedeemer.redeem`` succeeds.
"""
- tempdir = self.useFixture(TempDir())
- store = VoucherStore.from_node_config(
- get_config(
- tempdir.join(b"node"),
- b"tub.port",
- ),
- now=lambda: now,
- connect=memory_connect,
- )
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
controller = PaymentController(
store,
DummyRedeemer(),
@@ -175,7 +167,7 @@ class PaymentControllerTests(TestCase):
persisted_voucher = store.get(voucher)
self.assertThat(
persisted_voucher.state,
- Equals(Redeemed(
+ Equals(model_Redeemed(
finished=now,
token_count=100,
)),
@@ -187,15 +179,7 @@ class PaymentControllerTests(TestCase):
A ``Voucher`` is marked as double-spent after ``IRedeemer.redeem`` fails
with ``AlreadySpent``.
"""
- tempdir = self.useFixture(TempDir())
- store = VoucherStore.from_node_config(
- get_config(
- tempdir.join(b"node"),
- b"tub.port",
- ),
- now=lambda: now,
- connect=memory_connect,
- )
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
controller = PaymentController(
store,
DoubleSpendRedeemer(),
@@ -206,12 +190,109 @@ class PaymentControllerTests(TestCase):
self.assertThat(
persisted_voucher,
MatchesStructure(
- state=Equals(DoubleSpend(
+ state=Equals(model_DoubleSpend(
finished=now,
)),
),
)
+ @given(tahoe_configs(), datetimes(), vouchers())
+ def test_redeem_pending_on_startup(self, get_config, now, voucher):
+ """
+ When ``PaymentController`` is created, any vouchers in the store in the
+ pending state are redeemed.
+ """
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
+ # Create the voucher state in the store with a redemption that will
+ # certainly fail.
+ unpaid_controller = PaymentController(
+ store,
+ UnpaidRedeemer(),
+ )
+ unpaid_controller.redeem(voucher)
+
+ # Make sure we got where we wanted.
+ self.assertThat(
+ unpaid_controller.get_voucher(voucher).state,
+ IsInstance(model_Unpaid),
+ )
+
+ # Create another controller with the same store. It will see the
+ # voucher state and attempt a redemption on its own. It has I/O as an
+ # `__init__` side-effect. :/
+ success_controller = PaymentController(
+ store,
+ DummyRedeemer(),
+ )
+
+ self.assertThat(
+ success_controller.get_voucher(voucher).state,
+ IsInstance(model_Redeemed),
+ )
+
+ @given(
+ tahoe_configs(),
+ datetimes(
+ # I don't know that time-based parts of the system break down
+ # before the POSIX epoch but I don't know that they work, either.
+ # Don't time travel with this code.
+ min_value=POSIX_EPOCH,
+ # Once we get far enough into the future we lose the ability to
+ # represent a timestamp with microsecond precision in a floating
+ # point number, which we do with Clock. So don't go far enough
+ # into the future.
+ max_value=datetime(2200, 1, 1),
+ ),
+ vouchers(),
+ )
+ def test_redeem_error_after_delay(self, get_config, now, voucher):
+ """
+ When ``PaymentController`` receives a non-terminal error trying to redeem
+ a voucher, after some time passes it tries to redeem the voucher
+ again.
+ """
+ clock = Clock()
+ clock.advance((now - POSIX_EPOCH).total_seconds())
+
+ store = self.useFixture(
+ TemporaryVoucherStore(
+ get_config,
+ lambda: datetime.utcfromtimestamp(clock.seconds()),
+ ),
+ ).store
+ controller = PaymentController(
+ store,
+ UnpaidRedeemer(),
+ clock,
+ )
+ controller.redeem(voucher)
+ # It fails this time.
+ self.assertThat(
+ controller.get_voucher(voucher).state,
+ MatchesAll(
+ IsInstance(model_Unpaid),
+ MatchesStructure(
+ finished=Equals(now),
+ ),
+ )
+ )
+
+ # Some time passes.
+ interval = timedelta(hours=1)
+ clock.advance(interval.total_seconds())
+
+ # It failed again.
+ self.assertThat(
+ controller.get_voucher(voucher).state,
+ MatchesAll(
+ IsInstance(model_Unpaid),
+ MatchesStructure(
+ # At the new time, demonstrating the retry was performed.
+ finished=Equals(now + interval),
+ ),
+ ),
+ )
+
NOWHERE = URL.from_text(u"https://127.0.0.1/")
diff --git a/src/_zkapauthorizer/tests/test_model.py b/src/_zkapauthorizer/tests/test_model.py
index 7414c67103fa6cd31c361c63a32947d8615e8984..d31e669a9f2fd108fa67e25aa655ef747707e246 100644
--- a/src/_zkapauthorizer/tests/test_model.py
+++ b/src/_zkapauthorizer/tests/test_model.py
@@ -79,6 +79,9 @@ from .strategies import (
random_tokens,
unblinded_tokens,
)
+from .fixtures import (
+ TemporaryVoucherStore,
+)
class VoucherStoreTests(TestCase):
@@ -107,7 +110,7 @@ class VoucherStoreTests(TestCase):
``VoucherStore.get`` raises ``KeyError`` when called with a
voucher not previously added to the store.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
self.assertThat(
lambda: store.get(voucher),
raises(KeyError),
@@ -119,7 +122,7 @@ class VoucherStoreTests(TestCase):
``VoucherStore.get`` returns a ``Voucher`` representing a voucher
previously added to the store with ``VoucherStore.add``.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
store.add(voucher, tokens)
self.assertThat(
store.get(voucher),
@@ -136,7 +139,7 @@ class VoucherStoreTests(TestCase):
More than one call to ``VoucherStore.add`` with the same argument results
in the same state as a single call.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
store.add(voucher, tokens)
store.add(voucher, [])
self.assertThat(
@@ -155,7 +158,7 @@ class VoucherStoreTests(TestCase):
``VoucherStore.list`` returns a ``list`` containing a ``Voucher`` object
for each voucher previously added.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
for voucher in vouchers:
store.add(voucher, [])
@@ -259,7 +262,7 @@ class UnblindedTokenStoreTests(TestCase):
"""
Unblinded tokens that are added to the store can later be retrieved.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
store.insert_unblinded_tokens_for_voucher(voucher_value, tokens)
retrieved_tokens = store.extract_unblinded_tokens(len(tokens))
self.expectThat(tokens, AfterPreprocessing(sorted, Equals(retrieved_tokens)))
@@ -297,7 +300,7 @@ class UnblindedTokenStoreTests(TestCase):
),
)
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
store.add(voucher_value, random)
store.insert_unblinded_tokens_for_voucher(voucher_value, unblinded)
loaded_voucher = store.get(voucher_value)
@@ -322,7 +325,7 @@ class UnblindedTokenStoreTests(TestCase):
A voucher which is reported as double-spent is marked in the database as
such.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
store.add(voucher_value, random_tokens)
store.mark_voucher_double_spent(voucher_value)
voucher = store.get(voucher_value)
@@ -362,7 +365,7 @@ class UnblindedTokenStoreTests(TestCase):
unique=True,
),
)
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
store.add(voucher_value, random)
store.insert_unblinded_tokens_for_voucher(voucher_value, unblinded)
try:
@@ -383,7 +386,7 @@ class UnblindedTokenStoreTests(TestCase):
"""
A voucher which is not known cannot be marked as double-spent.
"""
- store = store_for_test(self, get_config, lambda: now)
+ store = self.useFixture(TemporaryVoucherStore(get_config, lambda: now)).store
try:
result = store.mark_voucher_double_spent(voucher_value)
except ValueError:
diff --git a/zkapauthorizer.nix b/zkapauthorizer.nix
index 1a352e4285283753b614732062be8777706e1cf6..cfa93e1fdc7d5001281e8fcfd71d606d88981d0e 100644
--- a/zkapauthorizer.nix
+++ b/zkapauthorizer.nix
@@ -44,7 +44,6 @@ buildPythonPackage rec {
checkPhase = ''
runHook preCheck
- set -x
"${pyflakes}/bin/pyflakes" src/_zkapauthorizer
ZKAPAUTHORIZER_HYPOTHESIS_PROFILE=${hypothesisProfile'} python -m ${if collectCoverage
then "coverage run --branch --source _zkapauthorizer,twisted.plugins.zkapauthorizer --module"