From f901806559cce23b4a1c21c3e8447419a708a97a Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Thu, 10 Oct 2019 13:00:01 -0400
Subject: [PATCH] Avoid spending passes on slot_testv_and_readv_and_writev
 calls without writes

---
 src/_zkapauthorizer/_storage_client.py         |  7 ++++++-
 src/_zkapauthorizer/_storage_server.py         |  4 +++-
 .../tests/test_storage_protocol.py             | 18 +++++++++++++++---
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/src/_zkapauthorizer/_storage_client.py b/src/_zkapauthorizer/_storage_client.py
index 7eebdc5..eafc119 100644
--- a/src/_zkapauthorizer/_storage_client.py
+++ b/src/_zkapauthorizer/_storage_client.py
@@ -37,6 +37,7 @@ from .storage_common import (
     add_lease_message,
     renew_lease_message,
     slot_testv_and_readv_and_writev_message,
+    has_writes,
 )
 
 @implementer(IStorageServer)
@@ -170,9 +171,13 @@ class ZKAPAuthorizerStorageClient(object):
             tw_vectors,
             r_vector,
     ):
+        if has_writes(tw_vectors):
+            passes = self._get_encoded_passes(slot_testv_and_readv_and_writev_message(storage_index), 1)
+        else:
+            passes = []
         return self._rref.callRemote(
             "slot_testv_and_readv_and_writev",
-            self._get_encoded_passes(slot_testv_and_readv_and_writev_message(storage_index), 1),
+            passes,
             storage_index,
             secrets,
             tw_vectors,
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index 7ae60c3..b95eb3b 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -246,7 +246,9 @@ class ZKAPAuthorizerStorageServer(Referenceable):
                 self._clock.seconds(),
             ):
                 # Passes may be supplied with the write to create the
-                # necessary lease as part of the same operation.
+                # necessary lease as part of the same operation.  This must be
+                # supported because there is no separate protocol action to
+                # *create* a slot.  Clients just begin writing to it.
                 valid_passes = self._validate_passes(
                     slot_testv_and_readv_and_writev_message(storage_index),
                     passes,
diff --git a/src/_zkapauthorizer/tests/test_storage_protocol.py b/src/_zkapauthorizer/tests/test_storage_protocol.py
index c736881..d0fe8bb 100644
--- a/src/_zkapauthorizer/tests/test_storage_protocol.py
+++ b/src/_zkapauthorizer/tests/test_storage_protocol.py
@@ -159,14 +159,21 @@ def assume_one_pass(test_and_write_vectors_for_shares):
 class ShareTests(TestCase):
     """
     Tests for interaction with shares.
+
+    :ivar int spent_passes: The number of passes which have been spent so far
+        in the course of a single test (in the case of Hypothesis, every
+        iteration of the test so far, probably; so make relative comparisons
+        instead of absolute ones).
     """
     def setUp(self):
         super(ShareTests, self).setUp()
         self.canary = LocalReferenceable(None)
         self.anonymous_storage_server = self.useFixture(AnonymousStorageServer()).storage_server
         self.signing_key = random_signing_key()
+        self.spent_passes = 0
 
         def get_passes(message, count):
+            self.spent_passes += count
             return list(
                 Pass(pass_.decode("ascii"))
                 for pass_
@@ -397,7 +404,7 @@ class ShareTests(TestCase):
     def test_create_mutable(self, storage_index, secrets, test_and_write_vectors_for_shares):
         """
         Mutable share data written using *slot_testv_and_readv_and_writev* can be
-        read back.
+        read back as-written and without spending any more passes.
         """
         # XXX
         assume_one_pass(test_and_write_vectors_for_shares)
@@ -418,19 +425,24 @@ class ShareTests(TestCase):
                 r_vector=[],
             ),
         )
-
         self.assertThat(
             wrote,
             Equals(True),
             u"Server rejected a write to a new mutable slot",
         )
-
         self.assertThat(
             read,
             Equals({}),
             u"Server gave back read results when we asked for none.",
         )
+        # Now we can read it back without spending any more passes.
+        before_spent_passes = self.spent_passes
         assert_read_back_data(self, storage_index, secrets, test_and_write_vectors_for_shares)
+        after_spent_passes = self.spent_passes
+        self.assertThat(
+            before_spent_passes,
+            Equals(after_spent_passes),
+        )
 
     @given(
         storage_index=storage_indexes(),
-- 
GitLab