From 170ce6ecf8391f8b9327b759ab435277244ee6ac Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 11 Nov 2021 16:16:13 -0500
Subject: [PATCH] turn off allocate_buckets implicit lease behavior
This requires a newer Tahoe to provide the `set_implicit_bucket_lease_renewal`
function. Then update our wrapper to use it. Also add a test for the
behavior.
There is an effect on mutables as well since Tahoe changed how you turn off
implicit lease renewal for them as well. Just update to use the newer API to
keep things working as they were.
---
nix/sources.json | 6 +--
src/_zkapauthorizer/_storage_server.py | 17 +++++-
src/_zkapauthorizer/tests/foolscap.py | 5 +-
.../tests/test_storage_protocol.py | 53 +++++++++++++++----
4 files changed, 64 insertions(+), 17 deletions(-)
diff --git a/nix/sources.json b/nix/sources.json
index 61f2d6e..0068d88 100644
--- a/nix/sources.json
+++ b/nix/sources.json
@@ -47,10 +47,10 @@
"homepage": "https://tahoe-lafs.org/",
"owner": "tahoe-lafs",
"repo": "tahoe-lafs",
- "rev": "4bfb9d21700b8084d5fb2c697ceeb7088dd97c37",
- "sha256": "1hcp9gq5hcw43xmg7n24xx580jrg0fd382pklv79r5lr4cicyx7g",
+ "rev": "2742de6f7c1fa6cf77e35ecc5854bcf7db3e5963",
+ "sha256": "17bbkk21hfln6gw5lq29g0s3jzbmfwk3921w36shx09i8asfwn56",
"type": "tarball",
- "url": "https://github.com/tahoe-lafs/tahoe-lafs/archive/4bfb9d21700b8084d5fb2c697ceeb7088dd97c37.tar.gz",
+ "url": "https://github.com/tahoe-lafs/tahoe-lafs/archive/2742de6f7c1fa6cf77e35ecc5854bcf7db3e5963.tar.gz",
"url_template": "https://github.com/<owner>/<repo>/archive/<rev>.tar.gz"
}
}
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index 392f2eb..68cfcc2 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -173,6 +173,19 @@ class ZKAPAuthorizerStorageServer(Referenceable):
default=attr.Factory(partial(namedAny, "twisted.internet.reactor")),
)
+ def __attrs_post_init__(self):
+ # Avoid the default StorageServer ``allocate_buckets`` behavior of
+ # renewing leases on all existing shares in the same bucket. It will
+ # still add leases to the newly uploaded shares.
+ self._original.set_implicit_bucket_lease_renewal(False)
+
+ # Similarly, wrapped ``slot_testv_and_readv_and_writev_message``
+ # renews leases on all shares that are being modified. Turn that
+ # behavior off. This means we have to take responsibility for
+ # creating the initial lease on shares when they are created (and we
+ # do in our wrapper for ``slot_testv_and_readv_and_writev_message``).
+ self._original.set_implicit_slot_lease_renewal(False)
+
def remote_get_version(self):
"""
Pass-through without pass check to allow clients to learn about our
@@ -202,7 +215,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# Note: The *allocate_buckets* protocol allows for some shares to
# already exist on the server. When this is the case, the cost of the
- # operation is based only on the buckets which are really allocated
+ # operation is based only on the shares which are really allocated
# here. It's not clear if we can allow the client to supply the
# reduced number of passes in the call but we can be sure to only mark
# as spent enough passes to cover the allocated buckets. The return
@@ -360,6 +373,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
if required_new_passes > len(validation.valid):
validation.raise_for(required_new_passes)
+ self._original.set_implicit_slot_lease_renewal(renew_leases)
# Skip over the remotely exposed method and jump to the underlying
# implementation which accepts one additional parameter that we know
# about (and don't expose over the network): renew_leases. We always
@@ -370,7 +384,6 @@ class ZKAPAuthorizerStorageServer(Referenceable):
secrets,
tw_vectors,
r_vector,
- renew_leases=renew_leases,
)
def remote_slot_readv(self, *a, **kw):
diff --git a/src/_zkapauthorizer/tests/foolscap.py b/src/_zkapauthorizer/tests/foolscap.py
index 3a984be..52b745a 100644
--- a/src/_zkapauthorizer/tests/foolscap.py
+++ b/src/_zkapauthorizer/tests/foolscap.py
@@ -37,8 +37,11 @@ class RIEcho(RemoteInterface):
@implementer(RIStorageServer)
class StubStorageServer(object):
- pass
+ def set_implicit_bucket_lease_renewal(self, enabled):
+ pass
+ def set_implicit_slot_lease_renewal(self, enabled):
+ pass
def get_anonymous_storage_server():
return StubStorageServer()
diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py
index a77dc29..8d9b89e 100644
--- a/src/_zkapauthorizer/tests/test_storage_protocol.py
+++ b/src/_zkapauthorizer/tests/test_storage_protocol.py
@@ -19,6 +19,7 @@ Tests for communication between the client and server components.
from __future__ import absolute_import
from allmydata.storage.common import storage_index_to_dir
+from allmydata.storage.shares import get_share_file
from challenge_bypass_ristretto import random_signing_key
from fixtures import MonkeyPatch
from foolscap.referenceable import LocalReferenceable
@@ -357,6 +358,8 @@ class ShareTests(TestCase):
cancel_secret=lease_cancel_secrets(),
existing_sharenums=sharenum_sets(),
additional_sharenums=sharenum_sets(),
+ when=posix_timestamps(),
+ interval=integers(min_value=1, max_value=60 * 60 * 24 * 31),
size=sizes(),
)
def test_shares_already_exist(
@@ -366,6 +369,8 @@ class ShareTests(TestCase):
cancel_secret,
existing_sharenums,
additional_sharenums,
+ when,
+ interval,
size,
):
"""
@@ -374,14 +379,22 @@ class ShareTests(TestCase):
"""
# A helper that only varies on sharenums.
def allocate_buckets(sharenums):
- return self.client.allocate_buckets(
- storage_index,
- renew_secret,
- cancel_secret,
- sharenums,
- size,
- canary=self.canary,
+ alreadygot, writers = extract_result(
+ self.client.allocate_buckets(
+ storage_index,
+ renew_secret,
+ cancel_secret,
+ sharenums,
+ size,
+ canary=self.canary,
+ ),
)
+ for sharenum, writer in writers.items():
+ writer.remote_write(0, bytes_for_share(sharenum, size))
+ writer.remote_close()
+
+ # Set some arbitrary time so we can inspect lease renewal behavior.
+ self.clock.advance(when)
# Create some shares to alter the behavior of the next
# allocate_buckets.
@@ -395,14 +408,15 @@ class ShareTests(TestCase):
canary=self.canary,
)
+ # Let some time pass so leases added after this point will look
+ # different from leases added before this point.
+ self.clock.advance(interval)
+
# Do a partial repeat of the operation. Shuffle around
# the shares in some random-ish way. If there is partial overlap
# there should be partial spending.
all_sharenums = existing_sharenums | additional_sharenums
- self.assertThat(
- allocate_buckets(all_sharenums),
- succeeded(Always()),
- )
+ allocate_buckets(all_sharenums)
# This is what the client should try to spend. This should also match
# the total number of passes issued during the test.
@@ -435,6 +449,23 @@ class ShareTests(TestCase):
),
)
+ def get_lease_grant_times(storage_server, storage_index):
+ for sharenum, sharepath in storage_server._get_bucket_shares(storage_index):
+ yield sharenum, list(lease.get_grant_renew_time_time() for lease in get_share_file(sharepath).get_leases())
+
+ expected_leases = {}
+ # Chop off the non-integer part of the expected values because share
+ # files only keep integer precision.
+ expected_leases.update({sharenum: [int(when)] for sharenum in existing_sharenums})
+ expected_leases.update({
+ sharenum: [int(when + interval)] for sharenum in all_sharenums - existing_sharenums
+ })
+
+ self.assertThat(
+ dict(get_lease_grant_times(self.anonymous_storage_server, storage_index)),
+ Equals(expected_leases),
+ )
+
@given(
storage_index=storage_indexes(),
renew_secrets=tuples(lease_renew_secrets(), lease_renew_secrets()),
--
GitLab