From 14fdd77e9edebcf345cbd3614a16d73c88dab374 Mon Sep 17 00:00:00 2001
From: Jean-Paul Calderone <exarkun@twistedmatrix.com>
Date: Tue, 23 Nov 2021 16:23:56 -0500
Subject: [PATCH] Stop considering existing shares when computing mutable
operation cost
now client and server agree that only shares with associated writes in
`tw_vectors` are considered when computing cost
---
src/_zkapauthorizer/_storage_client.py | 8 ++++++--
src/_zkapauthorizer/_storage_server.py | 22 +++-------------------
src/_zkapauthorizer/storage_common.py | 2 +-
src/_zkapauthorizer/tests/strategies.py | 3 +++
4 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/src/_zkapauthorizer/_storage_client.py b/src/_zkapauthorizer/_storage_client.py
index 21e2fed..8b41b21 100644
--- a/src/_zkapauthorizer/_storage_client.py
+++ b/src/_zkapauthorizer/_storage_client.py
@@ -40,9 +40,9 @@ from .storage_common import (
add_lease_message,
allocate_buckets_message,
get_required_new_passes_for_mutable_write,
- has_writes,
pass_value_attribute,
required_passes,
+ get_write_sharenums,
slot_testv_and_readv_and_writev_message,
)
@@ -443,7 +443,8 @@ class ZKAPAuthorizerStorageClient(object):
) in tw_vectors.items()
}
- if has_writes(tw_vectors):
+ write_sharenums = get_write_sharenums(tw_vectors)
+ if len(write_sharenums) > 0:
# When performing writes, if we're increasing the storage
# requirement, we need to spend more passes. Unfortunately we
# don't know what the current storage requirements are at this
@@ -465,6 +466,9 @@ class ZKAPAuthorizerStorageClient(object):
sharenum: stat.size
for (sharenum, stat) in stats.items()
if stat.lease_expiration > now
+ # Also, the size of any share we're not writing to doesn't
+ # matter.
+ and sharenum in write_sharenums
}
# Determine the cost of the new storage for the operation.
num_passes = get_required_new_passes_for_mutable_write(
diff --git a/src/_zkapauthorizer/_storage_server.py b/src/_zkapauthorizer/_storage_server.py
index 607daca..367120a 100644
--- a/src/_zkapauthorizer/_storage_server.py
+++ b/src/_zkapauthorizer/_storage_server.py
@@ -56,6 +56,7 @@ from .storage_common import (
get_required_new_passes_for_mutable_write,
pass_value_attribute,
required_passes,
+ get_write_sharenums,
slot_testv_and_readv_and_writev_message,
)
@@ -763,25 +764,8 @@ def get_writev_price(storage_server, pass_value, storage_index, tw_vectors, now)
get_share_sizes(
storage_server,
storage_index,
- # Consider the size of *all* shares even if they're not being
- # written. If they have an unexpired lease then we can apply some
- # or all of the remainder of the value of that lease towards this
- # operation.
- #
- # For example, if there are 5 0.5 MB shares already present then 5
- # ZKAPs (assuming a ZKAP value of 1 MB-month) have been spent to
- # store them while only 2.5 MB of storage has been allocated.
- # This means we could accept another 2.5 MB of data for storage at
- # no cost to the client while still having been completely paid
- # for the storage committed.
- #
- # This is sort of a weird trick to try to reduce the impact of
- # ZKAP value lost to the rounding scheme in use. There's a good
- # chance there are better solutions waiting to be discovered that
- # would obsolete this logic. Note that whatever the logic here,
- # it must agree with the client-side logic so that both server and
- # client come to the same conclusion regarding price.
- sharenums=None,
+ # Here's how we restrict the result to only written shares.
+ sharenums=get_write_sharenums(tw_vectors),
),
)
diff --git a/src/_zkapauthorizer/storage_common.py b/src/_zkapauthorizer/storage_common.py
index 554c355..5060f3a 100644
--- a/src/_zkapauthorizer/storage_common.py
+++ b/src/_zkapauthorizer/storage_common.py
@@ -203,7 +203,7 @@ def has_writes(tw_vectors):
)
-def get_sharenums(tw_vectors):
+def get_write_sharenums(tw_vectors):
"""
:param tw_vectors: See
``allmydata.interfaces.TestAndWriteVectorsForShares``.
diff --git a/src/_zkapauthorizer/tests/strategies.py b/src/_zkapauthorizer/tests/strategies.py
index 567c944..d14a62d 100644
--- a/src/_zkapauthorizer/tests/strategies.py
+++ b/src/_zkapauthorizer/tests/strategies.py
@@ -809,7 +809,10 @@ def slot_test_and_write_vectors():
slot_test_vectors(),
slot_data_vectors(),
one_of(
+ # The new length might be omitted completely.
just(None),
+ # Or it might be given as an integer. Allow a zero size which
+ # means "delete this share" in this context.
sizes(),
),
)
--
GitLab