diff --git a/src/_zkapauthorizer/_storage_client.py b/src/_zkapauthorizer/_storage_client.py index 3973164db6257f2d5d8b4bc1f4bc94c0b609db97..3e87eb87a0291618004346ba1830f7449c3f85a3 100644 --- a/src/_zkapauthorizer/_storage_client.py +++ b/src/_zkapauthorizer/_storage_client.py @@ -155,17 +155,26 @@ class ZKAPAuthorizerStorageClient(object): ) )) + @inlineCallbacks def renew_lease( self, storage_index, renew_secret, ): - return self._rref.callRemote( - "renew_lease", - self._get_encoded_passes(renew_lease_message(storage_index), 1), + share_sizes = (yield self._rref.callRemote( + "share_sizes", storage_index, - renew_secret, - ) + None, + )).values() + num_passes = required_passes(BYTES_PER_PASS, share_sizes) + returnValue(( + yield self._rref.callRemote( + "renew_lease", + self._get_encoded_passes(renew_lease_message(storage_index), num_passes), + storage_index, + renew_secret, + ) + )) def advise_corrupt_share( self, diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py index f76443a1e285f5743cea9a1cf0e568e71ec3ad7f..a22163048add55113a9527b7e670aa3c70686b5c 100644 --- a/src/_zkapauthorizer/_storage_server.py +++ b/src/_zkapauthorizer/_storage_server.py @@ -240,15 +240,11 @@ class ZKAPAuthorizerStorageServer(Referenceable): """ # print("server add_lease({}, {!r})".format(len(passes), storage_index)) valid_passes = self._validate_passes(add_lease_message(storage_index), passes) - allocated_sizes = dict( - get_share_sizes( - self._original, storage_index, - list(get_all_share_numbers(self._original, storage_index)), - ), - ).values() - # print("allocated_sizes: {}".format(allocated_sizes)) - check_pass_quantity(len(valid_passes), allocated_sizes) - # print("Checked out") + check_pass_quantity_for_lease( + storage_index, + valid_passes, + self._original, + ) return self._original.remote_add_lease(storage_index, *a, **kw) def remote_renew_lease(self, passes, storage_index, *a, **kw): @@ -256,7 +252,12 @@ class ZKAPAuthorizerStorageServer(Referenceable): Pass-through after a pass check to ensure clients can only extend the duration of share storage if they present valid passes. """ - self._validate_passes(renew_lease_message(storage_index), passes) + valid_passes = self._validate_passes(renew_lease_message(storage_index), passes) + check_pass_quantity_for_lease( + storage_index, + valid_passes, + self._original, + ) return self._original.remote_renew_lease(storage_index, *a, **kw) def remote_advise_corrupt_share(self, *a, **kw): @@ -362,6 +363,22 @@ def has_active_lease(storage_server, storage_index, now): ) +def check_pass_quantity_for_lease(storage_index, valid_passes, storage_server): + """ + Check that the given number of passes is sufficient to add or renew a + lease for one period for the given storage index. + """ + allocated_sizes = dict( + get_share_sizes( + storage_server, + storage_index, + list(get_all_share_numbers(storage_server, storage_index)), + ), + ).values() + # print("allocated_sizes: {}".format(allocated_sizes)) + check_pass_quantity(len(valid_passes), allocated_sizes) + # print("Checked out") + def check_pass_quantity(valid_count, share_sizes): """ Check that the given number of passes is sufficient to cover leases for diff --git a/src/_zkapauthorizer/tests/test_plugin.py b/src/_zkapauthorizer/tests/test_plugin.py index 0257683da1813e697407d9336ffdd4880ad62147..ae958bc8056fda36d1ddaea454262365a6133082 100644 --- a/src/_zkapauthorizer/tests/test_plugin.py +++ b/src/_zkapauthorizer/tests/test_plugin.py @@ -104,6 +104,9 @@ from .strategies import ( unblinded_tokens, storage_indexes, lease_renew_secrets, + lease_cancel_secrets, + sharenum_sets, + sizes, ) from .matchers import ( Provides, @@ -313,6 +316,9 @@ class ClientPluginTests(TestCase): unblinded_tokens(), storage_indexes(), lease_renew_secrets(), + lease_cancel_secrets(), + sharenum_sets(), + sizes(), ) def test_unblinded_tokens_extracted( self, @@ -323,6 +329,9 @@ class ClientPluginTests(TestCase): unblinded_token, storage_index, renew_secret, + cancel_secret, + sharenums, + size, ): """ The ``ZKAPAuthorizerStorageServer`` returned by ``get_storage_client`` @@ -344,13 +353,16 @@ class ClientPluginTests(TestCase): get_rref, ) - # This is hooked up to a garbage reference. We don't care about its - # _result_, anyway, right now. - d = storage_client.renew_lease( + # For now, merely making the call spends the passes - regardless of + # the ultimate success or failure of the operation. + storage_client.allocate_buckets( storage_index, renew_secret, + cancel_secret, + sharenums, + size, + LocalReferenceable(None), ) - d.addBoth(lambda ignored: None) # There should be no unblinded tokens left to extract. remaining = store.extract_unblinded_tokens(1) diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py index d74fa0a5bf05b59f78ffe9f207a8257adc084b14..8d28db831e18aa3dff385db6e12fe2d7208a68b2 100644 --- a/src/_zkapauthorizer/tests/test_storage_server.py +++ b/src/_zkapauthorizer/tests/test_storage_server.py @@ -89,6 +89,7 @@ from ..storage_common import ( required_passes, allocate_buckets_message, add_lease_message, + renew_lease_message, slot_testv_and_readv_and_writev_message, get_implied_data_length, get_required_new_passes_for_mutable_write, @@ -347,20 +348,26 @@ class PassValidationTests(TestCase): ), ) - @given( - storage_index=storage_indexes(), - secrets=tuples( - lease_renew_secrets(), - lease_cancel_secrets(), - ), - sharenums=sharenum_sets(), - allocated_size=sizes(), - ) - def test_add_lease_fails_without_passes(self, storage_index, secrets, sharenums, allocated_size): + def _test_lease_operation_fails_without_passes( + self, + storage_index, + secrets, + sharenums, + allocated_size, + lease_operation, + lease_operation_message, + ): """ - If ``remote_add_lease`` is invoked without supplying enough passes to - cover the storage for all shares on the given storage index, the - operation fails with ``MorePassesRequired``. + Assert that a lease-taking operation fails if it is not supplied with + enough passes to cover the cost of the lease. + + :param lease_operation: A two-argument callable. It is called with a + storage server and a list of passes. It should perform the + lease-taking operation. + + :param lease_operation_message: A one-argument callable. It is called + with a storage index. It should return the ZKAPAuthorizer binding + message for the lease-taking operation. """ # hypothesis causes our storage server to be used many times. Clean # up between iterations. @@ -381,33 +388,23 @@ class PassValidationTests(TestCase): ) # Advance time to a point where the lease is expired. This simplifies - # the logic behind how many passes will be required by the add_leases - # call (all of them). If there is prorating for partially expired - # leases then the calculation for a non-expired lease involves more - # work. + # the logic behind how many passes will be required by the lease + # operation (all of them). If there is prorating for partially + # expired leases then the calculation for a non-expired lease involves + # more work. # # Add some slop here because time.time() is used by some parts of the # system. :/ self.clock.advance(self.storage_server.LEASE_PERIOD.total_seconds() + 10.0) - # Attempt to take out a new lease with one fewer pass than is - # required. + # Attempt the lease operation with one fewer pass than is required. passes = make_passes( self.signing_key, - add_lease_message(storage_index), + lease_operation_message(storage_index), list(RandomToken.create() for i in range(required_count - 1)), ) - # print("tests add_lease({}, {!r})".format(len(passes), storage_index)) try: - result = self.storage_server.doRemoteCall( - "add_lease", ( - passes, - storage_index, - renew_secret, - cancel_secret, - ), - {}, - ) + result = lease_operation(self.storage_server, passes) except MorePassesRequired as e: self.assertThat( e, @@ -419,7 +416,6 @@ class PassValidationTests(TestCase): else: self.fail("Expected MorePassesRequired, got {}".format(result)) - @given( storage_index=storage_indexes(), secrets=tuples( @@ -429,40 +425,64 @@ class PassValidationTests(TestCase): sharenums=sharenum_sets(), allocated_size=sizes(), ) - def test_immutable_share_sizes(self, storage_index, secrets, sharenums, allocated_size): + def test_add_lease_fails_without_passes(self, storage_index, secrets, sharenums, allocated_size): """ - ``share_sizes`` returns the size of the requested iimutable shares in the - requested storage index. + If ``remote_add_lease`` is invoked without supplying enough passes to + cover the storage for all shares on the given storage index, the + operation fails with ``MorePassesRequired``. """ - # hypothesis causes our storage server to be used many times. Clean - # up between iterations. - cleanup_storage_server(self.anonymous_storage_server) - renew_secret, cancel_secret = secrets - write_toy_shares( - self.anonymous_storage_server, + def add_lease(storage_server, passes): + return storage_server.doRemoteCall( + "add_lease", ( + passes, + storage_index, + renew_secret, + cancel_secret, + ), + {}, + ) + return self._test_lease_operation_fails_without_passes( storage_index, - renew_secret, - cancel_secret, + secrets, sharenums, allocated_size, - LocalReferenceable(None), + add_lease, + add_lease_message, ) - actual_sizes = self.storage_server.doRemoteCall( - "share_sizes", ( - storage_index, - sharenums, - ), - {}, - ) - self.assertThat( - actual_sizes, - Equals({ - sharenum: allocated_size - for sharenum - in sharenums - }), + @given( + storage_index=storage_indexes(), + secrets=tuples( + lease_renew_secrets(), + lease_cancel_secrets(), + ), + sharenums=sharenum_sets(), + allocated_size=sizes(), + ) + def test_renew_lease_fails_without_passes(self, storage_index, secrets, sharenums, allocated_size): + """ + If ``remote_renew_lease`` is invoked without supplying enough passes to + cover the storage for all shares on the given storage index, the + operation fails with ``MorePassesRequired``. + """ + renew_secret, cancel_secret = secrets + def renew_lease(storage_server, passes): + return storage_server.doRemoteCall( + "renew_lease", ( + passes, + storage_index, + renew_secret, + ), + {}, + ) + return self._test_lease_operation_fails_without_passes( + storage_index, + secrets, + sharenums, + allocated_size, + renew_lease, + renew_lease_message, ) @given(