diff --git a/src/_zkapauthorizer/_plugin.py b/src/_zkapauthorizer/_plugin.py
index 95a45a4388485c136e00087d29db949046e84271..1f96295374e30064d0c51e9ade02bfab3cc135df 100644
--- a/src/_zkapauthorizer/_plugin.py
+++ b/src/_zkapauthorizer/_plugin.py
@@ -140,7 +140,8 @@ class ZKAPAuthorizer(object):
registry=registry,
)
storage_server = ZKAPAuthorizerStorageServer(
- anonymous_storage_server,
+ # unwrap the Foolscap layer, we'll do it ourselves.
+ anonymous_storage_server._server,
pass_value=pass_value,
signing_key=signing_key,
spender=spender,
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index a2c8e1f596fdab64f01706a131b9973cf50a3947..ff61dbe17f5f8faf14120aa2d01e3ac516466371 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -27,12 +27,17 @@ from functools import partial
from os import listdir, stat
from os.path import join
from struct import calcsize, unpack
-from typing import Dict, List, Optional
+from typing import Any, Dict, List, Optional, Tuple
import attr
from allmydata.interfaces import RIStorageServer, TestAndWriteVectorsForShares
from allmydata.storage.common import storage_index_to_dir
-from allmydata.storage.immutable import ShareFile, FoolscapBucketWriter
+from allmydata.storage.immutable import (
+ BucketWriter,
+ FoolscapBucketReader,
+ FoolscapBucketWriter,
+ ShareFile,
+)
from allmydata.storage.lease import LeaseInfo
from allmydata.storage.mutable import MutableShareFile
from allmydata.storage.server import StorageServer
@@ -47,6 +52,7 @@ from challenge_bypass_ristretto import (
)
from eliot import log_call, start_action
from foolscap.api import Referenceable
+from foolscap.ipb import IRemoteReference
from prometheus_client import CollectorRegistry, Histogram
from twisted.internet.defer import Deferred
from twisted.internet.interfaces import IReactorTime
@@ -189,7 +195,10 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# control it ourselves.
LEASE_PERIOD = timedelta(days=31)
- _original = attr.ib(validator=provides(RIStorageServer))
+ # A StorageServer instance, but not validated because of the fake used in
+ # the test suite.
+ _original = attr.ib()
+
_pass_value = pass_value_attribute()
_signing_key = attr.ib(validator=instance_of(SigningKey))
_spender = attr.ib(validator=provides(ISpender))
@@ -203,6 +212,12 @@ class ZKAPAuthorizerStorageServer(Referenceable):
)
_public_key = attr.ib(init=False)
_metric_spending_successes = attr.ib(init=False)
+ _bucket_writer_disconnect_markers: Dict[
+ BucketWriter, Tuple[IRemoteReference, Any]
+ ] = attr.ib(
+ init=False,
+ default=attr.Factory(dict),
+ )
@_public_key.default
def _get_public_key(self):
@@ -211,6 +226,14 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# so that `self._signing_key` will be assigned when this runs.
return PublicKey.from_signing_key(self._signing_key)
+ def _bucket_writer_closed(self, bw):
+ if bw in self._bucket_writer_disconnect_markers:
+ canary, disconnect_marker = self._bucket_writer_disconnect_markers.pop(bw)
+ canary.dontNotifyOnDisconnect(disconnect_marker)
+
+ def __attrs_post_init__(self):
+ self._original.register_bucket_writer_close_handler(self._bucket_writer_closed)
+
def _get_spending_histogram_buckets(self):
"""
Create the upper bounds for the ZKAP spending histogram.
@@ -253,7 +276,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through without pass check to allow clients to learn about our
version and configuration in case it helps them decide how to behave.
"""
- return self._original._server.get_version()
+ return self._original.get_version()
def remote_allocate_buckets(
self,
@@ -302,7 +325,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
allocated_size,
)
- alreadygot, bucketwriters = self._original._server.allocate_buckets(
+ alreadygot, bucketwriters = self._original.allocate_buckets(
storage_index,
renew_secret,
cancel_secret,
@@ -330,7 +353,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# StorageServer.remote_allocate_buckets.
for bw in bucketwriters.values():
disconnect_marker = canary.notifyOnDisconnect(bw.disconnected)
- self._original._bucket_writer_disconnect_markers[bw] = (
+ self._bucket_writer_disconnect_markers[bw] = (
canary,
disconnect_marker,
)
@@ -339,8 +362,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
validation.valid[:spent_passes],
)
return alreadygot, {
- k: FoolscapBucketWriter(bw)
- for (k, bw) in bucketwriters.items()
+ k: FoolscapBucketWriter(bw) for (k, bw) in bucketwriters.items()
}
def remote_get_buckets(self, storage_index):
@@ -348,7 +370,10 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through without pass check to let clients read immutable shares as
long as those shares exist.
"""
- return self._original.remote_get_buckets(storage_index)
+ return {
+ k: FoolscapBucketReader(bucket)
+ for (k, bucket) in self._original.get_buckets(storage_index).items()
+ }
def remote_add_lease(self, passes, storage_index, *a, **kw):
"""
@@ -364,9 +389,9 @@ class ZKAPAuthorizerStorageServer(Referenceable):
self._pass_value,
storage_index,
validation,
- self._original._server,
+ self._original,
)
- result = self._original._server.add_lease(storage_index, *a, **kw)
+ result = self._original.add_lease(storage_index, *a, **kw)
self._spender.mark_as_spent(
self._public_key,
validation.valid,
@@ -379,7 +404,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through without a pass check to let clients inform us of possible
issues with the system without incurring any cost to themselves.
"""
- return self._original._server.advise_corrupt_share(*a, **kw)
+ return self._original.advise_corrupt_share(*a, **kw)
def remote_share_sizes(self, storage_index_or_slot, sharenums):
with start_action(
@@ -387,13 +412,13 @@ class ZKAPAuthorizerStorageServer(Referenceable):
storage_index_or_slot=storage_index_or_slot,
):
return dict(
- get_share_sizes(self._original._server, storage_index_or_slot, sharenums)
+ get_share_sizes(self._original, storage_index_or_slot, sharenums)
)
def remote_stat_shares(self, storage_indexes_or_slots):
# type: (List[bytes]) -> List[Dict[int, ShareStat]]
return list(
- dict(get_share_stats(self._original._server, storage_index_or_slot, None))
+ dict(get_share_stats(self._original, storage_index_or_slot, None))
for storage_index_or_slot in storage_indexes_or_slots
)
@@ -472,7 +497,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# Inspect the operation to determine its price based on any
# allocations.
required_new_passes = get_writev_price(
- self._original._server,
+ self._original,
self._pass_value,
storage_index,
tw_vectors,
@@ -485,7 +510,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
validation.raise_for(required_new_passes)
# Perform the operation.
- result = self._original._server.slot_testv_and_readv_and_writev(
+ result = self._original.slot_testv_and_readv_and_writev(
storage_index,
secrets,
tw_vectors,
@@ -506,7 +531,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
# difference but this only grants storage for the remainder of the
# existing lease period. This results in the client being overcharged
# somewhat.
- add_leases_for_writev(self._original._server, storage_index, secrets, tw_vectors, now)
+ add_leases_for_writev(self._original, storage_index, secrets, tw_vectors, now)
self._spender.mark_as_spent(
self._public_key,
@@ -524,7 +549,7 @@ class ZKAPAuthorizerStorageServer(Referenceable):
Pass-through without a pass check to let clients read mutable shares as
long as those shares exist.
"""
- return self._original._server.slot_readv(*a, **kw)
+ return self._original.slot_readv(*a, **kw)
def check_pass_quantity(pass_value, validation, share_sizes):
diff --git a/src/_zkapauthorizer/tests/fixtures.py b/src/_zkapauthorizer/tests/fixtures.py
index aa967e27fdcf791a14bd249f4c40af82634cfe50..a01dc6ff430eef143a73c0b0242767514b136dbb 100644
--- a/src/_zkapauthorizer/tests/fixtures.py
+++ b/src/_zkapauthorizer/tests/fixtures.py
@@ -19,7 +19,7 @@ Common fixtures to let the test suite focus on application logic.
from base64 import b64encode
import attr
-from allmydata.storage.server import StorageServer, FoolscapStorageServer
+from allmydata.storage.server import StorageServer
from fixtures import Fixture, TempDir
from twisted.internet.task import Clock
from twisted.python.filepath import FilePath
@@ -38,9 +38,6 @@ class AnonymousStorageServer(Fixture):
:ivar backend: The protocol-agnostic storage server backend.
- :ivar anonymous_foolscap_server: The Foolscap-based server wrapped around
- the backend.
-
:ivar clock: The ``IReactorTime`` provider to supply to ``StorageServer``
for its time-checking needs.
"""
@@ -49,7 +46,6 @@ class AnonymousStorageServer(Fixture):
tempdir: FilePath = attr.ib(default=None)
backend: StorageServer = attr.ib(default=None)
- anonymous_foolscap_server: FoolscapStorageServer = attr.ib(default=None)
def _setUp(self):
self.tempdir = FilePath(self.useFixture(TempDir()).join(u"storage"))
@@ -58,9 +54,6 @@ class AnonymousStorageServer(Fixture):
b"x" * 20,
clock=self.clock,
)
- self.anonymous_foolscap_server = FoolscapStorageServer(
- self.backend,
- )
@attr.s
diff --git a/src/_zkapauthorizer/tests/foolscap.py b/src/_zkapauthorizer/tests/foolscap.py
index 8ad3345c45c44634052bb680032c4f8c0c18ede1..dba7e9fdd965eab41304ed1c1acfaa774f2b2156 100644
--- a/src/_zkapauthorizer/tests/foolscap.py
+++ b/src/_zkapauthorizer/tests/foolscap.py
@@ -18,6 +18,7 @@ Testing helpers related to Foolscap.
import attr
from allmydata.interfaces import RIStorageServer
+from allmydata.storage.server import StorageServer
from foolscap.api import Any, Copyable, Referenceable, RemoteInterface
from foolscap.copyable import CopyableSlicer, ICopyable
from twisted.internet.defer import fail, succeed
@@ -33,13 +34,19 @@ class RIEcho(RemoteInterface):
return Any()
+class StubStorageBackend(object):
+ def register_bucket_writer_close_handler(self, handler):
+ pass
+
+
@implementer(RIStorageServer)
-class StubStorageServer(object):
- pass
+@attr.s
+class StubFoolscapStorageServer(object):
+ _server = attr.ib(default=attr.Factory(StubStorageBackend))
def get_anonymous_storage_server():
- return StubStorageServer()
+ return StubFoolscapStorageServer()
class BrokenCopyable(Copyable):
diff --git a/src/_zkapauthorizer/tests/storage_common.py b/src/_zkapauthorizer/tests/storage_common.py
index a168395ca073ba197ece412a39dbcbf346c2a3f6..6538b3bc4afee1cab06b5f9b57c601eb79dbefd8 100644
--- a/src/_zkapauthorizer/tests/storage_common.py
+++ b/src/_zkapauthorizer/tests/storage_common.py
@@ -60,7 +60,6 @@ def write_toy_shares(
cancel_secret,
sharenums,
size,
- canary,
):
"""
Write some immutable shares to the given storage server.
@@ -71,19 +70,17 @@ def write_toy_shares(
:param bytes cancel_secret:
:param set[int] sharenums:
:param int size:
- :param IRemoteReference canary:
"""
- _, allocated = storage_server.remote_allocate_buckets(
+ _, allocated = storage_server.allocate_buckets(
storage_index,
renew_secret,
cancel_secret,
sharenums,
size,
- canary=canary,
)
for (sharenum, writer) in allocated.items():
- writer.remote_write(0, bytes_for_share(sharenum, size))
- writer.remote_close()
+ writer.write(0, bytes_for_share(sharenum, size))
+ writer.close()
def whitebox_write_sparse_share(sharepath, version, size, leases, now):
diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py
index b6a8c4dace48c96dedaf29c6de6da9d8c6e898aa..2ee75aaae77a34dadf1c239c2397ceddcd852fc7 100644
--- a/src/_zkapauthorizer/tests/test_storage_protocol.py
+++ b/src/_zkapauthorizer/tests/test_storage_protocol.py
@@ -160,7 +160,7 @@ class ShareTests(TestCase):
self.spending_recorder, spender = RecordingSpender.make()
self.server = ZKAPAuthorizerStorageServer(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
self.pass_value,
self.signing_key,
spender,
@@ -410,13 +410,12 @@ class ShareTests(TestCase):
# Create some shares to alter the behavior of the next
# allocate_buckets.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
renew_secret,
cancel_secret,
existing_sharenums,
size,
- canary=self.canary,
)
# Let some time pass so leases added after this point will look
@@ -504,13 +503,12 @@ class ShareTests(TestCase):
# Create a share we can toy with.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
add_lease_secret,
cancel_secret,
sharenums,
size,
- canary=self.canary,
)
self.assertThat(
@@ -592,13 +590,12 @@ class ShareTests(TestCase):
when,
leases,
lambda storage, storage_index, sharenums, size, canary: write_toy_shares(
- storage.anonymous_foolscap_server,
+ storage.backend,
storage_index,
renew_secret,
cancel_secret,
sharenums,
size,
- canary,
),
)
@@ -819,13 +816,12 @@ class ShareTests(TestCase):
"""
# Create a share we can toy with.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
renew_secret,
cancel_secret,
{sharenum},
size,
- canary=self.canary,
)
self.assertThat(
@@ -956,9 +952,7 @@ class ShareTests(TestCase):
def leases():
return list(
lease.to_mutable_data()
- for lease in self.storage.backend.get_slot_leases(
- storage_index
- )
+ for lease in self.storage.backend.get_slot_leases(storage_index)
)
def write():
diff --git a/src/_zkapauthorizer/tests/test_storage_server.py b/src/_zkapauthorizer/tests/test_storage_server.py
index 055a4be53be1b01e29c21432171e21f4648ad0a0..defab2b6842a34ffc513cc9a444a61a6ea0dd890 100644
--- a/src/_zkapauthorizer/tests/test_storage_server.py
+++ b/src/_zkapauthorizer/tests/test_storage_server.py
@@ -19,8 +19,8 @@ Tests for ``_zkapauthorizer._storage_server``.
from random import shuffle
from time import time
-from allmydata.storage.mutable import MutableShareFile
from allmydata.interfaces import NoSpace
+from allmydata.storage.mutable import MutableShareFile
from challenge_bypass_ristretto import PublicKey, random_signing_key
from foolscap.referenceable import LocalReferenceable
from hypothesis import given, note
@@ -202,7 +202,7 @@ class PassValidationTests(TestCase):
self.signing_key
).encode_base64()
self.storage_server = ZKAPAuthorizerStorageServer(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
self.pass_value,
self.signing_key,
spender,
@@ -532,13 +532,12 @@ class PassValidationTests(TestCase):
)
# Create some shares at a slot which will require lease renewal.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
renew_secret,
cancel_secret,
sharenums,
allocated_size,
- LocalReferenceable(None),
)
# Advance time to a point where the lease is expired. This simplifies
@@ -806,13 +805,12 @@ class PassValidationTests(TestCase):
# the subsequent `allocate_buckets` operation - but of which the
# client is unaware.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
renew_secret,
cancel_secret,
existing_sharenums,
size,
- LocalReferenceable(None),
)
# The client will present this many passes.
@@ -877,13 +875,12 @@ class PassValidationTests(TestCase):
):
# Create some shares at a slot which will require lease renewal.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
renew_secret,
cancel_secret,
sharenums,
allocated_size,
- LocalReferenceable(None),
)
num_passes = required_passes(
@@ -941,13 +938,12 @@ class PassValidationTests(TestCase):
# Put some shares up there to target with the add_lease operation.
write_toy_shares(
- self.storage.anonymous_foolscap_server,
+ self.storage.backend,
storage_index,
renew_secret,
cancel_secret,
sharenums,
allocated_size,
- LocalReferenceable(None),
)
num_passes = required_passes(