stabilize on 20-byte nodeids everywhere, printed with foolscap's base32

diff --git a/docs/mutable.txt b/docs/mutable.txt
index 2d64d60d5343c79ede2db35a7397e53c35cf8192..a67e8973ec5ee7bd8ca63380915348f02ede4188 100644 (file)
--- a/docs/mutable.txt
+++ b/docs/mutable.txt
@@ -223,19 +223,19 @@ data itself, and expansion space for additional lease structures.
 
  #   offset    size    name
  1   0         32      magic verstr "tahoe mutable container v1" plus binary
- 2   32        32      write enabler's nodeid
- 3   64        32      write enabler
- 4   72        8       data size (actual share data present) (a)
- 5   80        8       offset of (8) count of extra leases (after data)
- 6   88        416     four leases, 104 bytes each
+ 2   32        20      write enabler's nodeid
+ 3   52        32      write enabler
+ 4   84        8       data size (actual share data present) (a)
+ 5   92        8       offset of (8) count of extra leases (after data)
+ 6   100       368     four leases, 92 bytes each
                         0    4   ownerid (0 means "no lease here")
                         4    4   expiration timestamp
                         8   32   renewal token
                         40  32   cancel token
-                        72  32   nodeid which accepted the tokens
- 7   504       (a)     data
+                        72  20   nodeid which accepted the tokens
+ 7   468       (a)     data
  8   ??        4       count of extra leases
- 9   ??        n*104    extra leases
+ 9   ??        n*92    extra leases
 
 The "extra leases" field must be copied and rewritten each time the size of
 the enclosed data changes. The hope is that most buckets will have four or

diff --git a/src/allmydata/interfaces.py b/src/allmydata/interfaces.py
index 51999b63742f4a4db5dfef452d89d37391b51eb4..d4cf522f59a5d9fe330f3a1fb26d73575859bd87 100644 (file)
--- a/src/allmydata/interfaces.py
+++ b/src/allmydata/interfaces.py
@@ -215,6 +215,9 @@ class RIStorageServer(RemoteInterface):
 
          The write enabler was recorded by nodeid '%s'.
 
+        Note that the nodeid here is encoded using the same base32 encoding
+        used by Foolscap and allmydata.util.idlib.nodeid_b2a().
+
         """
         return TupleOf(bool, DictOf(int, ReadData))
 

diff --git a/src/allmydata/mutable.py b/src/allmydata/mutable.py
index e27a29806149d1aa5d001e5c6f39e7b411e92195..e207de59fc303effad27993e88e2fa8b8f23a201 100644 (file)
--- a/src/allmydata/mutable.py
+++ b/src/allmydata/mutable.py
@@ -27,9 +27,7 @@ class CorruptShareError(Exception):
         self.shnum = shnum
         self.reason = reason
     def __repr__(self):
-        # TODO: in some places we use idlib.b2a, in others (foolscap) we use
-        # stdlib b32encode. Fix this discrepancy.
-        short_peerid = idlib.b2a(self.peerid)[:8]
+        short_peerid = idlib.nodeid_b2a(self.peerid)[:8]
         return "<CorruptShareError peerid=%s shnum[%d]: %s" % (short_peerid,
                                                                self.shnum,
                                                                self.reason)
@@ -932,16 +930,19 @@ class MutableFileNode:
         crypttext = enc.encrypt(privkey)
         return crypttext
 
-    def get_write_enabler(self, nodeid):
-        return hashutil.ssk_write_enabler_hash(self._writekey, nodeid)
-    def get_renewal_secret(self, nodeid):
+    def get_write_enabler(self, peerid):
+        assert len(peerid) == 20
+        return hashutil.ssk_write_enabler_hash(self._writekey, peerid)
+    def get_renewal_secret(self, peerid):
+        assert len(peerid) == 20
         crs = self._client.get_renewal_secret()
         frs = hashutil.file_renewal_secret_hash(crs, self._storage_index)
-        return hashutil.bucket_renewal_secret_hash(frs, nodeid)
-    def get_cancel_secret(self, nodeid):
+        return hashutil.bucket_renewal_secret_hash(frs, peerid)
+    def get_cancel_secret(self, peerid):
+        assert len(peerid) == 20
         ccs = self._client.get_cancel_secret()
         fcs = hashutil.file_cancel_secret_hash(ccs, self._storage_index)
-        return hashutil.bucket_cancel_secret_hash(fcs, nodeid)
+        return hashutil.bucket_cancel_secret_hash(fcs, peerid)
 
     def get_writekey(self):
         return self._writekey

diff --git a/src/allmydata/storage.py b/src/allmydata/storage.py
index bd9949caa5d11566e13e25e899634f6c2eb0d6e1..85dd5dc0a56ee6a755872885cf2dbdf5112425a6 100644 (file)
--- a/src/allmydata/storage.py
+++ b/src/allmydata/storage.py
@@ -222,19 +222,19 @@ class BucketReader(Referenceable):
 
 # #   offset    size    name
 # 1   0         32      magic verstr "tahoe mutable container v1" plus binary
-# 2   32        32      write enabler's nodeid
-# 3   64        32      write enabler
-# 4   96        8       data size (actual share data present) (a)
-# 5   104       8       offset of (8) count of extra leases (after data)
-# 6   112       416     four leases, 104 bytes each
+# 2   32        20      write enabler's nodeid
+# 3   52        32      write enabler
+# 4   84        8       data size (actual share data present) (a)
+# 5   92        8       offset of (8) count of extra leases (after data)
+# 6   100       368     four leases, 92 bytes each
 #                        0    4   ownerid (0 means "no lease here")
 #                        4    4   expiration timestamp
 #                        8   32   renewal token
 #                        40  32   cancel token
-#                        72  32   nodeid which accepted the tokens
-# 7   528       (a)     data
+#                        72  20   nodeid which accepted the tokens
+# 7   468       (a)     data
 # 8   ??        4       count of extra leases
-# 9   ??        n*104    extra leases
+# 9   ??        n*92    extra leases
 
 
 assert struct.calcsize("L"), 4
@@ -242,13 +242,13 @@ assert struct.calcsize("Q"), 8
 
 class MutableShareFile:
 
-    DATA_LENGTH_OFFSET = struct.calcsize(">32s32s32s")
+    DATA_LENGTH_OFFSET = struct.calcsize(">32s20s32s")
     EXTRA_LEASE_OFFSET = DATA_LENGTH_OFFSET + 8
-    HEADER_SIZE = struct.calcsize(">32s32s32sQQ") # doesn't include leases
-    LEASE_SIZE = struct.calcsize(">LL32s32s32s")
-    assert LEASE_SIZE == 104
+    HEADER_SIZE = struct.calcsize(">32s20s32sQQ") # doesn't include leases
+    LEASE_SIZE = struct.calcsize(">LL32s32s20s")
+    assert LEASE_SIZE == 92
     DATA_OFFSET = HEADER_SIZE + 4*LEASE_SIZE
-    assert DATA_OFFSET == 528, DATA_OFFSET
+    assert DATA_OFFSET == 468, DATA_OFFSET
     # our sharefiles share with a recognizable string, plus some random
     # binary data to reduce the chance that a regular text file will look
     # like a sharefile.
@@ -266,7 +266,7 @@ class MutableShareFile:
             (magic,
              write_enabler_nodeid, write_enabler,
              data_length, extra_least_offset) = \
-             struct.unpack(">32s32s32sQQ", data)
+             struct.unpack(">32s20s32sQQ", data)
             assert magic == self.MAGIC
 
 
@@ -279,7 +279,7 @@ class MutableShareFile:
         assert extra_lease_offset == self.DATA_OFFSET # true at creation
         num_extra_leases = 0
         f = open(self.home, 'wb')
-        header = struct.pack(">32s32s32sQQ",
+        header = struct.pack(">32s20s32sQQ",
                              self.MAGIC, my_nodeid, write_enabler,
                              data_length, extra_lease_offset,
                              )
@@ -400,7 +400,7 @@ class MutableShareFile:
                       + (lease_number-4)*self.LEASE_SIZE)
         f.seek(offset)
         assert f.tell() == offset
-        f.write(struct.pack(">LL32s32s32s",
+        f.write(struct.pack(">LL32s32s20s",
                             ownerid, int(expiration_time),
                             renew_secret, cancel_secret, nodeid))
 
@@ -419,7 +419,7 @@ class MutableShareFile:
         f.seek(offset)
         assert f.tell() == offset
         data = f.read(self.LEASE_SIZE)
-        lease_info = struct.unpack(">LL32s32s32s", data)
+        lease_info = struct.unpack(">LL32s32s20s", data)
         (ownerid, expiration_time,
          renew_secret, cancel_secret, nodeid) = lease_info
         if ownerid == 0:
@@ -487,7 +487,7 @@ class MutableShareFile:
         # original server to a new one.
         msg = ("Unable to renew non-existent lease. I have leases accepted by"
                " nodeids: ")
-        msg += ",".join([("'%s'" % idlib.b2a(anid))
+        msg += ",".join([("'%s'" % idlib.nodeid_b2a(anid))
                          for anid in accepting_nodeids])
         msg += " ."
         raise IndexError(msg)
@@ -507,8 +507,7 @@ class MutableShareFile:
         accepting_nodeids = set()
         modified = 0
         remaining = 0
-        blank = "\x00"*32
-        blank_lease = (0, 0, blank, blank, blank)
+        blank_lease = (0, 0, "\x00"*32, "\x00"*32, "\x00"*20)
         f = open(self.home, 'rb+')
         for (leasenum,(oid,et,rs,cs,anid)) in self._enumerate_leases(f):
             accepting_nodeids.add(anid)
@@ -523,7 +522,7 @@ class MutableShareFile:
             return (remaining, freed_space)
         msg = ("Unable to cancel non-existent lease. I have leases "
                "accepted by nodeids: ")
-        msg += ",".join([("'%s'" % idlib.b2a(anid))
+        msg += ",".join([("'%s'" % idlib.nodeid_b2a(anid))
                          for anid in accepting_nodeids])
         msg += " ."
         raise IndexError(msg)
@@ -538,7 +537,7 @@ class MutableShareFile:
         (magic,
          write_enabler_nodeid, write_enabler,
          data_length, extra_least_offset) = \
-         struct.unpack(">32s32s32sQQ", data)
+         struct.unpack(">32s20s32sQQ", data)
         assert magic == self.MAGIC
         return (write_enabler, write_enabler_nodeid)
 
@@ -565,7 +564,7 @@ class MutableShareFile:
             # accomodate share migration by reporting the nodeid used for the
             # old write enabler.
             msg = "The write enabler was recorded by nodeid '%s'." % \
-                  (idlib.b2a(write_enabler_nodeid),)
+                  (idlib.nodeid_b2a(write_enabler_nodeid),)
             raise BadWriteEnablerError(msg)
 
     def check_testv(self, testv):
@@ -667,8 +666,7 @@ class StorageServer(service.MultiService, Referenceable):
         if self.parent:
             nodeid = self.parent.nodeid # 20 bytes, binary
             assert len(nodeid) == 20
-            self.setNodeID(nodeid + "\x00"*12) # make it 32 bytes
-            # TODO: review this 20-vs-32 thing, settle on one or the other
+            self.setNodeID(nodeid)
 
     def _clean_incomplete(self):
         fileutil.rm_dir(self.incomingdir)

diff --git a/src/allmydata/test/test_mutable.py b/src/allmydata/test/test_mutable.py
index fdb5083b4670df33d8016e2988bc6d8105233e48..d488448d34ca730c562c05817917235be29e300d 100644 (file)
--- a/src/allmydata/test/test_mutable.py
+++ b/src/allmydata/test/test_mutable.py
@@ -103,7 +103,7 @@ class FakeNewDirectoryNode(dirnode2.NewDirectoryNode):
 class MyClient:
     def __init__(self, num_peers=10):
         self._num_peers = num_peers
-        self._peerids = [tagged_hash("peerid", "%d" % i)
+        self._peerids = [tagged_hash("peerid", "%d" % i)[:20]
                          for i in range(self._num_peers)]
 
     def get_renewal_secret(self):

diff --git a/src/allmydata/test/test_util.py b/src/allmydata/test/test_util.py
index 9e9ba3d42a34ff6208c8f0164b1cc91d179ded27..284bf5e5f82384743d8fc3258678d45424786a7f 100644 (file)
--- a/src/allmydata/test/test_util.py
+++ b/src/allmydata/test/test_util.py
@@ -17,6 +17,8 @@ class IDLib(unittest.TestCase):
     def test_a2b(self):
         self.failUnlessEqual(idlib.a2b("ne4y"), "\x12\x34")
         self.failUnlessRaises(AssertionError, idlib.a2b, "b0gus")
+    def test_nodeid_b2a(self):
+        self.failUnlessEqual(idlib.nodeid_b2a("\x00"*20), "a"*32)
 
 class NoArgumentException(Exception):
     def __init__(self):

diff --git a/src/allmydata/util/hashutil.py b/src/allmydata/util/hashutil.py
index 8424d755a79cc6d1917bd7dbebd5cd95af546e98..0bd0b6382ad2143ae22776091561deae2688f386 100644 (file)
--- a/src/allmydata/util/hashutil.py
+++ b/src/allmydata/util/hashutil.py
@@ -80,10 +80,12 @@ def file_cancel_secret_hash(client_cancel_secret, storage_index):
                             client_cancel_secret, storage_index)
 
 def bucket_renewal_secret_hash(file_renewal_secret, peerid):
+    assert len(peerid) == 20, "%s: %r" % (len(peerid), peerid) # binary!
     return tagged_pair_hash("bucket_renewal_secret",
                             file_renewal_secret, peerid)
 
 def bucket_cancel_secret_hash(file_cancel_secret, peerid):
+    assert len(peerid) == 20, "%s: %r" % (len(peerid), peerid) # binary!
     return tagged_pair_hash("bucket_cancel_secret",
                             file_cancel_secret, peerid)
 
@@ -122,10 +124,10 @@ def ssk_writekey_hash(privkey):
     return tagged_hash("allmydata_mutable_writekey_v1", privkey)
 def ssk_write_enabler_master_hash(writekey):
     return tagged_hash("allmydata_mutable_write_enabler_master_v1", writekey)
-def ssk_write_enabler_hash(writekey, nodeid):
-    assert len(nodeid) == 32 # binary!
+def ssk_write_enabler_hash(writekey, peerid):
+    assert len(peerid) == 20, "%s: %r" % (len(peerid), peerid) # binary!
     wem = ssk_write_enabler_master_hash(writekey)
-    return tagged_pair_hash("allmydata_mutable_write_enabler_v1", wem, nodeid)
+    return tagged_pair_hash("allmydata_mutable_write_enabler_v1", wem, peerid)
 
 def ssk_pubkey_fingerprint_hash(pubkey):
     return tagged_hash("allmydata_mutable_pubkey_v1", pubkey)

diff --git a/src/allmydata/util/idlib.py b/src/allmydata/util/idlib.py
index caf0c16e45a053ebf41e226b6cb8356695406b34..fa990b7a3d3ab7bcb7fb10adf020f415bcea8951 100644 (file)
--- a/src/allmydata/util/idlib.py
+++ b/src/allmydata/util/idlib.py
@@ -249,3 +249,7 @@ def a2b_l(cs, lengthinbits):
     precondition(b2a_l(res, lengthinbits) == cs, "cs is required to be the canonical base-32 encoding of some data.", b2a(res), res=res, cs=cs)
     return res
 
+from foolscap import base32
+def nodeid_b2a(nodeid):
+    # we display nodeids using the same base32 alphabet that Foolscap uses
+    return base32.encode(nodeid)