From c4f7412f1c5c0053faf3203b2973696c9ece954e Mon Sep 17 00:00:00 2001
From: Brian Warner <warner@allmydata.com>
Date: Tue, 6 Nov 2007 18:49:59 -0700
Subject: [PATCH] stabilize on 20-byte nodeids everywhere, printed with
 foolscap's base32

---
 docs/mutable.txt                   | 16 +++++-----
 src/allmydata/interfaces.py        |  3 ++
 src/allmydata/mutable.py           | 19 ++++++------
 src/allmydata/storage.py           | 48 ++++++++++++++----------------
 src/allmydata/test/test_mutable.py |  2 +-
 src/allmydata/test/test_util.py    |  2 ++
 src/allmydata/util/hashutil.py     |  8 +++--
 src/allmydata/util/idlib.py        |  4 +++
 8 files changed, 56 insertions(+), 46 deletions(-)

diff --git a/docs/mutable.txt b/docs/mutable.txt
index 2d64d60d..a67e8973 100644
--- 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 51999b63..d4cf522f 100644
--- 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 e27a2980..e207de59 100644
--- 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 bd9949ca..85dd5dc0 100644
--- 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 fdb5083b..d488448d 100644
--- 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 9e9ba3d4..284bf5e5 100644
--- 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 8424d755..0bd0b638 100644
--- 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 caf0c16e..fa990b7a 100644
--- 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)
-- 
2.45.2